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

Variable/List merging/concat functions and language features #2564

Closed
kevinmartin opened this issue Apr 22, 2015 · 12 comments
Closed

Variable/List merging/concat functions and language features #2564

kevinmartin opened this issue Apr 22, 2015 · 12 comments

Comments

@kevinmartin
Copy link

I was looking through the feature documentation, and I noticed we can easily merge properties, but not variables.

I noticed #2467, but I felt like this should be part of the core language. Some examples:

// Example 1 - Sometimes, you just want it to be more reader friendly (especially if you have long lists):
@comma: a b c;
@comma+: d e f;

@space: a b c;
@space+_: d e f;

// Suggested Parser Interpretation:
@comma: a b c, d e f; // A parent list that contains two child lists with 3 items in each.
@space: a b c d e f; // One super list with 6 items within it.
// Example 2 - Sometimes, you have special conditions:
@base: g h i;

& when (@condition = true) {
    @base+: j k l; // There might be a scoping issue here?
}

.special-mixin();
.special-mixin() {
    @base+: m n o;
}

// Suggested Parser Interpretation:
@base: g h i, j k l, m n o;
// Example 3 - As functions (suggested as a plugin in #2467; maybe naming could be different?)
@first: a b c;
@second: d, e, f;

@merged: merge(@first, @second);
@concated: concat(@first, @second);

// Suggested Parser Interpretation:
@merged: a b c, d e f; // A parent list that contains two child lists with 3 items in each.
@concated: a b c d e f; // One super list with 6 items within it.

While creating my own mixin to simulate Example 3, I found a kind of discrepancy...

// LESS
.merge(@rest...) {
  @joined: @rest;
}

.test1 {
  @first: a b c;
  @second: d, e, f;
  .merge(@first, @second);

  firstLength: length(@first);
  secondLength: length(@second);
  joinedLength: length(@joined);
  joinedList: @joined;
}

.test2 {
  @first: a b c;
  @second: d e f;
  .merge(@first, @second);

  firstLength: length(@first);
  secondLength: length(@second);
  joinedLength: length(@joined);
  joinedList: @joined;
}

// Expected Output
.test1 {
  firstLength: 3;
  secondLength: 3;
  joinedLength: 2;
  joinedList: a b c, d e f;
}
.test2 {
  firstLength: 3;
  secondLength: 3;
  joinedLength: 2;
  joinedList: a b c, d e f;
}

// Actual Output
.test1 {
  firstLength: 3;
  secondLength: 3;
  joinedLength: 2;
  joinedList: a b c d, e, f;
}
.test2 {
  firstLength: 3;
  secondLength: 3;
  joinedLength: 2;
  joinedList: a b c d e f;
}

The suggested core language improvements and functions could definitely help clarify/fix/prevent the above issue and allow the Examples to executed.

@kevinmartin kevinmartin changed the title Variable/List merging/concat functions Variable/List merging/concat functions and language features Apr 22, 2015
@seven-phases-max
Copy link
Member

@comma+: d e f;

This is simply impossible because of declarative nature of the language. Less variables are actually immutable and stateless (i.e. "constants" in short), so (by design) you cannot modify the value of a variable but can only redefine/override it with a new variable of the same name. For the same reason you cannot do anything like @var: @var + 1; too. And for the same reason you cannot have a mixin or a built-in function that modifies a variable value (both can only produce a modified copy). (See also a notice about corresponding "property merge" syntax below).

I noticed #2467, but I felt like this should be part of the core language.

I'm afraid things like "felt" are not quite something that can become a rationale to put a feature into the core.
Beside #2467, earlier there were several suggestions to add join/concat-like functions to the core (for example #1785 (comment) + a few more), but overall community demand for this stuff seems too be pretty negligible (yet?). For example compare #2467/#1857 to #1075 or #1177 to actually "feel" the difference ;).
So in context of "community demand" it would be much more helpful to just stamp +1 in #2467 (and purpose your syntax idea there) instead of creating a new sort of duplicated ticket.


Either way for list/array manipulation functions see less-plugin-lists.
Also see related #1857 (same hint: simply put your +1 there).


Regarding of the property merge feature, strictly speaking its syntax is a direct violation of CSS and Less "declarativness", i.e. the property merge statements are clearly imperative. Fortunately this is not a critical defect (yet) because CSS properties are currently "write-only" and every non-duplicated property/value pair should appear in the CSS output (yet another help there is that `property: value` and `property+: value` statements create two independent property "layers"). But for sure this will become a problem when/if we decide to implement #2433 or so.

@seven-phases-max
Copy link
Member

I'm not closing this ticket yet (just in case anyone wants to express an opinion on the particular syntax).

But honestly, aside from the special syntax, this is really no more than just a duplicate of #2467 and if you believe there's some reason for such feature to be implemented via some dedicated syntax rather than just through trivial functions, please express your ideas in #2467 (closed issues/feature-requests can be reopened at any moment).

And in general the current approach is:

if some feature is uncertain to be really something must-have in the language core it's better to go as a built-in or even a plugin function.

