Skip to content
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

vararg parameters are being modified #813

Closed
scamden opened this issue Jan 6, 2015 · 23 comments · Fixed by #833
Closed

vararg parameters are being modified #813

scamden opened this issue Jan 6, 2015 · 23 comments · Fixed by #833

Comments

@scamden
Copy link

scamden commented Jan 6, 2015

if I call a mixin or function using the followin syntax the array is cleared:

@function foo($one, $two) {
  @return $one + $two; 
}

$nums: 1px 2px;

.foo {
  left: foo($nums...);
  bottom: $nums 3px;
}

expected

.foo {
  left: 3px;
  bottom: 1px 2px 3px; }

actual

.foo {
  left: 3px;
  bottom: 3px; }

this is a problem if you're reusing the array and calling things this way multiple times

Spec added sass/sass-spec#239.

@scamden
Copy link
Author

scamden commented Jan 6, 2015

@xzyfer changelog shows you added basic support for this feature, any help? we would love to use libsass but we use this syntax everywhere

@xzyfer
Copy link
Contributor

xzyfer commented Jan 6, 2015

@scamden do you know which version of libsass you're using? Are you using it via sassc or a 3rd party like node-sass, grunt-sass, gulp-sass etc..

@scamden
Copy link
Author

scamden commented Jan 6, 2015

i'm using gulp sass, but i manually installed node-sass@2.0.0-beta, to get
some of your latest features, not sure which version they're using since
it's not a node package, where could i find that info?

On 6 January 2015 at 14:56, Michael Mifsud notifications@github.com wrote:

@scamden https://github.com/scamden do you know which version of
libsass you're using? Are you using it via sassc or a 3rd party like
node-sass, grunt-sass, gulp-sass etc..


Reply to this email directly or view it on GitHub
#813 (comment).

Sterling Camden
Software Engineer
502 Emerson Street | Palo Alto, CA 94301
303.520.2968 | sterling@relateiq.com

@xzyfer
Copy link
Contributor

xzyfer commented Jan 7, 2015

I believe the node-sass CLI has a version flag now.

If you could create a test case on http://sassmeister.com that'd be great.
On 7 Jan 2015 09:58, "Sterling Camden" notifications@github.com wrote:

i'm using gulp sass, but i manually installed node-sass@2.0.0-beta, to
get
some of your latest features, not sure which version they're using since
it's not a node package, where could i find that info?

On 6 January 2015 at 14:56, Michael Mifsud notifications@github.com
wrote:

@scamden https://github.com/scamden do you know which version of
libsass you're using? Are you using it via sassc or a 3rd party like
node-sass, grunt-sass, gulp-sass etc..


Reply to this email directly or view it on GitHub
#813 (comment).

Sterling Camden
Software Engineer
502 Emerson Street | Palo Alto, CA 94301
303.520.2968 | sterling@relateiq.com


Reply to this email directly or view it on GitHub
#813 (comment).

@scamden
Copy link
Author

scamden commented Jan 7, 2015

here's the repro using the libsass version that' son there:

http://sassmeister.com/gist/a3d01d4828d7e07f48f4

On 6 January 2015 at 16:46, Michael Mifsud notifications@github.com wrote:

I believe the node-sass CLI has a version flag now.

If you could create a test case on http://sassmeister.com that'd be
great.
On 7 Jan 2015 09:58, "Sterling Camden" notifications@github.com wrote:

i'm using gulp sass, but i manually installed node-sass@2.0.0-beta, to
get
some of your latest features, not sure which version they're using since
it's not a node package, where could i find that info?

On 6 January 2015 at 14:56, Michael Mifsud notifications@github.com
wrote:

@scamden https://github.com/scamden do you know which version of
libsass you're using? Are you using it via sassc or a 3rd party like
node-sass, grunt-sass, gulp-sass etc..


Reply to this email directly or view it on GitHub
#813 (comment).

Sterling Camden
Software Engineer
502 Emerson Street | Palo Alto, CA 94301
303.520.2968 | sterling@relateiq.com


Reply to this email directly or view it on GitHub
#813 (comment).


Reply to this email directly or view it on GitHub
#813 (comment).

