-
Notifications
You must be signed in to change notification settings - Fork 23
adding 'to-string' function #84
base: master
Are you sure you want to change the base?
Conversation
I believe we should just use https://github.com/HugoGiraudel/SassyJSON/blob/master/stylesheets/encode/types/_string.scss. |
Hmm, that uses the "_proof-quote" function, which is the function in which we are trying to convert the value to a string. As a result, I'm getting "SystemStackError: stack level too deep". |
Ouch... That doesn't sound to good. I will have a closer look. Le mar. 8 sept. 2015 21:28, Edmund [email protected] a écrit :
|
// @param {String} $glue ('') - value to use as a join string | ||
|
||
@function to-string($list, $glue: '', $is-nested: false, $recursive: false) { | ||
$result: null; |
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 be ''
.
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.
Too large indentation, I'd say?
I've made the requested changes, and it's working perfectly on my local project. Just one thing regarding this updated line: $result + #{$e} + #{$glue}, $result#{$e} Perhaps this is expected, but this doesn't work: $result + #{$e} + #{$glue}, $result + #{$e} It outputs the '+' symbol. |
If you concatenate with |
Ok, I think we got there :) My butchered attempt at concatenating was causing issues when using |
Weird, a test is failing. |
@for $i from 1 through length($list) { | ||
$e: nth($list, $i); | ||
@if type-of($e) == 'list' and $recursive { | ||
$result: $result + #{to-string($e, $glue, true, true)}; |
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.
No need for interpolation here. :)
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.
$is-nested
and $recursive
should be passed rather than true
and true
.
Done and done :) Test still seems to be failing though? Cheers |
Not quite sure why... Might be just the linting. |
Actually I have been double-checking this and I think we don't need this |
o_O Well my project still works as expected after removing this, but I was definitely having issues which converting the value to a string fixed. I can't right now replicate it, but I believe it was when trying to pass a value of 'null' to the function, resulting in 'Invalid null operation: """ plus null".'. |
For now I'd say the only thing we need is getting rid of this |
So I just realised that SassyJSON does not come with your 'to-string' function - I just happened to already be using it for something else which is why it worked for me. And of course my previous pull-request relies on this function.
I've tried to be as consistent as possible when creating the new file but please let me know if you need any changes.