@matthew-dean
Copy link
Member

@seven-phases-max How would that reflect on #2433? That is, if we could treat properties like variables, does that mean it would conflict with property merging? It seems not quite right to say it's impossible, but just that it's difficult with the current code?

@seven-phases-max
Copy link
Member

@matthew-dean

It seems not quite right to say it's impossible, but just that it's difficult with the current code?

I would not say it's imposible or even difficult to allow/implement at all. It will just create a weird mix of imperative and declarative stuff that pretty hard to follow with LL and LDW in mind (i.e. "a language defect"), e.g.:

// less:
div {
    p+: 1;
    p : 2;
    p+: 3;
    x : property(p);
}

// -> css:
div {
    p: 1, 3;
    p: 2;
    x: ?; // p is 2 at this point, but did not we append 3 in the end?
}

In other words p: and p+: in fact create two different properties (they only become the same/cascaded in a browser).
It could be considered like nothing bad at all (we can always declare "don't use merging if you read properties, the result is undefined/unspecified" :). But this certainly is not something to model normal Less variable lists concatenation after.

@matthew-dean
Copy link
Member

Actually, your example seems straightforward to me. Less specifies that properties are merged only if both are marked as mergeable. I don't know what current behavior is, but the following would be my expectation:

// less:
div {
    p+: 1;  // p = 1, but is "marked" mergeable
    p : 2;   // p = 2. it is not marked mergeable. since it is a later declaration in the scope, final value is 2
    p+: 3;  // p = 3, and is marked mergeable, however the only other mergeable declaration has been overwritten in scope by a subsequent non-mergeable declaration, so nothing is merged
    x : property(p);  // p is 3
}

If the output is as you just specified (I just checked, and it is, yikes), this is an error in implementation. The +: syntax marks mergeability, but that cannot take precedence over the cascade (last declaration wins). If Less is simply looking for mergeable properties, and merging them wherever they occur in scope, that's incorrect because it changes declaration order. We can't do that for the same reason we don't do @media block merging.

That is, the last declaration of p+: 3 cannot move. It is still a declaration of a value of 3 at that position in the rules. Because of the + it can "consume" preceding mergeable declarations, but it can't be moved to previous values. That's an error. If no previous mergeables existed, the final value would not move its position, as in:

// less
div {
    p : 2;
    p+: 3;
}

// css
div {
  p: 2;
  p: 3;
}

Now I would consider an argument that the final value could be 1, 3, but that would seem to violate the intent of merging, which is to only produce that behavior when all property declarations are marked as such. But, you could say, that declaratively, merging happens for the last value because of previous mergeables in-scope. So, maybe it's more declarative to have it as 1, 3, I don't know. But, however you slice it, a final value of 2 is wrong, and you note yourself: didn't we append 3 at the end? A value of 2 is a serious bug.

The other Less quirk that makes this issue weird is that Less leaves conflicting declarations in final rules for no understandable reason. (corrected by @seven-phases-max - the reason would be only one rule may be valid for X browser.)

That is:

// css
div {
  p: 2;
  p: 3;
}

Because of CSS, the value of p is always 3. To include previous property declarations is weird. One might say it's there because it existed in the source Less, but that seems overly dogmatic. In reality, a property only ends up with one value, just like a variable.

As to this:

Regarding of the property merge feature, strictly speaking its syntax is a direct violation of CSS and Less "declarativeness", i.e. the property merge statements are clearly imperative.

Maybe. There isn't 100% consensus on the categories or differences between those two terms. But, yes, in general I think we try to implement things based on "traditional declarativeness". In this case, however, pragmatism wins. The nature of CSS (mostly a problem of CSS3), combined with how Less defines mixins, means that authors inevitably have encountered situations of wanting to declare "optional partial overwrites" of declarations, since CSS3 defines some "shorthand" syntax for which there is no way to separate into individual longhand declarations, which breaks with the CSS 2.1 model. Merging is a legitimate practical solution and meets Less's core directive, which is to make writing and managing CSS easier. Less's core directive is not to make sure all features pass a declarative test, even though it's important to do as much as possible for consistency.