Sterling Camden
Software Engineer
502 Emerson Street | Palo Alto, CA 94301
303.520.2968 | sterling@relateiq.com

@xzyfer
Copy link
Contributor

xzyfer commented Jan 7, 2015

@scamden I can't reproduce this. Everything appears to be working in your gist. Try compiling the 3.1.0 stable release.

@xzyfer xzyfer self-assigned this Jan 7, 2015
@scamden
Copy link
Author

scamden commented Jan 7, 2015

You can't reproduce in sassmeister? Or locally?
On Wed, Jan 7, 2015 at 5:44 AM Michael Mifsud notifications@github.com
wrote:

@scamden https://github.com/scamden I can't reproduce this. Everything
appears to be working in your gist. Try compiling the 3.1.0 stable release.


Reply to this email directly or view it on GitHub
#813 (comment).

@scamden
Copy link
Author

scamden commented Jan 7, 2015

screen shot 2015-01-07 at 10 40 18 am
screen shot 2015-01-07 at 10 40 28 am

@xzyfer
Copy link
Contributor

xzyfer commented Jan 7, 2015

I can't reproduce it either locally or on sassmeister - http://sassmeister.com/gist/855ff3f3c38808f6fcdb

I think you mean be running in to a bug with sassmeister. Try creating a new gist with Libsass.

@scamden
Copy link
Author

scamden commented Jan 7, 2015

What would be the recommended method for that?
On Wed, Jan 7, 2015 at 1:45 PM Michael Mifsud notifications@github.com
wrote:

I can't reproduce it either locally or on sassmeister -
http://sassmeister.com/gist/855ff3f3c38808f6fcdb

I think you mean be running in to a bug with sassmeister. Try creating a
new gist with Libsass.


Reply to this email directly or view it on GitHub
#813 (comment).

@xzyfer
Copy link
Contributor

xzyfer commented Jan 7, 2015

I've confirmed the supplied code produces

.myClass {
  left: 3px;
  bottom: 1px; }

in Libsass 3.1.0-beta, 3.1.0-beta.2, and 3.1.0 stable.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 7, 2015

@scamden
Copy link
Author

scamden commented Jan 7, 2015

Exactly that is the incorrect output
On Wed, Jan 7, 2015 at 1:48 PM Michael Mifsud notifications@github.com
wrote:

I've confirmed the supplied code produces

.myClass {
left: 3px;
bottom: 1px; }

in Libsass 3.1.0-beta, 3.1.0-beta.2, and 3.1.0 stable.


Reply to this email directly or view it on GitHub
#813 (comment).

@xzyfer
Copy link
Contributor

xzyfer commented Jan 7, 2015

What is the expected output?

@xzyfer
Copy link
Contributor

xzyfer commented Jan 7, 2015

Apologies, I was running into the Sassmeister bug. It was showing me Libsass output to Ruby sass.

@xzyfer xzyfer changed the title using ellipsis to expand an array into a set of arguments clears the original array vararg parameters are being modified Jan 7, 2015
@xzyfer
Copy link
Contributor

xzyfer commented Jan 7, 2015

I've update the issue description to clarify the problem.

@scamden
Copy link
Author

scamden commented Jan 7, 2015

@xzyfer awesome thank you!

@am11
Copy link
Contributor

am11 commented Jan 8, 2015

Guys this node-sass issue is also related: sass/node-sass#608. :)

@xzyfer
Copy link
Contributor

xzyfer commented Jan 8, 2015

Thanks @am11 it was reported #824 and closed a duplicate.

@am11
Copy link
Contributor

am11 commented Jan 8, 2015

Cool! I think instead of rerouting peeps, I should make a habit of submitting specs myself to make things easier for you guys to manage. :)

@xzyfer
Copy link
Contributor

xzyfer commented Jan 8, 2015

It gets tricky because our current practice is to name a spec after the issue.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 8, 2015

Also the code supplied is generally not idea for a spec. The root cause has to be narrowed down and the spec made as simple as possible. This is important since we often debug tricky issues against spec so the less sass code the better.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 12, 2015

This fixed and will be 3.2.

@xzyfer xzyfer added this to the 3.2 milestone Jan 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants