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

Interpolated selectos and three letters long id selectors #2481

Closed
SomMeri opened this issue Feb 26, 2015 · 20 comments
Closed

Interpolated selectos and three letters long id selectors #2481

SomMeri opened this issue Feb 26, 2015 · 20 comments

Comments

@SomMeri
Copy link
Member

SomMeri commented Feb 26, 2015

Interpolation treats three letters long id selectors as colors and converts them into their long form:

@id: #abc;

.table @{id} {
  property: value;
}

compiles into

.table #aabbcc {
  property: value;
}

expected result:

.table #abc {
  property: value;
}

Note: string interpolation does the same thing.

Workaround - store only identifier in the variable:

@clean: abc;

.table #@{clean} {
  property: value;
}
@seven-phases-max
Copy link
Member

expected result:

This (a related problem) was discussed once at #1648 (comment). The problem is that #, . symbols are ambiguous if you try to define a value with selector name (obviously since initially there was no selector interpolation, # and . are supposed to always mean a color and a floating-point number respectively (the latter is just parsed as Anonymous though when followed by a non-digit)). So for this to work in general the variable evaluation has to be "where-it-is-used" context depended, but currently the type of a variable has to be determined before it's added to the tree, hence it's all may be quite tricky to change.

Actually the example can be simplified to just:

@who-am-i: #abc;

foo {
    property: @who-am-i; // -> #aabbcc
}

A quick fix would be to parse #abc there as Anonymous too, but it will solve this specific case only and not the problem itself: variable values are not designed to hold CSS selectors (that may look like .a >.b.c .d + #e~ #f after all :). And the more common workaround is of course ~"" (which is ugly as always).

P.S. Ah, sorry, missed the fact that #abc cannot be parsed as Anonymous since it's a valid color value (and values like #foo are parsed as Anonymousalready).

@SomMeri
Copy link
Member Author

SomMeri commented Feb 26, 2015

@seven-phases-max One solution would be to keep original color format in color node. It could get lost during math operations and functions, but be kept when assigning variables to variables etc. If original color format would be available, that one would be printed into output.

@lukeapage
Copy link
Member

lukeapage commented Feb 26, 2015 via email

@seven-phases-max
Copy link
Member

@SomMeri Could be (this is similar to what I did for #1604). Though in general, for the selector interpolation case, it would mean unfortunate duplication of code, i.e. another to-css-ish-for-selector-interpolation function/flag to be set/used at the selector rendering code. Besides the issue can be extrapolated to:

@bar: 1-2; // or even more fun 1/3 etc etc.

.foo-@{bar} {
    _:;
}

:) This is totally artificial example of course but to illustrate it's not only color issue... I.e. almost every complex tree object would have to provide that special to-string method (in addition to toCSS).

@seven-phases-max
Copy link
Member

