@import conflicts with parser

[cloudhead]

It seems like parsing @imports is conflicting with what the parser is trying to do, and breaks some functionality:

In lib.less @var: 1px;

In all.less @import "lib"; h1 { border-width: @var }

Doesn't work as expected, raising a NameError on @var

changing the parse function as so, to remove the import parsing seems to fix the issue:

modified = self.read_imports(file[:source]).push(file[:source]).max { |x, y| x.mtime <=> y.mtime }.mtime

to

modified = file[:source].mtime

[logandk]

I've been trying to reproduce this error using the same two files, but with no luck so far... Were there any other content in the less files?

For now I have disabled import parsing

[cloudhead]

Ok, the problem only occures with 1.2.5, and it seems like it's caused by this patch:

http://github.com/cloudhead/less/commit/4b6f8a2cac8d9943036c0dbb6f6d3041dbc4b1fc

I'm looking into it..

[cloudhead]

ok so the problem might be a bit more complicated: if the plugin is invoked twice in a row, say for "X.less" and "Y.less", and they each have @import "lib"; — "lib" won't be imported the second time. From what I see, this isn't the plugin's fault, it's how that patch handles @imported_files, which is at the Parser level, instead of the individual file level. I'll see if I can fix this.

[augustl]

Do you think we should drop this feature altogether? If anything, the less gem should implement partials, not the Rails plugin.

[cloudhead]

I forgot to say I fixed this in 1.2.7.

Regarding partials, that's how less works by default.. If I have a file like this, called main.less:

@import "lib";

h1 {...}
...

Less will parse "lib.less", but will only output main.less

[logandk]

Since all .less files in app/stylesheets are parsed and saved to public/stylesheets, the partials feature was meant for files that are only included and should not be parsed to a separate file in public/stylesheets.

Say you have a file called _colors.less:

@text_color: black;
@background_color: #EFEFEF;
@link_color: blue;

Since this file only contains variables to be used when included in other less files, it would be parsed and saved as an empty file in public/stylesheets.

I reckon that it might not be that useful in the end, since the empty file in the example above doesn't really cause any harm :)

Regardless, I think that the @import parsing for mtime checks makes sense. I've often worked on included less files and had to touch all the other files which included the specific file, to update their mtime and force re-parse. Do you agree?

[augustl]

I don't see why checking mtime is useful in the first place. I didn't notice any speed improvement at all after I added it to my less plugin.

[logandk]

If there is no significant difference in performance, then lets go for the simple solution and remove the mtime check

[augustl]

Yeah, that makes sense imo. Checking mtime is one of those "it makes sense so let's do it" kind of things that adds overhead without having real value. It's only useful for development mode, since the CSS is only generated once in production mode, and the performance benefit in development mode are minimal (based on me not noticing any difference).

[augustl]

BTW, why does partials have to be a feature? Isn't in enough for the plugin to skip generating .css for _foo.less? Won't the less gem do the rest when @include "_foo"-ing in the less files?

[logandk]

Sure, the code would be a lot cleaner without it!

Regarding partials, basically the only thing the plugin does now is to skip parsing _foo.less as you described. That's all there is to it.

[cloudhead]

skipping _*.less is perfect, no need for anything extra

[logandk]

Removing mtime checks. Closed by bbc4246c9352503f1d346317be1d40d50f796056. Removed import parsing. Closed by bbc4246c9352503f1d346317be1d40d50f796056.