Convert values containing commas inside of an object into strings #55
Conversation
|
Thanks for the continuing contributions, @timkraut! This issue is unfortunately not straightforward and has come up before. We actually made this fix and reverted it. The ambiguous handling of strings in |
|
I read through the discussions on this topic. Unfortunately, I don't have a general solution which works for everything. I'm not sure if there is even one. My PR addresses one particular case which should be rather clear, though: If the value contains a comma, it should be always a string. In this case, I guess it's save to wrap them in a string. |
test/index.js
Outdated
| "key": "value with , somewhere", | ||
| })).to.eql('(key: "value with , somewhere")') | ||
| }); | ||
| it('can parse nested maps with invalid keys', function() { |
There was a problem hiding this comment.
I'm confused why this test shows up in the diff given that it was merged yesterday. Please rebase if you haven't
| } else if (value === '') { | ||
| return '""'; // Return explicitly an empty string (Sass would otherwise throw an error as the variable is set to nothing) | ||
| } else if (_.isString(value) && value.includes(',')) { | ||
| return '"' + value + '"'; // Sass throws an error if a value contains a comma and is not treated as a string |
There was a problem hiding this comment.
What about something like hsl(0,0,0)? Sass wouldn't throw, right?
There was a problem hiding this comment.
You're right... I haven't thought about this... this makes it more difficult
There was a problem hiding this comment.
I'm unsure if this is something people use, but it'd be a breaking change. Is there a list of SASS functions we could refer to?
There was a problem hiding this comment.
I don't think there is one. We would have to create one manually, I guess.
Maybe instead of checking a list of functions, we could just check for (what, ever).
There was a problem hiding this comment.
I think that sounds like a good compromise!
There was a problem hiding this comment.
I've implemented the discussed changes. I've adapted the initial PR description to reflect the changes.
Convert values containing commas inside of an object into strings
2 participants
We ran into an issue with the following code:
This results in:
which makes SASS fail.
This PR converts values outside of a function which contain a comma into a string to avoid that.
The new outcome is:
If the input contains a function, it is not wrapped in strings:
Input:
Output: