-
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
Feature request: the @options
directive
#1134
Comments
So this means if I import an external library, it can mess up compiler settings for my files unless I override them. I believe that's undesired behavior. If multiple @options declarations can exist, I think they need to be scoped somehow (file scope? like it's the case with js linters). If only one @options declaration can exist, then we're only trying to agree here on the format for storing compiler options in an external file (that can be used by multiple tools, e.g. grunt-based build system). As for Part 2 of the proposal, to me this looks more like a compiler argument that would allow specifying the necessary options file at run time. So far the proposal a bit complicated to me, don't you think that build configuration (the use case implied from your examples) should be defined at the level of build tools, not at the level of, in this case, .less files themselves? |
"So this means if I import an external library, it can mess up compiler settings for my files unless I override them." Lol hey @dpashkevich, you make it sound like so much work!
"I think they need to be scoped somehow" agreed. it seems to me that this is addressed with "contexts" as I suggested, or am I missing something. If I am, maybe you can help me come up with a better way of scoping? In any case, scoping can't be done at all today, so this is a step in the right direction "If only one @options declaration can exist, then we're only trying to agree here on the format for storing compiler options in an external file" yeah, that's what I was thinking. that's why I prefer the ability to store multiple directives, similar to media queries. I don't think it makes sense to require only one "don't you think that build configuration (the use case implied from your examples) should be defined at the level of build tools" Yes, this doesn't preclude that. As I mention above you can still specify options in your build tool. However, you and I have collaborated quite a bit on build tool stuff, Grunt in particular. And build tools couldn't possibly achieve what this directive could regarding contexts. Meaning the ability to create contexts throughout the stylesheets so that regardless of the compiler the
and that script will only show up in the compiled result when @dpashkevich thanks for the feedback, and aren't you a SASS fan? jk :) |
@jonschlinkert Thanks so much for picking this back up. This is pretty important to me, because 1) it allows a LESS tool to "save" things like output file so that the "link" to a CSS file is preserved (good for GUIs like Crunch), and 2) it facilities working in teams (all members of the team have the same options for working with that LESS library. Originally, I, too had thought that embedding this inline in a LESS file was the best way to go, then was persuaded for different reasons that it should be an external JSON, but you make some good arguments to keep it in LESS. I think my two favorite and most persuasive reasons to keep it in LESS are:
I think the reason I was swayed was that you could use one config file to apply to multiple "root" LESS files, but I think that's an edge case. PLUS.... on the other hand... you are giving instructions to the parser.... so there's probably trade-offs either way. I think this is great work, but can you address the additional options / configuration that I laid out in issue #850? Specifically, setting (saving) an output CSS file? What is your proposed syntax? Also, would love to see @agatronic's thoughts. |
I just realized I didn't even put any thoughts on how this would work in the command line, but my thinking was that you might create a context as described above, say by the way, @MatthewDL I'd love to run something (mostly) unrelated by you, can I get in touch? |
@jonschlinkert Yes, sent you an email just now. You can also get in touch via http://matthewdean.me/ |
dis-regarding how to handle an import statement (include or not) my thoughts would be that the configuration should live in the build process. e.g. if I move from one domain to another and need to set a different rootpath, I would think that I should modify my build/deployment parameters, not my less files. I think where your spec falls down is on its simplicity to specify whether an import is a less file or a css file - you have to use an option instead of being able to simply tell the compiler what it is you are importing (which won't change) - I would like to specify in a clear, simple (and consistent across less files and libraries) way whether the file you want to include is css (which may or may not be valid less) and less. It's very nice and elegant but I don't see the problem it is trying to solve (other than Could you go into more detail about what the problem is / why you have always wanted this? |
re-reading the original issue, I've changed it back to high priority and would like to maybe just suggest the following changes
|
@agatronic you make some really great points. I've read through everything and I'll put some comments and questions up as soon as I have a chance in the next day or two. But in the meantime, after reading some of your points I think that in general whatever solution we end up with ought to be clean and straightforward, and hopefully not come with any warnings or gotchas. Your suggestions will help achieve that, so give me some time to think it over and I'll come back with some ideas to get your feedback. |
@jonschlinkert thanks, hope I wasn't too negative, I'm just trying to give a 2nd point of view. Look forward to hearing your comments here and on any other bug. |
@agatronic no not in the least, I think your comments are really constructive, and thank you. I just haven't had a chance before now to give a thoughtful response... I will definitely be spending more time here. I'm still considering some of the points you made about options, and one thing that is at the top of my mind is use cases where this could either help or hinder with external libraries. I've created some project scaffolds and test scenarios with Gruntjs over the last couple of days. This will sound a little crazy, but it's working - just for proof of concept I externalized less options to a few One thing this has accomplished is that it made me realize this should have been two requests. So I think we should narrow down this request to just the I'll be back with more answers... |
yes.. less compiles like this parse to work out the value of a variable is the last step, but options are required for the import stage.. so we have to be very careful in what is allowed. |
I have spent some time thinking about this and it seems like it would help to differentiate the value of the There has been a lot of discussion about externalizing options into json format, I'm curious though about what other people perceive as the advantages and disadvantages of that solution compared to this one. This would help with some of the suggestions I'm thinking of making. |
I just added $200 to the bounty on this issue ($255 currently) - which will go to the person whose pull request gets merged: https://www.bountysource.com/#repos/cloudhead/less.js/issues/1134 |
Wow, that's awesome. I think we still need some consensus on the best implementation of this feature, but this is great to see. Thanks! |
@ccvergara by the way, can you add your thoughts on why this feature would be valuable to you? No need to go into too much detail if you don't have time, thanks again! |
@jonschlinkert Part 1 seems like a decent way to standardize our development/production environment without adding extra files. |
so..
Questions around part 1..
|
also the original issue had a map of less file paths to css file paths for less to compile.. should that be included and should it be under a directive called |
@agatronic, agreed on Part 1. We can assume that Part 2 is tabled for now. I still like what it could do for Less, but it would make more sense to come back to it later with some experience after using
Regarding your last questions, it might help if we put a gist together to make the picture more clear. I like how the comments are below on gists, so you can just focus on the code if you want. It sounds like we would have two files in the gist, one showing a separate Sound okay? And do you remember seeing another issue that had a list of files like what you're mentioning? Seems like I either commented on one or saw one on here recently... I'll look tomorrow when I get a chance too. |
In regards to the order of how thing are parsed, I think it's an easy requirement that a) @options must appear in the first file parsed, b) @options must be the first declarations of the file. Any reason to not require @options first? Otherwise, like @agatronic says, we run into logic issues, as well as unexpected behaviors where an external lib has an options block tucked away somewhere, driving devs crazy by overriding parsing settings. |
I would say that the @options blocks should be parsed and cascaded in the order they appear in the files being loaded. That way a library could have options if it needs them, but the user can override them in the sheets that import them. This may require that options be set after import statements, however. |
@MatthewDL I think that's okay. This also implies that options directives found in imported libs would be ignored, and not throw an error? That would have to be the case, otherwise this wouldn't work. The reason I initially did suggest that multiple |
ha, @Soviut lol. looks like we shared a brain for a moment. |
however, one battle at a time. So again, I'm supportive of only one @options directive being acknowledged, and if it must be at the beginning, then it must be at the beginning. I'm cool with that. |
oops I meant to hit the comment button, not "close and comment"!!!!. |
yes, I added support for and no.. I think his concern was exactly because of this.. that he wanted clear seperation over what modifies the page, and what modified the parser - I guess that someone who doesn't know less or css won't know if There was no concern to the fact that it was a less syntax however, if it was in comments (or - not discussed - but seperate files?) any bright ideas on how to resolve all of these (and the above..).. I have to admit it feels like a pretty tricky feature to nail down, so look forward to ideas. |
Lol. And yeah that makes sense. I don't like the comments idea as I mentioned on the phone, at least it doesn't sound like something I want in my own code. I do prefer Any, unless I'm forgetting a reason that we didn't want to do |
... and if |
The syntax he was concurring with on the phone would be something like:
That's MS's convention for setting up special config code. The extra comment slash tells GUIs / code editors that it's not just a block you commented out, but code to be colored normally. And by having at least 2 slashes, it's clear to LESS authors that nothing's going to show up in their resulting CSS from this block. |
@lukeapage and @MatthewDL, I think this feature is interesting and could be useful, but I'd prefer to close it if we have to go the comments route. We could always revisit down the road if someone comes up with another way to approach this. I'm not attached to this feature, and if a straightforward implementation isn't transparent, then it might be more trouble than it's worth. So I'm completely okay with closing this if you decide to on a whim. However, in case you do decide to keep this open, I've thought about this feature and the discussion we had, and I can definitely understand where @cloudhead was coming from about to the first point
In other words, at-rules look and feel a lot like configuration options. We have: @charset, @document, @font-face, @import, @keyframes, @media, @page, @supports, @namespace
Nested at-rules, a subset of nested statements, that can be used not only as a statement of a style sheet, but also inside conditional group rules:
All of the above are CSS at-rules, so what would a more dynamic and powerful "LESS at-rule" do? IMO to the second point I don't think we should worry about it. In general we should take things like this into consideration, but:
Anyway, I'm just having fun discussing this, and I do think it would be useful. But you don't even need to ask my feedback again on this if you don't want. I'm good with whatever you guys decide on it. |
@jonschlinkert I agree it might be a good idea to leave this for a skype session, try to flesh it out more and then come back here with a proposal? We all agree its a good idea but I don't think we have a complete proposal yet that covers how it would all work (in an implementable way) - - many apologies if you think we do, I think its because its split across many comments, its hard for me to sum it all up. I wouldn't like to close this because then presumably we loose the bounty on it - which is quite cool - for $255 I'm surprised it isn't attracting someone to have a go at fleshing out a full proposal, getting support and implementing. |
Sounds okay to me, I forgot about the bounty too |
There's a bounty? o_O |
Copying this over from another (unrelated) Issue, as a reminder Suggestion was to use alternative JSHint style config options via code comments: /* less strictMaths: false, strictUnits: false */ |
I've also been thinking more and more that we really should consider implementing both JSON formatted options ( It makes sense that we should try to be more accommodating with offering different ways to control options, with the impact being that some of our more controversial Issues, such as As for the specifics of how the various options formats should be implemented, I strongly urge that we follow JSHint conventions as close to the letter as possible. JSHint is wildly popular, and using their conventions will make life a lot easier for anyone who is already familiar with |
JSHint is appropriate for JavaScript. @options { } is appropriate for LESS for the same reason, but JSHint syntax does not match LESS. Potential problems I see with having both formats is a) confusion, b) conflict (both @options and JSON are defined, who wins?). Incidentally, Jon, I'm in favor of @options w/o comments, but thanks for the lengthy case made. |
@MatthewDL do you read entire posts, or just skim? |
With the volume of posts on Github for this library (which is great, btw) and the length of comments, skimming is what I typically have time for, unfortunately. Was there a specific thing I missed you wanted feedback on? |
I was thinking of this today when the subject of plugins came up. JSON is more logical in some cases, specifically if/when we support plugins (because it's weird that I'm linking to a JavaScript file from within a LESS file). And considering that options potentially tell the parser how to parse, it's somewhat counter-intuitive that it's in the file that's being parsed. @options { } in-line in your LESS seems more intuitive at first glance. But it's not really related to your LESS, it's meta data / instructions ABOUT your LESS, and those instructions are for a JavaScript-based system. I'm not sure if it should be one or the other, but you're right, in some specific cases, JSON is a clear and perhaps necessary winner. In other cases (ease of use), LESS-style seems better. But, if we did both, it may make more sense to do JSON first, since it's the one needed for sure, and strikes me as easier to implement. However, if we do it in JSON, then the value-add of declaring inline in code shrinks drastically. |
One reason inline options might be useful is in situations where tools don't allow you to set command line options or use external files. In the following issue, the user is using CodeKit which doesn't let them set command line flags. |
I'd love to see this implemented, JSON first, and I think it would make a lot of sense to be able to use this JSON file to define LESS variables as well. Issues such as #1359 (comment) come to mind. |
@jonschlinkert I've reopened issue #850, and am looking back into this, along with some other developers. I agree, JSON is easier (and probably cleaner) to implement as a baseline. |
👍 |
In ordered preference:
|
Agreed, that would be my order of preference as well. |
I'm hoping to address this early 2014. And yes, JSON-focused makes sense moving forward. |
Closing since we settled on JSON, merging separate options discussion to #1893. |
This is something I've wanted since first using LESS, and I think I've come up with a solution that could be pretty awesome if we get some feedback and tweak it to get the kinks out.
Proposal, Part 1: The
@options
directiveI propose that we use the
@options
directive to allow setting processing options directly inside less files themselves, like this:@options { // options }
As with media queries, the
@options
directive wraps a declaration block which contains the options/flags for how you want your less to compile. When this directive is found, less.js reads in the options and processes all less files accordingly. If multiple@options
directives are identified, less.js either throws an error or it uses the options from the last directive (I'll leave that decision to others). If the latter is chosen and multiple@options
directives can exist simultaneously, my suggestion would be to process it like this:If multiple
@options
directives could co-exist:First directive identified...
Second directive identified
Another suggestion would be to use
!important
to designate which options should not be overridden.Why would multiple
@options
directives exist? A good use case is that when importing external libraries, such as Twitter Bootstrap, you may wish to override the options from that library.Regardless of how it's actually implemented, what I find so compelling about this concept is that it paves the way for keeping the syntax and language cleaner, it keeps options closer to the less files, and forces a stronger separation of concerns between processing/compiling features and language features. To that end, I often see feature requests or Issues where the OP is asking for something to be added to the language, when really all that is needed is a new processing/compiling option. LESS is a language, but less.js is a compiler! This makes that distinction much clearer, and it allows for the user to store different options across different projects, either directly inside a single less file, in several less files, or even organized in
options.less
files that can be reused across projects. Whichever you prefer, it's a better approach than putting those options in ajson
file or javascript.see: #850
Proposal, Part 2: Setting "Contexts" with the
@options
directiveWith this proposal, "context" is created in a kind of similar way to setting context in "logic-less" templating languages, such as mustache. So, for instance, rather than hard-coding processing options in
@import
or@include
directives, we would instead use "contexts" so that the choice is made by the developer, and they have the choice of setting the options in the less files themselves or on-the-fly at compiling time.(and how awesome would it be to be able to use variables in
paths
androotpaths
now that we are storing those properties in our less files!)A common use case is to create contexts for environments, like development and production, so that less.js (
env
) processes the options according to how they are defined in the less files themselves:and...
Applying contextual options in your less files
Media queries are a great starting point for inspiration, for example:
And CSS syntax already allows us flexibility with media queries with @import and @media, like:
And written in HTML, XHTML, XML, @import and @media:
So following convention established by
@imports
and media queries, here are some ideas for how we can take the contexts we created using the@options("context")
directive and apply them throughout our less stylesheets:@context("prod") { .some-experimental-class { display: none; } .sidebar { width: 250px; } ... }
and...
@context("dev") { .some-experimental-class { display: block; } .sidebar { width: 210px; } ... }
This gives us the choice to extend the directive in the future:
As with media queries, these would be equivalent:
Or, for example, we could set the "contexts" from our
@options
on other directives like this:Example of other Issues that could be resolved with this:
@include
: @include .css file contents #560I would solve the
@include
issue like this:and...
or maybe just...
Last, it's understood that
@options
could collide with custom variables, so either 1) bake it in as a reserved word and force people to not use that as a variable name, 2) allow@options: variable
and@options {}
to co-exist peacefully, or 3) we just use a different namespacey term than@options
. I like #2 the most, alternatively #1.The text was updated successfully, but these errors were encountered: