Skip to content

feat: implement std.remove and std.removeAt#1071

Merged
sparkprime merged 2 commits into
google:masterfrom
rohitjangid:feat/array-remove
May 25, 2023
Merged

feat: implement std.remove and std.removeAt#1071
sparkprime merged 2 commits into
google:masterfrom
rohitjangid:feat/array-remove

Conversation

@rohitjangid
Copy link
Copy Markdown
Member

Implement std.remove and std.removeAt

go-jsonnet PR: google/go-jsonnet#689

Comment thread doc/_stdlib_gen/stdlib-content.jsonnet Outdated
availableSince: 'upcoming',
description: html.paragraphs([
|||
Remove given <code>elem</code> from <code>arr</code>.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically the first one found

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh it actually removes all of them! Say that then :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it seems the go version only removes the 1st one and that's the semantics i'd expect from the name of the function so I guess that's what you wanted.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is for first occurrence. Made appropriate changes.

Comment thread stdlib/std.jsonnet

removeAt(arr, at):: [
arr[i],
for i in std.range(0, std.length(arr) - 1)
Copy link
Copy Markdown
Contributor

@sparkprime sparkprime May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: Never mind, this is fine

Comment thread stdlib/std.jsonnet Outdated

isEmpty(str):: std.length(str) == 0,

remove(arr, elem):: [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can express this as something like:

local indexes = std.find(arr, at);
if std.length(indexes) == 0 then
  return arr
else
  return std.removeAt(arr,indexes[0])

So it does not remove all of them

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@rohitjangid rohitjangid force-pushed the feat/array-remove branch from 15a935f to 74ec9b0 Compare May 4, 2023 04:19
@sparkprime sparkprime merged commit 9f9cf3a into google:master May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants