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

& combinator inside an escaped string selector #864

Closed
borodean opened this issue Jul 7, 2012 · 21 comments
Closed

& combinator inside an escaped string selector #864

borodean opened this issue Jul 7, 2012 · 21 comments

Comments

@borodean
Copy link

borodean commented Jul 7, 2012

There is no way to use both & combinator and escaped string selector this way (outputs an error):

@var: 'test';
.foo {
  &(~'@{var}') {
    color: red;
  }
}

Nor this way (outputs & as is):

@var: 'test';
.foo {
  (~'&@{var}') {
    color: red;
  }
}

Nor any other way.

@matthew-dean
Copy link
Member

What are you trying to output?

@borodean
Copy link
Author

@var: 'test';
.foo {
  &.(~'@{var}') {
    color: red
  }
}

Expected output:

.foo.test {
  color: red;
}

@matthew-dean
Copy link
Member

Confirmed. That looks like a bug.

@lukeapage
Copy link
Member

Yes, there s no way round it.. @MatthewDL @Synchro @cloudhead what do you think is the best format

&(~'.@{var}')
(&~'.@{var}')
(~'&.@{var}')

I guess the last implies that we "parse" the selector for use of & which I'm not sure I like since maybe thats why you are escaping it?

Am also thinking that if we go for (1) or (2) of supporting just & immediately before or & immediately after.. since that covers the main bases. what do you think?

@Synchro
Copy link
Member

Synchro commented Aug 17, 2012

Of those I think the first is the clearest.

@borodean
Copy link
Author

BTW, in LESS syntax, what does the brackets mean? I think it should be made clear before considering proper syntax.

@lukeapage
Copy link
Member

Brackets are already required. I assume @cloudhead added it to make it very clear its not a selector.

I prefer the first, but I can see an issue being reported next month that wants & in the middle of the interpolated string.. then we can't remove the first option and have to additionally support the third second option.. but to take devils advocate, if we do option 3 we have to parse the string as a selector basically and I don''t like that idea. So I think I agree that option 1 is best.

@matthew-dean
Copy link
Member

+1 to @Synchro . & should be on the outside, since it's not normally used within expressions. So option #1 looks correct. The others are weird.

@borodean
Copy link
Author

If we consider using & in the middle of the interpolated string and option 1 is prefferable, it may be done that way:

(~'@{var1}') & (~'@{var2}')

@borodean
Copy link
Author

Also, if for some weird reason we want to output & in selector as is:

/* CSS */
.foo&bar { ... }

We need to have an option to escape it somehow. Option 1 solves this clearly:

/* LESS */
.parent {
  .foo&bar { ... }      // -> .foo.parentbar
  .foo(~'&')bar { ... } // -> .foo&bar
}

@lukeapage
Copy link
Member

@MatthewDL @Synchro @dmcass Can you take a look at the pull request and see if you agree with implementation.

@borodean the syntax in your last comment will not be supported - for reasons explained in the source of the pull request.

@matthew-dean
Copy link
Member

The results look good, but the test cases are strange, since no one would do this:

.a {
  (~".b") {
    color: red;
  }
}

I don't know if it was a big deal, but since this is about a syntax to allow string substitution, that might be the better example in the test. (Like @borodean's up top.) It just threw me for a sec.

@dmcass
Copy link
Contributor

dmcass commented Aug 20, 2012

Agree with @MatthewDL, I feel like the test case could add a bit more complexity. Otherwise, looks good.

@cloudhead
Copy link
Member

+1 for syntax #1 -- but be aware that string escaping in selectors like that is an undocumented feature and therefore not recommended. It's highly probable that it will change. /cc @agatronic

@cloudhead
Copy link
Member

Looking at this again and looking at the proposed implementation - I would say no.

The feature is unsupported, and to be used "as-is" - the patch couples the selector parser with this feature and increases complexity for little gain.

👎

@lukeapage
Copy link
Member

I agree with the implementation comments, the existing syntax makes it a bit hacky.

Id rather depreciate the existing syntax and allow @{var} as an element.. e.g.

&.tst@{var}. {
    color: black;
}

the existing syntax may not be documented in less documentation, but use in bootstrap and explanation on stack overflow mean lots of people are using it.

@cloudhead
Copy link
Member

Yes, let's do it that way and deprecate the (~"") syntax

@borodean
Copy link
Author

Hah. What if I want to output @{var} construction as is, with no evaluation? Consider it is some of kind of brand new css hack that may spring up some day.

@cloudhead
Copy link
Member

Well, Mick Jagger says you can't always get what you want:

http://www.youtube.com/watch?v=WbjZA3aAH3s&feature=fvst

@lukeapage
Copy link
Member

@borodean

@at: ~"@";
@var: ~"@{at}{var}";
@var {
}

I think its highly unlikely for a css construct to use that form. No-ones complained yet about being unable to make a string with "@var" in it..

@lukeapage
Copy link
Member

pulled new selector interpolation format

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

6 participants