-
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
Import options #1185
Comments
@MatthewDL's suggestion from #560
assume the options need to come before media queries and are differentiated from media queries because they are no in brackets? |
I would add to your list the option to either combine imported files or keep them separate, regardless of whether they are LESS or CSS. The use case for me is that during development I often compile Less files separately so that it's much faster to debug, improve styling for a specific component, and force a separation of concerns. Then I combine stylesheets for production. It would be nice not to have to add |
Isn't that an overall option rather than one to do on a single import? It I think I'd like to tackle it seperately after this. All the above options (bar the import but silent) are easy.. I just want |
I prefer the single line syntaxes rather than nested in an options block. Yes, I like the assumption of "true" for keywords. CSS behaves the same way. (If present, is on., a la the id attribute in header: The last part (min-width) isn't valid is it?
The parser wouldn't know the width of your screen (unless compiling in browser). Unless, of course, we pushed the parsed block into a media query? Is that already supported in Less? (Pretty fucking cool if it is.) I'm a fan of that or just straight keywords:
The @options block I had thought was for project-wide options. Wrapping it multiple times for stuff muddies the options feature, in my mind. |
@MatthewDL that syntax is css - if it is a css file imported less leaves it as it is and the browser wraps it in a media query essentially. If its a less file, less wraps the contents in a media query ;) that example looks good to me.
and the |
@MatthewDL that syntax is css - if it is a css file imported less leaves it as it is and the browser wraps it in a media query essentially. If its a less file, less wraps the contents in a media query ;) that example looks good to me.
|
Well then that's fucking cool. I didn't know that. Is that documented? |
@MatthewDL lol yeah I know, it needs to be documented for sure! I thought that was pretty awesome too when I figured that out - that's why I threw it into the hat - since it already had precedence.
I'm sold, that makes a lot of sense, and I don't really have any strong preferences regarding how this is accomplished. It would just be really useful to have options regardless of how it's done. And anyway my preference has always been to do this inline, and I think my suggestion for doing this using the
I imagine Less would throw and error if you specified subdirectories but one of them did not contain an index.less file. I want to sell you guys on this, particularly @MatthewDL ;-) because I think this would be the feature that puts LESS on the path to offering Compass-like capabilities. Please spend some time looking at node-glob before you dismiss this, it's very flexible, has strong support, it's used pervasively by Grunt.js for instance and it's addictive to use, and it allows many different patterns for including or excluding characters, files or directories, etc. So I envision using this to begin creating libraries similar to Compass. I understand that it wouldn't work in the browser, but IMO we should not allow a "worst practice" to prevent progress of the rest of the spec. And there are many tools out that that allow you to edit Less and have it compile and refresh the browser in real-time. |
I like this idea:
I think "grouping" settings might be valuable (such as a case where I have a group of files that end in ".css", but are actually Less files, which a number of users have described). However, I think we might need one tweak for syntax. In CSS, curly braces inevitably wraps either property/value pairs terminated by semi-colons, or a group of selectors, such as with media queries. Animation keyframes have a group of keywords (values) which are followed by curly braced set. So, can we make this more CSS-y? I can't think of how, off the top of my head. Not that Less has never added things that don't resemble CSS, but I know in conversations with @cloudhead that he was always loathe to. That is, let's make sure the syntax doesn't exist in CSS before we add something new. I'll see if I can take some time to look at node-glob. I would say that could be its own issue (mass import). It does raise a question that's been tickling the back of my brain, but it would be a major shift, so I'm almost afraid to mention it. lol |
we could grab one of the other syntax ideas that were thrown out for imports and just swap it in. The file list part is what I think would be nice to have, so whatever syntax makes the most sense is cool with me. But just a comment on this:
IMO the example is a CSS-y syntax, I chose it because it's similar to media queries. We probably won't be able to please everyone anyway, so whatever syntax makes the most sense from the standpoint of enabling the functionality that we require is what I think we should implement.
I did make another issue for that #1181. But I'm actually thinking that since it does keep coming up along with the other import-related issues that it might be best to merge it over here. One thing we could start doing is using the new checkbox feature that GitHub added to Issues. I think it's for this very use case, and it allows you to add "tasks" to Issues. You just do
Then anyone with edit rights can check the boxes in read mode. I saw these a couple of times before I realized you could actually check and uncheck them |
By CSS-y, I mean specifically that there's nothing in CSS where a comma-separated list is enclosed in curly braces. CSS typically treats lists as single line declarations (such as the CSS3 multiple-background syntax, or font-family). If you think of the font shorthand, a more CSS-y type declaration would be like the following:
or, alternatively:
I think of LESS as CSS+. If a syntax pattern exists in CSS, we should either use it or make a strong case why we're not using it. I couldn't think of any exceptions to this that might include a comma-separated list in curly braces, which is why I raised the issue. |
@MatthewDL Oh, lol. Yeah that's true, and I didn't know you were talking about that part of the syntax, which is the part I like. ;-) And what you're saying makes sense, but technically there are dozens of properties that allow a comma separated list, so if we wanted to be creative (before I put this, I really could go with whatever pattern works best from an implementation standpoint. I'm just throwing out stuff to see if anything works): The thing about the @options concept that might not have been clear above, is that it's most useful for more than one statement, and wouldn't like be as complicated in most cases. So, this might be more common (I'm not really focusing on what options are actually here, just the general syntax, so whether it's
With the idea being that we would still have a "global other concepts
By the way, I think I need clarification on what the options are: "include", "multiple" and "less". I'm confused between multiple and include in particular.
|
It is documented here someone needs to pull that. I suggest that we start simply and allow ourselves room to maneuver I prefer this syntax best...
and I suggest we start with only implementing the first two for now. I suggest we can include these options in the After we have done points (1) and (2) and the I think its important we look ahead, but I think as long as we are happy we have a rough roadmap, we don't need to consider this advanced usage - no-one has asked for it yet. Remember that files with extension .less won't need a less option, they will be by default parsed as less still - and the that the silent option is likely used with a library where you just need to import a single file - so I've not yet seen a use-case where someone needs to apply these |
How about this for the option names? multiple, once - whether to import the file with that href/path once or multiple times. Default once. less, css - whether to treat the import as less or css. Defaults to less unless ending in .css, then it is css silent - whether to silence the output and just allow the importer to reference the styles inline - whether to put the contents inline or keep it as an import. must be true for less files, css files default to false |
I like all of the above keywords except I think inline is unnecessary. If it's a CSS, can we not just interpret as LESS? Of course, you may have invalid CSS, but just wondering about that one. In general, in agreement with @agatronic. I don't like the @options blocks used with this. I think this should be allowed:
It makes it additive to this:
So, the dev isn't required to add parentheses by adding more options. Seems pretty similar to shorthand syntax then. And if its shorthand, maybe longhand is this?:
(Combining some of @jonschlinkert's ideas.) |
if we get rid of the brackets I would like to get rid of the commas between file names
reads as
I don't like the keywords at the end because media queries can look like this
and I'd rather not be trying to distangle a media query keyword (tv) from a inline is useful if you have a component with its own stylesheet, which isn't less compatible and you want to compile your stylesheets together without using an extra tool. There are quite a few things that less doesn't parse around comments and particularly css hacks. One use-case was a guy using the jquery ui stylesheet which has css hacks in it - an obviously you don't want to edit the library - but at the same time would quite like the css inlined in the place you want it. |
Ah, interesting.... that being the case, then forcing compile flags at start seems reasonable. So, you're suggesting syntax like this? :
Basically, one can add as many quoted files as they want (and import keywords, but must be at start)? By the way, what's the use case for importing a file multiple times? Changing variable scope? I never quite got that. |
Multiple has no know usecases but is current behavior. In previous pulls |
Yep, makes sense. |
How about the "@use" directive suggestion that was merged into this issue? The case is that sometimes we need to have rules for .one-class in one file exactly like .another-class in another file. That is to say, we need an @import directive, that even if the file is included only once, no rules will be produced additional rules, and its contents can be used as mixins only. |
@ztane that is the silent option |
@agatronic
and
Sounds good to me.
Sounds good to me. @agatronic's proposal for options:
multiple
My gut tells me that something like this happened: originally, Less.js just allowed multiple by default, then someone made a request to "fix" it, and acknowledging that it needed to be fixed, when @cloudhead implemented "multiple" he was probably planning for when I say "to the gallows with multiple"! IMO it's just clutter and I think we should deprecate it "officially" and we should stop using it in examples so we don't confuse anyone (I already do that enough for us) - I don't think any of us have ever heard one use case for it. We could still keep in the code for now, and I can label it clearly as deprecated in any documentation (primarily here #64, as @agatronic pointed out) - allowing anyone who sees the deprecation notice ample opportunity to speak up and fill us all in and provide the heretofore undiscovered use case for keeping it (and maybe win some kind of prize?). With the net impact that nothing really changes and you're not really doing anything in the code, but we can stop using it in these examples and plan to phase it out. That's my 4 cents on that. lessI suppose this makes sense, but what was the use case for this? Whatever it is I agree with @MatthewDL that it should default to LESS. silentWhy don't we just call this what it is? "mixin". Isn't that what we're doing here? @import mixin "one"; If I'm wrong then ignore my point as well as the next statement lol. But "silent" kind of reminds me of how SASS chose terminology for mixins. You create a mixin with inline
I'm inclined to agree, seems like if you have access to a CSS file to import it, why not just rename it to ".less". Although it has been requested by several folks as a way to concatenate CSS files alongside LESS files, rather than pass them through individually as with a regular CSS So assuming we do implement this feature, unless I'm missing something, here are the reasons I think this should be called concatenate, or "concat" for short:
I don't even mind if we call it "concatcss", to describe exactly what it's doing. There are plenty of plugins out there called "mincss" or "cssmin" etc. In any case, if you don't want to use concat, at least don't use "inline". However, I personally would much rather see the option for what I requested earlier in this thread (here #1185 (comment)), which is kind of the opposite of the inline feature and would add an option to split out all |
Regarding the last paragraph of my comment, that option could be called "separate" or "split". |
@jonschlinkert There's a lot of related issues to reference. "less" is for when it's not a file that explicitly ends in ".less", but you want to interpret as LESS. There are a number of reasons people have mentioned why this might happen, but I don't want to summarize them all. It's valid.
No. This reads like I'm importing a mixin called "one", or maybe only mixins from "one.less". But, in any case, it's not a mixin. "Silent" has nothing to do with mixins. I just don't want it to render anything. Whether or not that roughly matches a definition of mixin seems neither here nor there, it's just an obfuscation of terms. I think "silent" is an elegant keyword for importing an entire library without rendering, whereas mixins in LESS do not span libraries, typically, plus the import takes no variables. So, no. "multiple" - You're probably right. LESS just faithfully respected every time you used the @import statement, and assumed you knew what you were doing. It was probably just not anticipated that you would obnoxiously import the same thing several times, yet somehow expect it to show up once. But of course, as you said, people have done just that, and understandably, because they are importing libraries as components that can separately reference the same file. So, I think that's an interesting proposal. If I recall, @agatronic or someone mentioned some use cases where someone claimed to use multiple import behavior, but if so, it's probably an edge case and not the only way to go about things to get the same result in LESS. "inline" - @jonschlinkert - Your definition of "inline" actually describes the behavior @agatronic is definining. Rather than the CSS staying external, it's rendered inline. But, I don't disagree that's a weird feature in and of itself.
There have been lots of times historically where LESS couldn't read a CSS library. @font-face, for example, wasn't supported for a while. In those cases, you just referenced the CSS separately in your HTML. Wasn't the end of the world. I don't think LESS has to necessarily be a minifier for all manner of CSS that isn't valid. There are minifiers to do just that. So, I can have my LESS lib, my CSS libs, and if I really care about the extra HTTP requests, I can install a minifier to lump up various http resources. On the other hand, devil's advocate, it's trivial to include, but yeah, kinda iffy. |
yep.. I suggest reading the 3 linked bugs. multiple - I think I might implement it but not document it - if people complain we can add it to the documentation, if they don't I'll remove it the release after inline - @MatthewDL it may not be "the end of the world" to render css outside less, but its hardly making things easy. Also there are css hacks less has never supported and still doesn't - should we just ask people to wait forever.. ideally people want only 1 stylesheet. Regarding getting an extra minifier, if less didn't include its own minifier and use yahoos minifier I would agree with you, but it does include those. So for all those reasons and because it is a low cost implementation, I would like to do it. @jonschlinkert your option for keeping files seperate - I would like to handle that as a seperate issue/request - all the above options are relatively simple and have been asked for time and time again over the last 2 years. As I said it is slightly different because I think it would benefit from being a global option better than a option on each import. |
Agreed, thanks. I'll create that issue. How fast are we moving on this one? Seems like there should be more discussion before a final decision is made. I'm having a difficult time understanding some of the reasoning here. |
As I said, these issues have been long standing for a long time. I wanted to get this resolved for 1.40. what more discussion needs to take place? |
I think if we support this:
...then we should probably support both of these:
Or is that too complicated? Just feels weird to change the config syntax based on number of options. CSS doesn't usually act that way. If we don't support both, then I would support this:
What do other people think? |
that's a strict superset of the existing syntax, i think lukeapage could easily merge the current patch and you could extend the syntax in the next release if users demanded it. The parentheses make parsing less ambiguous, fwiw. Although the only acceptable production is a url, and I think that either has to be quoted or start with "url"... although someone should read the CSS specs closely to be sure. |
Ok will merge tomorrow and make brackets required.. not sure this exact |
👍 |
…& less. First step in implementing syntax for @import options, proposed in #1185 (comment) (steps (1) and (2)). I've implemented the 'multiple' and 'less' options. One could trivially add 'once' and 'css' options as well, if there was need. Proposed "silent" and "inline" options are deferred for future work. I left the existing "@import-multiple" and "@import-once" syntax in place, although the proposal is for this to be deprecated once the new option syntax is in place.
moved to seperate issues for inclusion in 1.4.1 |
Whoo-hoo. Your changes look good to me, @lukeapage . |
Huge thank you for this feature. Being able to extend vendor css is an awesome feature, thanks! |
If I do an |
@cscott Ah okay, I see. The Edit: By the way will the |
@atomi another use of the (less) option might be to import "less-compatible" CSS files (again, with fixed extensions) with selectors you want to extend. The bug describing (inline) is targeted at 1.4.1, and seems to have been coded and committed already on the 1.4.1 branch. Further discussion should probably happen in #1209. |
@cscott Thanks again. I'm trying to extend selectors from a css file using I'm able to extend most selectors, but less seems to be having issues extending selectors with number in them - with the error |
@atomi if you use inline you cannot access the selectors as less - the file is just plunked "unread" into your output. If you are reading a css file as a less file, it should work as long as the file parses.. How are you "extending".. using |
@import (less) "vendor/css/bootstrap.css";
#main-content {
.row; // this works
.btn-mini; // this works
.span2; // fails
} Edit: In a simple selector use case, everything worked as expected. So this is likely bootstrap.css not parsing as a less file correctly. |
@atomi please open a new bug for "bootstrap.css not parsing as a less file correctly" |
@atomi I suggest you use bootstrap before it is compiled, rather than after, that way you have more control. otherwise work out why |
Any progress on this? I want to inline an external css, so I don't have to make an additional request for this (e.g. |
@donaldpipowitch import options as a whole are in 1.4.0. The inline option was added to the 1.5.0 wip branch, which is not yet released. I hope to release a beta of it in the next couple of weeks. |
Thank you for the update! |
We have decided that the best way to handle the import bugs is by having options.
Given the scope of the bugs I feel strongly that the options need to be inline with the actual import statement
I suggest removing
@import-once
and@import-multiple
and allowing options to be passed to the@import statement
Note that import can already be followed by media statements e.g.
The options I propose we support are
?.css
onto the end of url's at the moment to treat as css - a bit of a hack@import-multiple
though not so important if we want to dropso here are some options.
downsides are its a bit confusing with the media query syntax. we could also put the options second and mix them with the media query, defining our own special media query options essentially, but I don't like that in case we conflict in the future with css.
from @jonschlinkert
and variations on the above, such as using closer media query syntax like
I initially disliked @jonschlinkert's options idea, but it does actually allow for setting defaults.. what I don't like is that it looks a bit verbose.
we could also assume
:true
and haveand I am open to any other suggestions.
The text was updated successfully, but these errors were encountered: