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

Is escaping supposed to work with @supports? #3059

Closed
JayPanoz opened this issue Apr 29, 2017 · 24 comments
Closed

Is escaping supposed to work with @supports? #3059

JayPanoz opened this issue Apr 29, 2017 · 24 comments

Comments

@JayPanoz
Copy link

Much to my surprise, since @supports is bubbling like @media, it seems that you can’t use any arbitrary string as a variable value for it.

While

@min768: ~"(min-width: 768px)";
.element {
  @media @min768 {
    font-size: 1.2rem;
  }
}

results in

@media (min-width: 768px) {
  .element {
    font-size: 1.2rem;
  }
}

Escaping a string with @supports

@test: ~"(font-variant-caps: small-caps)";

.small-caps {
  @supports @test {
    font-variant-caps: small-caps;
  }
}

results in

@supports @test {
  .small-caps {
    font-variant-caps: small-caps;
  }
}

It failed in any implementation I tried (more or less 20) so I guess this is probably a core issue/feature request?

Is this result to be expected?

Problem is you can have awful feature queries like

@supports ((-ms-font-feature-settings: 'dlig') or (-webkit-font-variant-ligatures: discretionary-ligatures) or (font-variant-ligatures: discretionary-ligatures))

and escaping would be very welcome for such cases, especially as people discover @supports with the grid spec, Open Type features and stuff.

@seven-phases-max
Copy link
Member

seven-phases-max commented Apr 30, 2017

@min768: ~"(min-width: 768px)";

This is a stone age hack. Use Passing Rulesets to Mixins instead, e.g.:

// usage:
.foo {
    .if-small-caps({
        font-variant-caps: small-caps;
    });
}

// impl:
.if-small-caps(@style) {
    @supports (font-variant-caps: small-caps) {
        @style();
    }
}

I'll add feature-request label to this ticket (this was mentioned earlier I guess, but no dedicated ticket yet) - but personally I'd rather vote for removing this macro-like replacement from @media eventually.

@matthew-dean
Copy link
Member

matthew-dean commented Apr 30, 2017

@min768: ~"(min-width: 768px)"; This is a stone age hack.

I would tend to disagree. Wrapping media queries (or support queries in this instance) in mixins is just one additional layer of abstraction. I much prefer something like @media @tablet { } to declare variations of a class for different @media targets.

Since @supports is supposed to bubble like @media, it's intuitive to be able to use the same variable replacement features. So I would label this a bug, just because users won't necessarily know that @media was historically special-cased from other at-rules. Any at-rules that can have rulesets should have the same general features IMO.

@seven-phases-max
Copy link
Member

seven-phases-max commented Apr 30, 2017

Well, the problem with these "macro-like" variable usage is that it needs a special dedicated support everywhere it's used and inherits all the cons of the vanilla text-replacement macros w/o bringing in any pros of them (at least a consistency of where and how they can be used).
If we need vanilla macros then they should be real macros not some hardcoded "variable-as-a-macro" code spread all over the compiler code-base w/o any consistency.

Someone would also find it useful to have something like:

@var: ~"rgb(128,";
color: @var 255, 0);

with the equal justification ("I don't need any abstraction level bla-bla-bla").

@matthew-dean
Copy link
Member

matthew-dean commented Apr 30, 2017

o_O

Er, an un-parseable monstrosity is not an equal example. I'm not sure I agree with the description that it's a macro. It's creating an anonymous node, and it can go where anonymous nodes can go. Between color: and 255, 0); (an un-parseable value fragment) is not one of those.

However, it COULD look like:

@var: ~"rgb(128,";
color: @var ~"255, 0)";

...because it's just output of two simple anonymous nodes. There's nothing wrong with that in Less, so I don't get the argument. Anonymous node output is available pretty much (almost) anyplace in Less. If a variable that evaluates to an anonymous node is not working as expected, it's a bug. It has nothing to do with "macros".

Your argument seems to be just that you don't like the way that code looks, which is fine. Or that alternatives should be promoted, which is also fine. But that's really just a matter of programmer preference. It's still important that the feature is consistent as-is.

@matthew-dean
Copy link
Member

matthew-dean commented Apr 30, 2017