As far as this feature, if we addressed that bug and quirk of Less regarding properties, then this request in relation to variables would, I think, make more sense. Meaning, it would make it clear that properties end up with only one value, (and don't move), so incorporating variables into similar declarations wouldn't seem as far-fetched.

@matthew-dean
Copy link
Member

Yes, thinking about it more, the final value should probably be 1, 3. The way these declarations should probably be thought about is like this.

// less:
div {
    p+: 1;
    p : 2;
    p+: 3;
}

// -> css (not desired output, but effectively)
div {
    p: 1;
    p: 2;
    p: 1,3;
}
// -> css (ideal output)
div {
    p: 1,3;
}

@seven-phases-max
Copy link
Member

The other Less quirk that makes this issue weird is that Less leaves conflicting declarations in final rules for no understandable reason.

There's reason to leave non identical property values in the output:

div {
   background: rgb(200, 54, 54); /* Hello IE6 etc. */
   background: rgba(200, 54, 54, 0.5); 
}

@kevinmartin
Copy link
Author

Hi @seven-phases-max,

Thanks for pointing out less-plugin-lists - It can definitely be useful for personal development, but not when you are creating a LESS framework. You would need to tell your users to install the plugin and make sure to use it. This would (for example) make Bootstrap harder to use if they required any of those functions.

As far as #2467, I apologize for it looking like a duplicate issue. I actually didn't notice the proposed +: syntax at the very end of the issue until now.


This is simply impossible because of declarative nature of the language. Less variables are actually immutable and stateless (i.e. "constants" in short), so (by design) you cannot modify the value of a variable but can only redefine/override it with a new variable of the same name. For the same reason you cannot do anything like @var: @var + 1; too.

Two comments:

  1. Properties are also supposed to be immutable, yet you were able to find a way to support property merging...
  2. If you can't "modify" an existing variable, but only redefine/override it, why wouldn't it be possible for the +: syntax to do that. That syntax wouldn't be modifying the variable, but redefining it with new values?

I understand that you're trying to create a plugin system, but that is only going to make it more difficult for open-source project owners. Their downloaders have to download an extra (external) plugin and figure out how to implement it (in the case of grunt/gulp/etc).

@seven-phases-max
Copy link
Member

(upd., oops, sorry I always forget to mention a recipient) @kevinmartin

This would (for example) make Bootstrap harder to use if they required any of those functions.

They do require Autoprefixer now, so bootstrap.css compiled with "just pure Less" is no longer valid.
I.e. the build-tools approach is changing for modern frameworks... Sooner or later any descent framework will require something beyond just "pure Less". After all, many already require the lessc/less.js (and derived tools) only, since most today Less features simply won't work in any ports. And when a new feature is added you will need to tell your users to install/update to a certain Less version anyway so "You would need to tell your users to install the plugin and make sure to use it. " is not such a good excuse these days. (In short: "advanced Less framework -> predefined/recommended build tool chain").


  1. Properties are also supposed to be immutable, yet you were able to find a way to support property merging...

Properties are evaluated by a browser (so Less does not care of any cascading in this regard except certain specific cases) and the CSS has no scope. E.g. taking you first basic example:

@base: g h i;

.special-mixin();
.special-mixin() {
    @base+: m n o;
}

It won't work even w/o +: as you probably expect it to, because even if inside .special-mixin the @base is m n o, the value outside the mixin remains to be g h i. In case of CSS properties that will result in two different properties of the same name but with different values. But you can't have two different values for the same variable at the same time in the same scope - that's the key difference.
I suspect before we could get into further details (yes, there're more) I would suggest to start from some basic principles of Less variables... Hmm, probably the links there will be a good starting point.

  1. If you can't "modify" an existing variable, but only redefine/override it, why wouldn't it be possible for the +: syntax to do that. That syntax wouldn't be modifying the variable, but redefining it with new values?

Because this will mean either an infinite recursion (when LDW and LL apply) or break existing behaviour (if LDW is disabled for such syntax). E.g.:

@v: 0;

div-0 {
    something: @v;
}

div-1 {
    @v+: 1;
    something: @v;
}

@v+: 2;

div-2 {
    something: @v;
}

@v: boo!;

What result would you expect for such code?

@matthew-dean
Copy link
Member

There's reason to leave non identical property values in the output:

Agh, damn it, of course. That said, the output of the property merge still seems a plainly obvious bug in that the merged result position is at the first property position and not the last property position, but of course that's tangential to this issue.

As to this issue, you're right about imported variable scope and recursion in your example. While properties could possibly be referenced like variables, they still would not behave like variables in the sense that they are not inherited into the scope (which is an important distinction to make on #2433). It didn't compute that both could be referenced, but property combinators wouldn't be possible for both (which is why I brought up property referencing since the distinction between properties / variables was being asked for implicitly in this issue). Thanks for taking the time to explain.

I'm satisfied that this can be closed. You're right that variables have a certain built-in behavior of "replacement in scope", whereas properties "output as is" and even duplicate. So merging of vars would have unexpected behavior based on design. Let's close.

@rjgotten
Copy link
Contributor

@kevinmartin :
It can definitely be useful for personal development, but not when you are creating a LESS framework. You would need to tell your users to install the plugin and make sure to use it. This would (for example) make Bootstrap harder to use if they required any of those functions.

No, you don't. Less 2.5.0 supports loading additional functions inline via the new @plugin keyword.
Functions loaded inside a namespace, ruleset or mixin are locally scoped to said namespace, ruleset or mixin to ensure they don't bleed into other code. This is the perfect kind of encapsulation for large styling frameworks based on Less.

The format in which such a plugin has to be written is slightly different though, meaning you may end up having to spend a bit of time refactoring a plugin written for commandline consumption.

(Full disclosure: I actually wrote the @plugin support. And for exactly these reasons...)

@lukeapage
Copy link
Member

closing due to lack of support (and I agree)

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