Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal for string.split #3284
Proposal for string.split #3284
Changes from 1 commit
b3fbef3
6652244
c3d3df3
f6db4e0
03e841f
e79584d
4b663fe
ead86d4
a17bab4
f8f4a96
52ee123
16ba697
719f988
084e054
9109048
07c4fc2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the limit truncate the end of the string, or should it take all the remainder in the last part ? Javascript is one of the few languages implementing the truncation (which is easy to achieve otherwise by doing an unlimited split and slicing the array)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stof Are you saying, should this work by 1) making all the splits and then slicing the array at the limit or 2) only splitting up to the limit and then stopping?
Do you think there would be downsides to taking one approach or the other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, to me, it would be more useful if
limit
would work by splitting the string on thelimit -1
first separators, with the last item containing all the remaining work.Your current proposal does the opposite (truncating the remaining parts), and this could be achieved through an unlimited split and then slicing the list (but I just figured out that
list.slice
does not exist in Sass).I remember reading an article a few weeks ago comparing the behavior of the string splitting API of lots of different languages in that regard (but unfortunately, I did not manage to find it back for now to share it here. But AFAIR, most languages are providing the non-truncating version of the API (Javascript being one of the exceptions).
Also, a benefit of the non-truncation is that we don't loose content when splitting the string, so joining the list items with the separator (not exposed as a function in
sass:list
either btw) always produces the original string.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd lean towards matching the JS behavior here. @nex3: Do you have any thoughts on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @stof here. I think JS's behavior is a misfeature, and we should match other languages in preserving the entire remainder of the string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, let's change this to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jathak I've updated this section (and the examples) to show
limit
working as noted in the conversation above. The function should preserve the remainder of the string now.@mirisuzanne made a function based on these steps that you can see here