Other alternatives as future features might be:

  1. Allowing @var: (min-width: 768px) to be parseable, so it looks less "hackish".
  2. Requiring at-rules to have something like @media @{var} (like selectors and properties) rather than @media @var, which might also look less hackish, since one is a variable in that declaration, and one is not.

However, those are just alternatives for possible future discussions, and neither is related to the core issue of this thread, which is just at-rule consistency.

@seven-phases-max
Copy link
Member

seven-phases-max commented Apr 30, 2017

Er, an un-parseable monstrosity is not an equal example.

Absolutely equal. Content of ~"(min-width: 768px)" is never parsed so it's possible only because the @media some-values { pattern is hardcoded right into the parser and the MediaQuery type itself (Not because the variable expands to a valid media features block). Just an easy-n-dirty kludge to quickly bring "what they asked" in - non-maintainable non-generic (that's what we can see right now - if you need something like this elsewhere, e.g. within @support, you need to go and implement this kludge for that elsewhere separately).

@matthew-dean
Copy link
Member

so it's possible only because the @media some-values { pattern is hardcoded right into the parser

I think we're both agreeing on the same thing, and just suggesting a different outcome. Those patterns were hard-coded into @media, but then when at-rules support was made more generic, it seems those patterns were then narrowly still tied to @media at the parsing/evaluation stages.

So, I wouldn't suggest special-casing variable support to @supports; I would suggest just removing "special-case" support for @media so that all "wrapping" at-rules have the same variable interpolation features.

Probably the original version of @media @var should have looked like @media @{var} to look less ambiguous. But what the ~"" syntax allows in Less is for developers to essentially "fill-in" whatever parsing / styling feature may not be directly supported in Less, and just output a string as-is (and un-quoted). It's a powerful feature that allows a lot of flexibility, and support for workarounds. So it makes sense to just allow vars in at-rule statements like any other value, especially since it exists in @media already.

@matthew-dean
Copy link
Member

So it makes sense to just allow vars in at-rule statements like any other value, especially since it exists in @media already.

Which, 🤔 when I phrase it that way, kind of implicitly DOES make it sound like a feature request. So I withdraw saying it's a bug, as it was designed that way intentionally. I would support it as a natural evolution of that feature though.

@seven-phases-max
Copy link
Member

seven-phases-max commented Apr 30, 2017

But what the ~"" syntax allows in Less is for developers to essentially "fill-in" whatever parsing / styling feature may not be directly supported in Less, and just output a string as-is

