-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Parser: Multiline strings #1648
Conversation
this would be so awesome! 👍 |
👍 |
1 similar comment
👍 |
I did not want to express my opinion on this feature since I probably am a greatest adversary of the A. So if we consider multiline escaped strings to be a valueable addition to the LESS why should it be another syntax? P.S. Just in case, personally I'd prefer such |
@seven-phases-max I fully agree The only downside I can see in just allowing multiline strings is that you are less likely to catch errors where someone misses off the closing quote. I'm trying to think why most languages ban it and I can only come up with ambiguity over whether it is \n or \r\n etc. and that you don't usually want re-formatting a code file to windows or unix to effect the string constants. |
OK, some more points of mine. But please interpret this only as a friendly discussion )
Multi-line literals are present in many good languages - C#, Python, Scala, Groovy. Only in C# is used We also face no problems with the CRLF/LF. Actually we do nothing about it - as browsers manage this well by itself, and if a consumer of the literal needs to handle it, it is free to do this. Other side - do Less preprocessor needs block literals? I came across the lack of it in keyframes declarations - nowadays keyframe animation is a usual thing, and as for now in less there is no good ways to declare them. Or do I miss something? @seven-phases-max, mixins can be good solution for this also, but when they will be availabe in Less? Will it handle also nested mixins inside keyframe step declaration?
|
You don't need to escape any parentheses inside
Well, yes, lesshat will benefit from these 'block literals' (more over, only lesshat will benefit from it - notice that you can't extract But yet again, I do not agitate against this proposal, I just concern about chosen syntax. Six quotes leaded by tilde...
Sorry but I don't think
How soon I will find this animation does not work properly with my page and how soon I will find why exactly it does not work? I.e. these solutions are barely comparable. |
Max, it is OK, all clever people understand what a good desput is, and that nobody is a bad guy in it )
You are right until there is a strict - (tree structured) - macro/block syntax inside. And when we assume, that the argument is handled by the `javascript hacks', then the restriction on a syntax could prevent from some good transformations. Autoprefixer is great, but I prefer e.g Using 'block literals' and 'javascript hacks' is surely more error prone approach then an official LESS parser, but it allows to create things, that with LESS are not possible. You can think about this technic as a low level integration into LESS Core. With it some new ideas, features, other syntax and stuff can be implemented as experimental or production ready feature. May be some widget based library can use this 'low level integration' ... brief ideas: .wdgt-carousel(~'''
background: red
images:
- 'img-1).png'
- 'img-2).png'
# some other settings
'''); You see, I used yaml syntax as an argument for the widget. And the widget itself contains of some LESS 'javascript hack' addon (that parses the arguments and generates css), and the plain javascript that generates html and handles interactivity. Your last statement is applicable to the whole javascript. Many people say - "No strict types? Really? How soon I will find an error, when something goes wrong" :) Max, but in general you are also right, so any decision upon this proposal, even negative, will not make LESS less great :) |
To be honest, both my examples were a bit unfair, the
And the typo example ignores the fact that I'll get exactly the same problem with just plain CSS misprints, e.g.:
|
@tenbits The benefits of autoprefixer aren't just that it applies prefixes, but that it only applies the necessary prefixes. Even if you're using mixins that generate prefixes, you should still be running the resulting CSS through autoprefixer to eliminate all the unnecessary prefixes that are inevitably generated. |
You're comment is a perfect example of someone who made no attempt to assist in those decisions at the time, or has made no attempt to contribute the solutions you're creating back to the community - for commercial or other reasons, it doesn't matter. So @petrbrzek what are you planning to contribute to this community help less.js achieve the things your company needs? |
Only if you use Less as a wrapper for a JavaScript autoprefixing library like the one mentioned here. |
Um... @petrbrzek I don't agree with all of Less's design decisions either, but that's why I spend my time giving input into it's future growth, which, as @jonschlinkert said, is probably a bit more constructive. If you think Less Hat "solves" architecture issues in Less, why not a) open an issue describing your real-world problem(s), b) propose solutions. For the record, I've been looking at Lesshat specifically. One of my goals is to help remove any need for Less.js to ever use JavaScript (and completely deprecate its usage). Evaluating inline JavaScript is terribly hackish, is not supported in all JS-environments, and makes .less files a mess of half-CSS-looking, half-JS-looking oatmeal. (See: Lesshat.) Ideally, we need Less library authors like you to work with the Less community, propose solutions, and make pull requests. That's why I've been pushing for things like plugin architecture, a proper Less.js API, auto-loading options, etc. All the things a library like Less hat might need to create clean code. At the same time, there are some things that Lesshat may be doing wrong in its architecture approach which you may not be aware of, so some of the hoops you've had to jump through may be as a result of Lesshat's design. Often by proposing issues / solutions, other people in the community might know of a way to meet your requirements using Less in a way that's cleaner. |
Now, to put this thread back on track, I notice that, in the example, the reason this is being proposed is not because the value is a string at all, such as something that will be evaluated as a multi-line CSS string, but basically an "escaped" string to pass in a set of keyframe values. Shouldn't we instead look at how to pass in a set of declarations like that into a mixin, unescaped? Keyframes are especially hard to solve with Less, and I've never figured out a great solution. Multi-line strings still seems to be circumventing the problem. |
Well, yeah, my thoughts exactly. with the bottom line being that if that time had been spent contributing directly to Less.js, there wouldn't have been so many hoops to jump through with LESS HAT (e.g. LESS HAT wouldn't be such a mess of bad hacks) and everyone in the community would have benefited from the effort, not just your company. |
+1. I was thinking to at least start to do something in this direction but no results so far... |
Agreed as well. this is only nomenclature, but I think this is a better use case for the term "include" than others I've seen. I also have seen use of "raw", implying that code won't be evaluated before it's "copied in". e.g. I'm sure there are other ways to approach this, but I agree this kind of feature would be useful. and in other use cases as well. |
Hmm.... So we have this problem that's essentially: I have a set of CSS blocks that I want to put in other CSS blocks. Maybe we've been overthinking this. What about something simple like:
For issue #965:
Alternatively, allow mixins to be passed into mixins. (Best / most flexible solution??)
Like, check this shit out:
Why not just allow a mixin to evaluate and pass its evaluated block into a variable, and then just reference the variable? It still looks like plain ol' regular LESS to me. |
Ah, I see similar ideas have been discussed on #1640. |
AMENDMENT: I realized that to be consistent with interpolation, variables used to reference blocks should probably look like:
|
@matthew-dean, for your first example I would prefer this syntax, e.g.:
This is a bit easier to implement (and provides bonus of the |
Just in case. It's usually missed that it is possible to "pass" blocks to a mixin in indirect way w/o any #965 or similar. A block can be implemented as a callback mixin with a predefined name. The only problem is that this predefined name makes things difficult when we need more then one such callback per scope, but there're a few workarounds like "tagged callbacks" (discussed here for example) or "scoped callbacks" (like I used there). In context of 1. Using "scoped callbacks":
2. Using "tagged callbacks":
It's just has quite confusing syntax and also is pretty verbose, so I usually reference #965 (or more specifically, "features proposed in #965": [1] and [2] in particular, not the issue itself) as a solution to similar problems. |
@seven-phases-max Oh, interesting, so have the ability to have pass in a mixin reference, rather than evaluate the mixin. That works too. |
You're right, your way is cleaner, and still serves the same purpose. |
Oh, and since we're assigning a mixin to a variable via a mixin, we might as well allow just assigning mixins to variables that AREN'T in mixins:
Which is, of course, different from:
The first being a direct mixin reference, the second interpolating a string. |
Ah, and I see you proposed the same on #965. I missed that, sorry. Of anything I've seen, I think that's the most straightforward solution, and it looks like we had a similar conclusion. I'd say +1 for implementation of assigning mixins to vars, and passing mixin references to mixins as parameters. If we can implement this, I think we can close a pile of open issues. |
Something to keep in mind when fixing this is a behaviour I personally consider a bug in that this: @myVar: ".foo";
@{myVar} {
content: "Hello World!";
} Actually evaluates to: ".foo" {
content: "Hello World!";
} Anything quoted you use in a selector currently (annoyingly) remains quoted. Which leads to fun situations like: @unquoted: orange;
@quoted: "orange";
.color-@{unquoted},
.color-@{quoted} {
color: orange;
} .color-#ffa500,
.color-"orange" {
color: orange;
} That'll probably need fixing before that syntax could be implemented. |
It's just an example. The |
Keep in mind the context. As @seven-phases-max mentioned, while that example does provide some complexity, all we're doing here is trying to pass a block by reference. And the best way we have to define blocks is via mixins. So the most common use case will be a mixin that executes with no parameters. But, sure if the developer wants to add more complexity, it adds some powerful combinations. The compexity that they CAN create should not be confused with the complexity of the proposal, which it really is not. It's assigning a mixin to a variable, and calling that variable as a mixin, which is pretty straightforward (I'm speaking conceptually, not algorithmically). Also, if we can do this:
It seems unnecessary to do this:
A mixin name cannot currently conflict with any keywords, can it? |
I think the mixin bit is more complex to handle - you have to abstract out or use mixin call, you have to support calling with arguments and you have to parse a "selector" The other part of the proposal should just involve creating a new ast node (like expression) that supports calling a new node that absorbs Hrmm actually they are probably equal complexity to implement! I'd still go for the blocks before the mixin call.
Its the paramaters that concern me mainly in terms of adding complexity
I agree its not a complex proposal, it does however make use of the language more complex by adding another way of doing things. Thats what I am talking about. And I think we shouldn't add things which are "cool" unless we have concrete use-cases for them and think people will actually use that way of doing things.
so 1, as I said, the first bit only compiles because originally less tried to be clever and just ignore values it didn't understand. Thats why you can't use and yes a mixin name can conflict with a keyword. A mixin name can take the form edit I forgot you cannot use mixins without a . or a hash so no, currently mixins don't directly conflict with keywords. I'm still against it for the reasons below. If we start letting selectors be used as values then we are opening ourselves up for a whole load of future issues if our mixin syntax is allowed to become more like selectors or if css values are introduced that look like selectors. Thats why I want to keep selectors and values seperate. At the moment the only way of defining a selector (unless it parses by fluke) is this
we don't have to use a function called selector, I just very strongly feel we need some unque way of the parser being able to parse a selector rather than trying to work out whether it is a selector or a value. Since I think we need this anyway, I don't think we should make an exception for mixin selectors. I'm not sure if I like this but we could use a jquery like syntax
which is shorter. |
I like the |
On the syntax front, remember that in Less, we traditionally look for something that either parallels a CSS syntax or is CSS-like. CSS normally leans towards keywords. A @lukeapage You know more of the parser side, so if it's more complicated in the mixin call to do direct assignment, no problem. So, if we need to wrap it, "mixin" feels more intuitive to me than "selector" and a keyword is Less/CSS-like to me:
This implies that you can only "execute" the variable with ....Unless your suggestion is that someone would be able to pass in any matching selector, and you're defining a mixin as a selector? So, it would somewhat borrow the Huh. I never thought about being able to pass in and output the content of any matching selector, along with executing a mixin. That's kind of even cooler (if I'm following you correctly). But I wonder if "selector" is intuitive to refer to mixins for the layperson? Maybe supporting |
thats the reason I said I'm not sure if I like this. But I do very much like the short syntax.
there is a generic need to be able to describer a selector. since it would be a special "magic" function that made the parser act differently (like url) I would rather keep it to 1 for selector, not one for selector and 1 for mixin.
we do not need to wrap rule blocks, that does not have the same problems
No, I never meant that, I meant that the mechanism for describing a mixin should be the same as decribing a selector, I wouldn't want you to be able to mixin call any selector (though there is a seperate issue devoted to someone asking for that) |
Okay, so if we aren't mixin calling any selector, then we're just talking about referencing mixins, correct? So |
@matthew-dean a mixin call is a subset of a selector and how much of a subset it is could change in the future. See I implemented this feature - or at least phase 1 - @seven-phases-max I think this is simpler than the second case.. My main concern is evident from the tests.. its difficult to see whats a mixin call and whats a mixin definition. I wonder if there is something in the syntax that can be done to improve that ala ES6 inline function calls |
And now for 1.7 we have everything needed.. this can be closed? |
You said you only implemented phase 1? Which part was phase 1? And if so, should we leave open if there are other phases to implement? |
I guess this can be closed (#1859 entirely covers the subject use-case, and other yet more advanced cases/features we discussed above are probably better to have their dedicated tickets anyway. Later it will be not so easy to find anything important here hidden under the "Multiline Strings" title). |
Yes see my comments above for links to pull. This bug is about multiline |
Right ok. |
Added multiline string feature to the parser:
''' long_multiline_string '''
""" long_multiline_string """
As you see from the example above, this is especially useful for the mixin arguments.
In case you have other ideas regarding syntax, etc, I'm eager to change this pull request.
Things to consider:
What do you think about this pr?
Cheers, Alex