Yes, I guess I will agree with @lukeapage for the need of selector-like special function (though behind the scenes I think I'll keep in mind some lobby for allowing to use basic selector values (e.g. #foo or .bar but not #abc and .foo > .bar) w/o selector function as a syntactic sugar exception (maybe... eventually...)).

@rjgotten
Copy link
Contributor

I want a way of describing selectors in variables e.g. $(#abc) or selector(#abc)

Would be awesome if you could also capture current selector scope that way, for later re-use.
E.g.

.mixin() {
  @selector : selector(&);
  // ( ... )

  &-sub {
    // ( ... )

    & + @{selector} {
      // ( ... )
    }
  }
}

// ( ... )

.prfx-foobar { .mixin(); }
.prfx-foobar {
  // ( ... )
}

.prfx-foobar-sub {
  // ( ... )
}

.prfx-foobar-sub + .prfx-foobar {
  // ( ... )
}

Could come in quite handy when you want to parent everything in a mixin against a common ancestor CSS classname, as is the case when you develop mixins for a BEM-style framework of components.

@SomMeri
Copy link
Member Author

SomMeri commented Feb 27, 2015

@seven-phases-max I would not duplicate code, just render it in original (shorthand) notation everywhere. Browsers should be able to accept that, so there is no reason to split flow depending on context.

@seven-phases-max
Copy link
Member

@SomMeri I mean duplicated code in the compiler (assuming the same fix method to be used for every complex object and not just this color example).

@rjgotten Please create a dedicated ticket (your request has really nothing to do with this issue).

@SomMeri
Copy link
Member Author

SomMeri commented Feb 27, 2015

@seven-phases-max I through about doing it only for colors. It did not occurred me to do it for everything. So the change would be only in parser or whoever it is that fills this.value in toCss function of color object.

@seven-phases-max
Copy link
Member

@SomMeri Then it's OK I guess.

Though.. hmm... then when/if selector function is implemented what would be a plan? Notice the following example:

its-not-just-variable {
    .mixin(#abc); // OK
    .mixin(#foo); // Err.
}

Obviously the second statement would require selector function and it becomes a little bit inconsistent (with this fix we would allow #abc to be used as non-color w/o any selector so then we won't be able to require selector for both statements anymore).

Or maybe I'm just overcomplicating this and you should just do it and let it fly whatever way it flies :)

@SomMeri
Copy link
Member Author

SomMeri commented Mar 2, 2015

I think that it should be solved the same way as this, e.g. they are the same problem:

its-not-just-variable {
    .mixin(#aabbcc); // OK
    .mixin(#foofoo); // Err.
}

@seven-phases-max
Copy link
Member

I think that it should be solved the same way as this

And that's what my initial concern is. It's too many examples to fix if we're about fixing them all (in that case selector function solution looks more simple and solid) and if we're about fixing only #abc case it looks too minor and brings a few more inconsistencies in (those examples can be further extrapolated to functions and so on: e.g. lighten(#aabbcc, 10%); vs. replace(#aabbcc, b, x); etc.).

Now, after all, how many real code you can imagine out there that is really to use a CSS selector with an identifier being ambiguous with a hex color? (i.e. # with 3 or 6 of a to f)? :)

@SomMeri
Copy link
Member Author

SomMeri commented Mar 3, 2015

I meant that your and my example should be solved by the same code. There is no reason to have different code path for #080 and #008800 beyond rgb decomposition and parser.

It is just the question of knowing that color can be also valid id, you do not need to list all the ways in which they can be the same. That is something you can not avoid because lime=#080=#008800 . If they are consistently treated the same (e.g. stored inside the same object and using same algorithm), then you do not need special code for every case. Both #080 and #008800 can be used as selector ids, so it can go through same code path. Technically, also lime could be part of selector as tag name in some weird extension.

The replace(#aabbcc, b, x); function works only on strings and escaped strings, so it will not work on none of these three lime, #080, #008800 nor on replace(identifier, b, x);

The lighten(#aabbcc, 10%); works only on colors, so it should work on all three.

@seven-phases-max
Copy link
Member

OK, now I guess I get it. So you mean if variable is defined as #abc then it should always be output as #abc (and not as #aabbcc), right? This is possible (actually this would be a tiny patch on top of the existing code, notice how the following code works in current versions:

@x: lime;
@{x} {
    y: replace(lime, i, y);
}

).

But still for a consistency this ideally would need more changes in the parser so that it works the same for #abc and #foo (the latter is not recognized as a valid arg for mixins and functions).

@SomMeri
Copy link
Member Author

SomMeri commented Mar 5, 2015

Yes, that is what I meant. Just using the same code that already exists because of lime. I think it was originally added because of indirect variables.

You think that #foo or #foofofofofo should be parsed as color or rather some other kind of object? (I have no idea how it works now.)

@seven-phases-max
Copy link
Member

You think that #foo or #foofofofofo should be parsed as color or rather some other kind of object?

No. I mean it does not really matter how it's to be parsed (well, currently @var: #foo; is parsed as Anonymous) but the matter of parsing it (when used as arg for mixins and functions) at all. I.e. summarising all above, for now it's OK to explain why these work differently:

div {
    .mixin(#abc); // OK
    .mixin(#foo); // Err.
}

Currently the excuse is: the first is a color value (hence it works) and the second is some unknown "typo" (hence it bails out). But as soon as #abc turns into a generic value (it does not matter if internally it's stored as a color object) we don't have such excuse anymore.

@matthew-dean
Copy link
Member

I don't see how this is an issue when you can use a string to define the selector. Seems pretty expected that a non-quoted #abc value would be interpreted as a color. I think if you want to assign a selector to a variable, you should have to do something like:

@id: ~"#abc";

.table @{id} {
  property: value;
}

How is this a significant problem worth changing?

@SomMeri
Copy link
Member Author

SomMeri commented Apr 23, 2015

@matthew-dean It looks like a bug and in all likelihood trivial to fix.

@seven-phases-max
Copy link
Member

I'm still concerned of the inconsistencies the "trivial fix" does not takes into account, e.g.:

@a: #abc; // OK

@{a} {1: 1}

@b: #foo; // OK

@{b} {2: 2}

@c: #abcfoo; // SyntaxError: Invalid HEX color code

@{c} {3: 3}

In other words, I'd expect if we legalize the code like this, then all three cases should pass. (By now while the first two pass OK, they still are in "unspecified behaviour" category. But if we make this officially supported syntax it should become consistent. As I already mentioned above it would be not so easy to reasonably answer "why first two are OK and the 3rd isn't?" stuff).

@SomMeri
Copy link
Member Author

SomMeri commented Jul 20, 2015

@seven-phases-max I see this mostly as applying the "keep color in its original form" rule to shorthand colors too. So I see the inconsistency you mention as slightly different issue (although likely to be easy to fix too).

Since following thing compiles:

@c: .abcfoo;
@{c} {3: 3}

into:

.abcfoo {
  3: 3;
}

Maybe #abcfoo; should parse into identifier the same way as .abcfoo does. I am willing to do that if we agree on this, but I do not really have an opinion on that so you tell me :).

My primary goal is to close all easily closeable bugs I can find, either by fixing or closing as 'wont fix'. There is no advantage in keeping tiny stuff like this open forever. Feel free to suggest favorites if you have some.

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

5 participants