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

add the any and deep keywords for extending, see issue #2101 #2506

Closed
wants to merge 1 commit into from

Conversation

bassjobsen
Copy link
Contributor

No description provided.

@seven-phases-max
Copy link
Member

I'm not 100% sure but it looks like any implementation differs from what specified in #2101.
My understanding is that according to

Match partial selector ( :extend(.button any) matches .button {} and #main .button a )

this:

.grand {
    grand: rules;

    .mother {
        parent: rules;

        .child {
            child: rules;
        }
    }
}

.father:extend(.mother any) {}

should result in:

.grand {
    grand: rules;
}

.grand .mother,
.father {
    parent: rules;
}

.grand .mother .child,
.father {
    child: rules;
}

I.e. any is not a sort of parent/child reversed deep but literally "extend a selector it matches as a whole".

P.S. Although rereading it once more it looks like my interpretation also conflict with

all - set "any" and "deep" to true.
But that statement itself conflicts with

(.button any) matches #main .button a

if any = all - deep it can't match #main .button a (and it can't be named any then) :) So some drift is expectable.

@matthew-dean
Copy link
Member

@seven-phases-max In short, any was intended to match selectors the same way all is matching selectors, but should output tree depth the same way that no keyword would output.

Your output demonstrates a flaw in the way the keyword is described, because while it doesn't technically extend the nesting under .mother, it does in actuality, because it represents a partial match on the next selector. So, if it behaved exactly as written, it wouldn't be a "shallow" nesting, because of the greedy matching on all child trees.

The reason for this discrepancy I think is in some of the disagreement of what and where a selector was being matched. So the write-up for any probably reflects the view that .less selectors would be matched and not the "evaluated" CSS selector.

So that said, the intended output is probably more like this.

.grand {
    grand: rules;
}

.grand .mother,
.grand .father {
    parent: rules;
}

.grand .mother .child {
    child: rules;
}

It would have effectively matched the middle segment, but not continued to match the descendants. And because it extends .mother as a child of .grand, then it would would also have .grand prepended.

I had another look at the PR, and as far as I can tell, @bassjobsen's interpretation matches the (granted, very ambiguous) spec. More importantly, it does selector matching like all, and tree depth like exact. That's the intent of the keyword.

The output for deep also looked correct to me, although I just looked at the variants for the .grand / .mother .child tree. In the PR, .father correctly does not match, and .father2 does an exact match to the middle of the tree, and stops there.

Correct me if I'm wrong in how :extend behaves now (which is important), but from writing up the original discussion, it looks right as far as that discussion went.

@seven-phases-max
Copy link
Member

any - OK, this makes sense then except I can't now see how extend a selector with its ascenders translates to any semantically, but never mind, it's not that critical :)


deep- Yes, it looks all correct for me.

@matthew-dean
Copy link
Member

@seven-phases-max Do you think there's a better keyword? Or a better way to describe it?

@seven-phases-max
Copy link
Member

lofty? (OK, just joking... Honestly I don't know, actually even deep would be quite confusing for me in this context if it would already not be used for similar concepts (e.g. CSS /deep/ combinator etc.). Hmm, in sketching mode: maybe asc/desc, sup/sub, upper/lower, outer/inner, Λ/V for both).

@bassjobsen
Copy link
Contributor Author

Or call them according their implementation, before (any) and after (deep). Then not any or deep is the default and both any and deep = all.

Then we also have the exclamation point or Capitalizing of the keyword. Personally i think exclamation point is the most clear solution.

@matthew-dean
Copy link
Member

Before and after are way too easily confused with the CSS pseudo elements. And I don't see how it conceptually matches what's happening. So that doesn't seem like a better choice.

Then we also have the exclamation point or Capitalizing of the keyword. Personally i think exclamation point is the most clear solution.

I agree, and it's the more "CSS-y" of the two. CSS as a rule doesn't use capitalization at all, and many CSS authors are allergic to it, so it's almost an "anti-CSS" pattern. Whereas CSS has exactly one flag syntax, which is the exclamation point.

That said, I don't see any reason why keywords can't be case insensitive, AND have the optional flag syntax. Best of both worlds?

@lukeapage
Copy link
Member

I don't see any reason why keywords can't be case insensitive, AND have the optional flag syntax. Best of both worlds?

(Y)

I'll let people decide on names before I merge this. I was surprised the implementation is that simple - I've forgotten alot of it by now though - possibly I didn't add the other 2 keywords because no-one could agree.. or maybe it was something else that was missing :S I hope not.

@matthew-dean
Copy link
Member

Oops, I missed @seven-phases-max's suggestions when responding, so I was only responding to @bassjobsen.

@seven-phases-max I don't think they would necessarily go in pairs like that, as the other two modes are not exactly opposites. I mean, they switch on opposite switches, but the result isn't exactly a mirror of the other, so to pair them might be confusing in a different way.

The logic of the keywords and their naming had to do with the switches. If you think about all you have to consider that it's signifying "more than one feature/switch turning on". The other keywords, then, identified each of those features individually.

any referred to selector matching. Instead of needing to match the selector in its entirety, you could do a "partial" match, so any nodes of the tree, or any parts of the selector could match.

deep refers to the depth of nesting that's extended. With it off, it's a shallow match of extending just that specific block. On, and it extends all the declarations in that block and as far deep as that tree goes.

So any and deep have nothing to do with each other and shouldn't be named like they do. They're individual features. They both relate to all (or all refers to both, is probably the better way to say it). all enables both partial matching and deep extending.

I think that because there's been only one keyword, Less authors have probably associated "all" to mean "all matches". But that's really any that does that. So, all actually isn't defining a type of extending at all, but simply means "all features" or "all keywords". But because we haven't HAD other features, the association has probably been a bit different in people's minds.

@matthew-dean
Copy link
Member

Another way to say that is that these two lines are equivalent (although multiple keywords are not currently supported):

.a:extend(.b deep any) {}     // or .a:extend(.b !DEEP !ANY)
.a:extend(.b all) {}          // or .a:extend(.b !all)

Just to clarify this piece:

if any = all - deep it can't match #main .button a (and it can't be named any then) :) So some drift is expectable.

