Skip to content
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

force-ignore specific imports #30

Closed
Pomax opened this issue Apr 15, 2014 · 17 comments · Fixed by #31
Closed

force-ignore specific imports #30

Pomax opened this issue Apr 15, 2014 · 17 comments · Fixed by #31

Comments

@Pomax
Copy link

Pomax commented Apr 15, 2014

I'd like to lint all the LESS that is used in my project, but not the LESS file(s) that come from things like bower components that are linked into my project, so I'd like to tell it to ignore anything it finds "wrong" in files from the bower_components dir, even if it has to be linked into my own less files to compile to a valid .css result.

Is this possible?

@jgable
Copy link
Owner

jgable commented Apr 16, 2014

We shouldn't be reporting errors from imported files unless you pass an imports option that specifies them.

Can you paste your grunt config so we can check if that is what you are doing?

@Pomax
Copy link
Author

Pomax commented Apr 16, 2014

that's what I mean. My less has "import ...." and some of those imports are from bower_components locations (so that we can use not-our-module variables, etc). However, whether they lint their css/less or not can completely ruin less linting of our own code, since there does not appear to be a way to say "lint only the sections that belong to files that we made. follow dependencies, but don't lint ones that come from [bower_components, node_modules, etc]". Even if our less is technically good, linting errors in not-our-style can make the entire linting process fail, which ends up causing continuous integration testing failures (travisCI etc) and thus means we can't be sure whether we can merge code in for deployment or not =(

@jgable
Copy link
Owner

jgable commented Apr 16, 2014

Yes, I understand. We don't report those errors unless your grunt configuration passes an imports option.

.... Can you paste your grunt config so we can check if that is what you are doing?

@Pomax
Copy link
Author

Pomax commented Apr 16, 2014

aha. Yeah, let me find the person whose branch raised this issue for us, he'll have a pastable gruntfile =)

@Hamabama
Copy link

https://github.com/Hamabama/popcorn.webmaker.org/blob/lesslint/Gruntfile.js
Here please see we have attempted to exclude bower dir from linting, however it follows import paths, so it ends up linting files from bower dir.

@Hamabama
Copy link

Additional information: Here you can see we import languages.less on line 4, so it follows it and lints it and lints all import paths from imported languages.less as well.
https://github.com/Hamabama/popcorn.webmaker.org/blob/lesslint/public/css/header.less

@jgable
Copy link
Owner

jgable commented Apr 16, 2014

I cloned down your project and am not getting any errors reported when running grunt lesslint. Here is my output

> grunt lesslint --verbose

Running "lesslint" task

Running "lesslint:src" (lesslint) task
Verifying property lesslint.src exists in config...OK
Files: public/css/butter.ui.less, public/css/embed.less -> src
Options: less=undefined, csslint={"adjoining-classes":false,"box-model":false,"box-sizing":false,"bulletproof-font-face":false,"compatible-vendor-prefixes":false,"duplicate-background-images":false,"fallback-colors":false,"ids":false,"important":false,"outline-none":false,"overqualified-elements":false,"qualified-headings":false,"regex-selectors":false,"star-property-hack":false,"underscore-property-hack":false,"universal-selector":false,"unique-headings":false,"unqualified-attributes":false,"vendor-prefix":false,"zero-units":false}, imports=undefined, cache=false
Linting 'public/css/butter.ui.less'Reading public/css/butter.ui.less...OK
Linting 'public/css/embed.less'Reading public/css/embed.less...OK
>> 2 files lint free.

Can you give me an example where it's giving you an error it shouldn't?

@Hamabama
Copy link

Oh sorry. You need to change on line 12 version of webmaker-language-picker in bower.json to 19.
https://github.com/Hamabama/popcorn.webmaker.org/blob/lesslint/bower.json

@Hamabama
Copy link

In version 20 we made temporary solution, so it passes Travis. But the real issue is with version 19.

@Hamabama
Copy link

In header.less it will import webmaker-language-picker/stylesheets/languages.less and next will be imported files form languages.less and Travis will complain that it can't find 'colors.less'.

@jgable
Copy link
Owner

jgable commented Apr 16, 2014

Right, that's not reporting a linting error, that's reporting a parsing error. It can't parse the less file, so it can't lint it.

I just tried running lessc directly on the file and it works (via lessc --strict-imports public/css/butter.ui.less) for some reason.

@Hamabama
Copy link

In other words, we have to force grunt to don't parse certain imports, right?

@Hamabama
Copy link

"!public/static/bower/*/" is directive for grunt to exclude path from array of paths, so theoretically it shouldn't parse it. However it follows import paths and parses it. Oo

@jgable
Copy link
Owner

jgable commented Apr 16, 2014

It doesn't work like that. The actual LESS Parser is the thing that is throwing that error. I'm not even sure why the command line allows it, I'm still doing some research on what options might be different.

@Hamabama
Copy link

Thank you for quick response. We'll be waiting to hear from you :)

@jgable
Copy link
Owner

jgable commented Apr 16, 2014

Ok, the only reason it works when you run the command line directly (lessc --strict-imports public/css/butter.ui.less) is that they add the public/css directory to the paths to look for imports in and it finds public/css/colors.less to resolve that import and it doesn't fail.

I don't think that's actually what you wanted to do with that import statement, but I'm going to see about adding the current files path to the parse call to match the command line default behavior.

jgable added a commit that referenced this issue Apr 16, 2014
Closes #30

- Use fully resolved path for file names
- Pass paths option to LESS Parser
@jgable
Copy link
Owner

jgable commented Apr 16, 2014

I'm still having some problems after publishing a new version. Looks like the source maps aren't able to resolve some of the errors in your files.

It also looks like CSS Lint is reporting some "rollup" errors which aren't associated with files and cause problems when I try to get a source map location for them. I'll create another issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants