Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Interpolation Issue - Combining Values #442

Closed
grayghostvisuals opened this Issue Jul 12, 2014 · 12 comments

Comments

Projects
None yet
6 participants

grayghostvisuals commented Jul 12, 2014

I have a user on my project that filed an issue in regards to interpolation using libsass w/gulpsass so I'm curious if this is a bug within libsass' interpolation as the current method I use works just fine according to the docs and compiling with Ruby.

Here's the code that seems to be the problem…

@function context-calc($scale, $base, $value) {
  @return ($scale/$base) + $value; // was @return ($scale/$base)#{$value};
}
@function measure-margin($scale, $measure, $value) {
  @return ($measure/$scale) + $value; // was @return ($measure/$scale)#{$value};
}

As you can see the commented lines were the original lines that worked just fine when compiling with Ruby otherwise libsass was giving this font-size: 32 rem as a result.

anyone willing to take a look at this yet? + @akhleung @nex3

Owner

hcatlin commented Oct 3, 2014

Yeah definitely looks like an issue with interpolation.

@hcatlin hcatlin added this to the 3.1 milestone Oct 3, 2014

Thanks for taking the time to look Hampton Inn & Suites ;) 🍻

malrase commented Oct 6, 2014

I'm in the process of writing a test and put the code into a gist:
http://sassmeister.com/gist/ed4788de313bcabe9f68

(That should be using Sass 3.4)

Is that the desired interpolation behaviour? I can see how the libsass result:

.test {
  margin: 10 1px 2px 3px; }

is not correct.

However, I guess I'd expect Sass 3.4 to give a result closer to

.test {
  margin: 101px 102px 103px; }

@malrase Results have a separation. I get this reported to me via libsass users… font-size: 32 rem Notice the gap between the last digit and the string “rem”

malrase commented Oct 6, 2014

Ah! I see what you mean. It's difficult to test right now, unfortunately.

http://sassmeister.com/gist/7b8f1f58a2b356aac923

The above will be the test when we can test it.

@hcatlin can you remove the "needs test" tag from this while we get it sorted out.

@malrase malrase referenced this issue in sass/sass-spec Oct 7, 2014

Merged

Issue 442 #71

Contributor

xzyfer commented Dec 22, 2014

I've moved this to 3.2 because it's appears to be non-insignificant fix and 3.1 is just about ready for release.

@xzyfer xzyfer modified the milestones: 3.2, 3.1 Dec 22, 2014

@xzyfer xzyfer referenced this issue Dec 22, 2014

Closed

Time for a 3.1 release! #697

12 of 12 tasks complete

Adding to the discussion: appending a unit as a string is a very poor way of converting a unitless number to a length since it results in having a string instead of a number, preventing any further calculation.

@hcatlin hcatlin changed the title from Interpolation Issue - Combining Values to Interpolation Issue - Combining Values [$15] Feb 21, 2015

@hcatlin hcatlin added the bounty label Feb 21, 2015

Contributor

mgreter commented Mar 2, 2015

IMHO this is a pretty difficult issue since it involes parse_list, which we currently always expect to be at least space separated. I have added a debugger.hpp file which contains a debug_ast function:

input:

foo { bar: (10/10)#{rem}; }

output:

foo { bar: 1 rem; }

debug_ast:

Block 0x2630200 0
 Ruleset 0x22652c0 0
  Selector_List 0x26302e0 - -
   Complex_Selector 0x26e6300 - - -> { } <> X < >
    Compound_Selector 0x23993c0 - - <> X < >
     Selector_Reference 0x2991a20 @ref 0
   -Complex_Selector 0x26e61a0 - - -> { } <> X <>
   - Compound_Selector 0x23992d0 - - <> X <>
   -  Type_Selector 0x2991980 <<foo>> - <> X < >
  Declaration 0x2b27dc0 0
   prop: String_Quoted : 0x26e63b0 [bar] < > X <>
   value: List 0x2b27cf0 [Space] [delayed: 0]  [interpolant: 0]
   value:  Binary_Expression 0x26e65c0 [11]
   value:   left:  Textual  [NUMBER]0x26e6460 [10]
   value:   right: Textual  [NUMBER]0x26e6510 [10]
   value:  String_Schema 0x2b30d90 4 {has_interpolants}
   value:   String_Quoted : 0x26e6670 [rem] {delayed} <> X <>

As you can see this gets parsed as a space separated list, and that's the reason why it fails! IMHO this needs to be parsed and render completely different, so this doesn't look like an easy fix!

Also consider this example:

input.scss

// interpolated-urls
fudge { walnuts: blix"fludge"#{hey now}123; }

ruby sass output:

fudge { walnuts: blix "fludge"hey now123; }

libsass output:

fudge { walnuts: blix"fludge"hey now123; }

The differences are very subtile and I have no idea yet what the actual rule behind it is (and if it actually makes sense or if it should be considered a ruby sass bug).

Contributor

xzyfer commented Mar 3, 2015

Agreed I've looked into this a couple times and it's non trivial. Ruby Sass handles this my having a separate SassScript parser.

I also think our approach to "everything is a list" is a bit overzealous and is the cause some tricky bugs.

Contributor

mgreter commented Mar 3, 2015

IMO we should re-scope this to Milestone 3.3

Contributor

xzyfer commented Mar 3, 2015

👍
On 4 Mar 2015 08:43, "Marcel Greter" notifications@github.com wrote:

IMO we should re-scope this to Milestone 3.3


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

@mgreter mgreter modified the milestones: 3.3, 3.2 Mar 4, 2015

mgreter added a commit to mgreter/libsass that referenced this issue May 11, 2015

@mgreter mgreter modified the milestones: 3.2.4, 3.3 May 11, 2015

mgreter added a commit to mgreter/libsass that referenced this issue May 11, 2015

mgreter added a commit to mgreter/libsass that referenced this issue May 11, 2015

mgreter added a commit to mgreter/libsass that referenced this issue May 12, 2015

@mgreter mgreter closed this in #1198 May 12, 2015

@mgreter mgreter removed the bounty label Oct 22, 2016

@hcatlin hcatlin changed the title from Interpolation Issue - Combining Values [$15] to Interpolation Issue - Combining Values [$15 awarded] Nov 4, 2016

@hcatlin hcatlin added the bounty label Nov 4, 2016

@hcatlin hcatlin changed the title from Interpolation Issue - Combining Values [$15 awarded] to Interpolation Issue - Combining Values Jan 30, 2017

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