Yes, and this essentially brings us to #1648 (comment). So I'm just asking again maybe it's time to stop abusing variables as macros and bring macros in. The problem is that with allowing a more relaxed "where vars can be used w/o any syntax check" thing we just open the box of "uh-oh, I misprinted some ; - shouldn't compiler give me a error?"...
While with macros (while they don't have to perform any syntax checking at all) we still can validate things where they're supposed to be valid at least:

@min768: pi();

// OK
@media @min768 {
    .element {font-size: 1.2rem}
}

// ...
#define min333 pi() // <- whatever syntax

// error - not valid media query:
@media min333 {
    .element {font-size: 1.2rem}
}

While escaping thing (e.g. @var: ~"some non-parse-able mess";) remains untouched, keeps to do what it does now and is not involved where it should not be involved.

@matthew-dean
Copy link
Member

Oh okay, the linked comment illuminates your thinking a little better. I'm only familiar with macros in Microsoft Windows & Office programming, which is recording and replaying a sequence of actions, and not just used for text replacement. (Is that a convention of text-based editors?)

Because I'm not familiar with whatever usage you might be familiar with, I don't really understand what your example is doing. So because I don't understand your example, and the example doesn't use actual media queries (as far as I can tell?), I'm not sure why macros are advantageous as you understand them.

I think one thing we can agree with: we both hate this as typical syntax @tablet: ~"(min-width: 768px)"; I may prefer it to wrapping in mixins, but I don't disagree that it's a hack for media queries.

So I get where you're coming from, basically: "If it's a hack, why expand usage of the hack?"

And my position is essentially, "If this hack is going to continue to be used, it should at the least be consistent."

So, if this hack is going to go away, then it probably needs a proper feature proposal. Otherwise (and maybe even if that feature will happen at some point in the future), it seems reasonable to extend to @supports and other at-rules in the short-term.

@seven-phases-max
Copy link
Member

seven-phases-max commented Apr 30, 2017

I don't really understand what your example is doing.

In above example (here I'm obviously talking about the simplest 'macro'-variants like those in C or PHP), we define a macro min333 with value of pi() so that the parser (before its later stages - see below) simply replaces any occurrence of min333 with pi() (regardless of context, scope - anything - since nothing actually exists yet). So when the media statement is actually parsed the source code text coming in is already @media pi() { ....

Though it might be a relatively tricky to implement within Less parser since it does not have a separate tokenizer stage - where a macro substitution could be easily injected.

@matthew-dean
Copy link
Member

matthew-dean commented May 1, 2017

Hmm.... so if I understand correctly, the difference between ~"" and a macro is that a macro replaces the text, but after the replacement, it has to be a valid statement? If so, I get why that would be preferable.

I don't think that's a tricky as you might think, as long as:

  1. Macro references had a very distinct signature for the parser (couldn't be interpreted as any thing else)
  2. Unlike vars, macro definitions had to appear before they are referenced, and wouldn't follow scoping rules (or were simply required to be a child of the root node).

I don't think the macro reference could be a naked keyword for the Less parser, but if it was a special character that couldn't be mistaken for anything else, then you could easily add to a macro dictionary and quickly swap out values while parsing.

@define tablet "(min-width: 768px)";
@define mobile "(min-width: 640px";   // note missing parentheses
.box {
  @media ~tablet~ { 
    // ...
  }
  // could easily throw immediate parsing error
  @media ~mobile~ {
    // ...
  }
}
@define mobile "(min-width: 440px)";  // this does nothing, since it's not like a variable

Wrapping the macros / text snippets in quotes would allow for things like including semi-colons in your macro, since we already support quote escapes (and don't support semi-colon escapes). And using something like ~ might tie it to the idea of removing the quotes.

Of course, ~ would prevent easy detection and use of macros in selectors. It would be nice if it was language-wide without being over-verbose (like ~~mobile~~).

Thanks for explaining a bit more. Is my understanding of macros basically correct?

@matthew-dean
Copy link
Member

matthew-dean commented May 1, 2017

I suppose something like this would be not too challenging, parser-wise, if we added a second tilde ~ on only one side:

@define selectors ".title, #main";
.box {
  .other-selector, ~~selectors, .and-one-more  {
    // easy detection of unique characters, and quick exit (lookup) at first non-alpha character
  }

@matthew-dean
Copy link
Member

matthew-dean commented May 1, 2017

By the way, at-rule-based defines or even #define is a bit Sass-like. A more Less-like solution would probably involve the basic : assignment operator, such as:

~selectors: ".title, #main";

.box {
  .other-selector, ~~selectors, .and-one-more  {
    // easy detection of unique characters, and quick exit (lookup) at first non-alpha character
  }
}
~tablet: "@media (min-width: 768px)";

.box-2 {
  color: red;
  ~~tablet {
    // just spit-balling, there's a lot of possibility here
  }
  ~~tablet and (max-width: 1024px) {
    // okay now I really like this macro idea
  }
}

Or something like that. It's still quick to parse (faster than using @define or #define), and it's a little like CSS --variable syntax (kinda).

@seven-phases-max
Copy link
Member

seven-phases-max commented May 1, 2017

~tablet

I'd rather prefer a substituted macro identifier to may have either form. E.g. "compatible" ~ident and "intrusive" ident (or even whatever distinct piece of text like .foo#bar@{var}) so that the latter can be used to hack any code w/o modifying it - e.g. as in %define true false // happy debugging!. Otherwise the efforts implementing it may not be really worth. Hmm, but I guess I need to provide a little more justification behind that. Honestly I did not realize that choosing even the simplest form may have so many details in realization so I need some time to make my mind up (to express some reasonable vision).

But in general, yes, I assumed something like you wrote above...

@seven-phases-max
Copy link
Member

(to be moved to a separate ticket or even meta before it goes too deep/out-of-the-FR-scope?)

@matthew-dean
Copy link
Member

I'd rather prefer a substituted macro identifier to may have either form. E.g. "compatible" ~ident and "intrusive" ident (or even whatever distinct piece of text like .foo#bar@{var}) so that the latter can be used to hack any code w/o modifying it

The problem is that it's a much more difficult problem / rewrite for the parser AFAIK (and also- is it not a more complex feature than a well-defined and limited system?) If you're just going to search and replace anything with anything, you might as well be using Gulp or PostCSS. I'm not sure why Less would need to be involved at that point. It's too generic. And I would probably vehemently oppose a "feature" that would instantly make a .less file potentially meaningless and unmaintainable, where a developer on a team couldn't trust that identifiers and keywords were actually what they said they were. If someone wants to hack their code, there are plenty of tools to do so, but it seems outside the scope of Less, and outside the scope of just solving the particular problem of variable media queries and @supports.

Honestly I did not realize that choosing even the simplest form may have so many details in realization so I need some time to make my mind up

I think you're onto the right idea, though. So yes, when you have a complete proposal in meta and there's consensus, then yes, this can probably be closed in favor of that. But if that does not happen, then it seems reasonable to extend the @media "hack" to @supports.

@rjgotten
Copy link
Contributor

rjgotten commented May 8, 2017

Some food for thought regarding the 'stone age media hack':

#media {
  @phone   : { min-width : 0      }
  @tablet  : { min-width : 768px  }
  @desktop : { min-width : 1024px }
}

@media (#media[@tablet]()) { 
  ...
}

Wouldn't it be nice if that 'just worked' ™ ?
(Yes. Yes it would...)

@seven-phases-max
Copy link
Member

seven-phases-max commented May 8, 2017

Wouldn't it be nice if that 'just worked' ™ ?

What is the point of having @media () garbage there?
Why can't just:

±tablet {

}

be the proper code?
(obviously for now it's something like:

.media(tablet, {

});

) <- that's food for thought (still comparing count of parens vs. @media (#media[@tablet]()))

Personally I just can't see how min-width: 1024px may become a valid variable value w/o breaking strict CSS values validation.
Escaping (min-width: 1024px) is never a problem, but is it really the problem for such use-case? We already have "non-text-replacing" macros - they are called mixins, and imho, fear/misunderstanding of that language feature is the real problem here (thus more friendly documentation, more friendly usage for them is an important part of the proper solution).

(No, I'm not proposing a hack similar to .for I just keep wondering how variables are kept to be tried to be used everywhere even if it goes more and more ugly just like with ~"@{foo-@{bar-@{baz}}}" thing).

@rjgotten
Copy link
Contributor

rjgotten commented May 8, 2017

What is the point of having @media () garbage there?

I kind of like the explicitness of it in showing to readers that they're unambiguously dealing with a media query. But I agree that the amount of braces required in (#media[@tablet]()) takes away from it.

@matthew-dean
Copy link
Member

Wouldn't it be nice if that 'just worked' ™ ?

Errrr I get that we might support that output at some point, but that doesn't seem like the ideal solution to this problem. I'm kinda with @seven-phases-max on this one.

@rjgotten
Copy link
Contributor

rjgotten commented Jun 23, 2017

The real deal is not the property lookup; the real deal is expanding a detached ruleset inside the braced section of a media query.

You could continue to write the media query without the fancy lookup stuff, e.g.

@media (@tablet) {
 // ...
}

But you'd be able to define the variable in question without resorting to escaped literals, e.g.

@tablet: { min-width: 768px }

And as a result the compiler keeps a firm hand on syntax validation and can bail on failure. That way you won't have to implement C-like pre-processor macros, which fly sort of in the face of the lazy evaluation in Less, imho.


Could even have it handle and combinators by expanding multiple properties in one DR to them. And handle or / , combinators by expanding lists of DRs. E.g.

@tablet-range : { min-width: 768px; max-width: 1024px }

@media (@tablet-range) {
  // ...
}

To produce

@media (min-width: 768px) and (max-width: 1024px) {
  // ...
}}

And ofcourse; if you expand a DR, that means inside the DR, you can continue to use variables, mixins, etc. at will, should you really have a need to do truly complicated @media work. Can't really think of any immediate use-cases, but the support for it would atleast be there.

@stale
Copy link

stale bot commented Nov 14, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 14, 2017
@stale stale bot closed this as completed Nov 28, 2017
@falkenhawk
Copy link

cool. the issue is closed now so we can forget about it

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