Is that still confusing? It matches the middle of the selector. That's why in the PR, any is able to match in the middle of the tree without including the parent selector in the extend statement. It just doesn't "continue" to extend the depth of the rest of the tree. So yes, it would match #main .button a because all does. It's the same selector matching behavior, just not the same depth extending behavior.

@seven-phases-max
Copy link
Member

So yes, it would match #main .button a...

It matches the middle of the selector. That's why in the PR, any is able to match in the middle of the tree...

I guess this is where the miscomprehension sits. Since a tree and a selector are not the same things you can't interchange them freely. E.g. the

#main {
   .button {
     a {}
  }
}

tree creates three independent selectors having their own sets of rules, so you can (probably) say it matches in the middle of the #main .button a tree but you can't really say it matches in the middle of the #main .button a selector. Obviously #main .button (of the same tree) is a selector on its own and it's not a "middle" of the #main .button a selector. (Thus was the point of my confusion, since #main .button a usually means selector (not a tree), the fact "it matches" misdirected me (specifically because "matches" -> extends)).


Speaking of any name. Let's try it in other way maybe. What would be those "one ot two sentences" to describe any briefly in the docs? Maybe something that really implies "any" makes me stop to complain? :)


Speaking of the docs, curiously I just realized that the following dumb and mechanical descriptions of all three are sufficient and exhaustive:
xyz all - extends any selector containing xyz element.
xyz deep - extends any selector where xyz is the last element.
xyz any - extends any selector where xyz is the first element.
(This is definitely not what I'd like to see in the docs but probably it explains why I don't either like deep and any too much).

@matthew-dean
Copy link
Member

Since a tree and a selector are not the same things you can't interchange them freely.

