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

Regression: String functions #988

Closed
Snugug opened this issue Mar 27, 2015 · 5 comments
Closed

Regression: String functions #988

Snugug opened this issue Mar 27, 2015 · 5 comments

Comments

@Snugug
Copy link

Snugug commented Mar 27, 2015

node-sass 3.0.0-beta which gives me access to Libsass 3.2.0-beta.2 and I've discovered a regression: In Libsass 3.1, This str-replace function works without a hitch. I believe it may be related to #987, but this particular function worked in 3.1 and no longer works in 3.2:

@function str-replace($string, $search, $replace: '') {
  $index: str-index($string, $search);

  @if $index {
    @return str-slice($string, 1, $index - 1) + $replace + str-replace(str-slice($string, $index + str-length($search)), $search, $replace);
  }

  @return $string;
}


$string: 'Foo Bar Baz Qux';

.foo {
  content: str-replace($string, ' ', '-');
}

Original Output

.foo {
  content: "Foo-Bar-Baz-Qux"; }

**Regressed Output in Libsass 3.2

.foo {
  content: "Foo"-Bar-Baz-Qux; }

How should I go about writing a test to catch this? It's not Sass function, but it's a regression that should be tested for.

@mgreter
Copy link
Contributor

mgreter commented Mar 27, 2015

I already have this on my radar and a proper fix will need some more re-factoring, as it will cause another spec-test to fail otherwise. Altough I have a hotfix which I probably will include in 3.2.

in functions.cpp line 945

- return new (ctx.mem) String_Constant(pstate, newstr);
+ if (newstr == "\"\"") new (ctx.mem) String_Constant(pstate, newstr);
+ return new (ctx.mem) String_Quoted(pstate, newstr);

@mgreter mgreter self-assigned this Mar 27, 2015
@mgreter mgreter added this to the 3.2 milestone Mar 27, 2015
@Snugug
Copy link
Author

Snugug commented Mar 27, 2015

@mgreter Great, thanks. That's exactly where it's choking, on \". I've got a URL Encode function I'm using and the error I'm getting happens when I'm trying to encode that bit.

@mgreter
Copy link
Contributor

mgreter commented Mar 27, 2015

If you can, please test with PR #989 applied and report if it fixes your issues! Thanks!

@Snugug
Copy link
Author

Snugug commented Mar 27, 2015

I'm not entirely sure if I'm able to do that on pure Libsass :\ I compile through node-sass and not entirely sure what the process is for testing Libsass updates through that.

@drewwells
Copy link
Contributor

@mgreter pr #989 fixes this issue 👍

/* line 19, stdin */
.foo {
  content: "Foo-Bar-Baz-Qux"; }
Rebuilt: test.scss
2015/03/27 13:16:39 main.go:94: Compilation took: 8.654ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants