less / less.js

Less. The dynamic stylesheet language.
http://lesscss.org
Apache License 2.0
17.01k stars 3.41k forks source link

Import options #1185

Closed lukeapage closed 11 years ago

lukeapage commented 11 years ago

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.

@import "file.css" (min-width:400px);

The options I propose we support are

  1. treat as less or treat as css - people have to add ?.css onto the end of url's at the moment to treat as css - a bit of a hack
  2. import-multiple to replace @import-multiple though not so important if we want to drop
  3. whether to keep the import in place or import it - at the moment we keep css imports inline, but it would be nice to be able to include in css files (treated as an anymous node)
  4. The ability to include a less file, but not output anything - just make the classes available as mixins.

so here are some options.

 @import (multiple: true, less: true, include: true) "file.less" (min-width:400px);

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

@options (multiple: true, less: true, include: true) {
    @import "file.less" (min-width: 400px);
}

and variations on the above, such as using closer media query syntax like

@import (multiple: true) and (less: true) "file.less";

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 have

@import (multiple, less) "file.less" (min-width:400px);

and I am open to any other suggestions.

cscott commented 11 years ago

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.

lukeapage commented 11 years ago

Ok will merge tomorrow and make brackets required.. not sure this exact syntax matters too much as long as it is consistent and usable (which I think it is)

jonschlinkert commented 11 years ago

:+1:

lukeapage commented 11 years ago

moved to seperate issues for inclusion in 1.4.1

cscott commented 11 years ago

Whoo-hoo. Your changes look good to me, @lukeapage .

leeola commented 11 years ago

Huge thank you for this feature. Being able to extend vendor css is an awesome feature, thanks!

atomi commented 11 years ago

If I do an @import (less) "myfile.css", the css code gets mangled, with white space removal and other changes.

cscott commented 11 years ago

@atomi I think you were expecting the behavior of the "inline" option (which is issue #1209). That's not implemented yet.

atomi commented 11 years ago

@cscott Ah okay, I see. The @import (less) "myfile.css" option is for less files which for some reason have a fixed non .less extension. Thanks.

Edit: By the way will the (inline) option be available for the 1.4.0 release?

cscott commented 11 years ago

@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.

atomi commented 11 years ago

@cscott Thanks again.

I'm trying to extend selectors from a css file using @import (less) option as you've described.

I'm able to extend most selectors, but less seems to be having issues extending selectors with number in them - with the error NameError: .span2 is undefined

lukeapage commented 11 years ago

@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 :extend() or are you trying to call a selector as a mixin.. Please post some source.

atomi commented 11 years ago

@lukeapage


@import (less) "vendor/css/bootstrap.css";

#main-content {
    .row; // this works
    .btn-mini; // this works
    .span2; // fails
}

Edit: I made another test case just to test if this was a problem with the (less) import option or with less itself or with the bootstrap css file parsing as less.

In a simple selector use case, everything worked as expected. So this is likely bootstrap.css not parsing as a less file correctly. If this is intended behavior, sorry for the extra noise, thanks again.

cscott commented 11 years ago

@atomi please open a new bug for "bootstrap.css not parsing as a less file correctly"

lukeapage commented 11 years ago

@atomi I suggest you use bootstrap before it is compiled, rather than after, that way you have more control.

otherwise work out why .span2 isn't being picked up and raise a issue - it is almost certainly because it isn't used on its own?

donaldpipowitch commented 11 years ago

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. @import url(http://fonts.googleapis.com/css?family=Lato:300,400,700,300italic,400italic,700italic);).

lukeapage commented 11 years ago

@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.

donaldpipowitch commented 11 years ago

Thank you for the update!