I agree. I was more trying to reference how extend was implemented. As I said, how the particular descriptions of keywords were written is not exactly how it was implemented. In the implementation process, it was decided that :extend(#button a) would match #button a{} and #button { a {} }

So, how it should be described or how keywords should be labeled now is a legitimate question / point. The final implementation (of all) is not logical to me, so I'm not a good judge of how to word that. However, knowing how it works is sufficient to use it, so I have no qualms with that; I just don't know how to adjust the wording, because it seems like extend matches pieces of a tree instead of a selector. So using "selector" and "tree" in the context of the other keywords is probably not exactly correct.

xyz all - extends any selector containing xyz element.
xyz deep - extends any selector where xyz is the last element.
xyz any - extends any selector where xyz is the first element.

I had to re-read these descriptions a few times before I got what you were saying. While they seem cleverly technically accurate (aside from xyz not necessarily being an element), they still kind of read as "huh?" to me.

The disparity in understanding is not new in regards to this feature. Extend, unlike any other feature, exposed the different "mind models" that Less authors use to approach their style sheets. Everyone was writing the same thing, but held a different model of what that "thing" was, and so when extend was introduced, we suddenly realized that we had different ideas and different internal models about the thing.

So, I know how extend works, but despite the descriptions and people using it, I don't know WHY it works that way. That is, I can't grok the reasoning because I don't think along the same path. However, I know that a lot of people understand why it works that way and agree with it, and that's fine.

That's a bit off-topic, except to say that I understand why we're each confused by the other person's description. So this is the labeling and description challenge. It's possible each of us won't understand the other person's reasoning, so I think the best middle ground is to find keywords / wording that sufficiently express our understanding.

That is: let's keep tossing ideas back and forth until something sticks.

@seven-phases-max
Copy link
Member

More sketching: upward/downward.

@matthew-dean
Copy link
Member

I get downward and the other words you've used, but I don't get the upward part, which is similar to your other key pairs. How does it extend upward? (Or "outer" or "asc".) I don't recognize that behavior in this feature. Do you have a code example that could demonstrate how it is extending "upward"?

@seven-phases-max
Copy link
Member

@matthew-dean

I'm just sketching. OK, maybe upward / downward is a bad exampe. But forouter/inner it could be something like:

extend(x any) extends any x with whatever outer stuff (e.g. a x, b x etc.)

since in:

a {
  x {
   color: red;
  }
}

a is an "outer" one of x
And exactly in opposite for deep.

@matthew-dean
Copy link
Member

Hmm, but it isn't exactly outer because of this:

LESS:

.a button .q {
    color: red;
}
.b:extend(button any) {
    background: black;
}

CSS:

.a button .q,
.a .b .q {
  color: red;
}
.b {
  background: black;
}

I don't see how that matching is described as "outer". If anything it's "middle". Or end or beginning. So... any. Default extend is "exact" matching, and any/all match any part of the selector or token or tree segment, however you want to think about it. But I don't see how it's semantically "outer".

@matthew-dean
Copy link
Member

Another example of any. (For demonstration, these are the same as all because there are no nested blocks.)

#mike.bob.joe.bill {
    color: red;
}
.b:extend(#mike all) {
    background: black;
}
.c:extend(.bill all) {
    background: blue;
}
#mike.bob.joe.bill,
.b.bob.joe.bill,
#mike.bob.joe.c {
  color: red;
}
.b {
  background: black;
}
.c {
  background: blue;
}

@matthew-dean
Copy link
Member

Demonstration of deep:

LESS:

.element {
    border-color: green;
    &:hover { color: blue; }
    &:active { color: red; }
}
.foo:extend(.element) {}
.bar:extend(.element deep) {}

CSS:

.element,
.foo,
.bar {
  border-color: green;
}
.element:hover,
.bar:hover {
  color: blue;
}
.element:active,
.bar:active {
  color: red;
}

@seven-phases-max
Copy link
Member

For your first example, no, the result of this PR is different:

// warning: extend ' button' has no matches
.a button .q {
  color: red;
}
.b {
  background: black;
}

So it looks like we are getting back to the top of the thread? :)
Look at the example in #2506 (comment): this is how this PR works and it's the same example as this .a button .q (Unless you mean that selectors made by nesting should differ from plain selectors, but this is not how extend algorithm works).


P.S. So let's try again... can you, please, provide an example that shows the difference between any and all?

@matthew-dean
Copy link
Member

For your first example, no, the result of this PR is different:

Er, then that seems wrong. Since "all" is the "sum" of deep and any, then it should behave interchangeably with all depending on the structure (in theory). So, if you notice, I did a "flat" tree with partial matching, in which case "all" and "any" output should match. Then I did a nested tree with exact matching, in which case "all" and "deep" output should match. The new "cases" would be a partial match without extending nested blocks, and exact matches that extend nested blocks. Outside of those cases, the output should theoretically match.

How do you test the PR? Is there a way to do this with npm?

@matthew-dean
Copy link
Member

Not only would that be wrong, but it wouldn't pass the example given right at the top description of any, lol.

any: Match partial selector (:extend(.button any) matches .button {} and #main .button a)

Err, @bassjobsen ?

@seven-phases-max
Copy link
Member

Is there a way to do this with npm?

I'm was under expression you can install via npm directly from github. Though to be honest I'm not sure now (personally I just checkout the branch via git).

@lukeapage
Copy link
Member

lukeapage commented Mar 21, 2015 via email

@matthew-dean
Copy link
Member

@lukeapage Oh, right. I saw something similar when you edit an npm module, and you can set the version to the tarball URL from Github in your package dependencies. Thanks! That makes life easier.

@lukeapage
Copy link
Member

Please re-open when/if this is ready

@lukeapage lukeapage closed this Jun 11, 2015
@matthew-dean
Copy link
Member

@bassjobsen Pinging again to see if you want to address the remaining issues.

@bassjobsen
Copy link
Contributor Author

@matthew-dean i will try soon

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

Successfully merging this pull request may close these issues.

4 participants