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

.editorconfig option from dependency not working, even though file exists #30

Closed
localjo opened this issue Jan 15, 2016 · 14 comments
Closed

Comments

@localjo
Copy link

localjo commented Jan 15, 2016

I've been having trouble with the editorconfig option (see AlbertoElias/gulp-lintspaces#4).

When I pass in editorconfig: '.editorconfig' the feature works as expected. However, if I try to pass in an .editorconfig file from one of my dependencies (editorconfig: './node_modules/@myorg/code-standards/.editorconfig'), I don't get any linting errors reported. I verified that the file exists when node-lintspaces looks for it (if I remove the file from the dependency, I get Error: The config file "./node_modules/@myorg/code-standards/.editorconfig" wasn't found.).

I also tried renaming my .editorconfig file to .nonstandardname and passing in editorconfig: '.nonstandardname', and everything works as expected.

Any ideas why the file at ./node_modules/@myorg/code-standards/.editorconfig wouldn't work?

@localjo
Copy link
Author

localjo commented Jan 15, 2016

Seems that the issue is related to the .editorconfig being in a subdirectory. If I create /testconfig/.editorconfig and pass in editorconfig: './testconfig/.editorconfig' none of the rules seem to apply either.

@schorfES
Copy link
Owner

Have you tried to build your path with
path.join(__dirname, 'subdir', '.editorconfig')?

@localjo
Copy link
Author

localjo commented Jan 15, 2016

Nope. I can experiment with that. It's not clear exactly what I should be passing in there. But the file is found if I use a path relative to my project root; editorconfig: './node_modules/@myorg/code-standards/.editorconfig', the settings just don't get loaded. I suspect the problem is somewhere on these lines: https://github.com/schorfES/node-lintspaces/blob/master/lib/Validator.js#L146-L152, but testing out changes to that file is taking me awhile to set up, since it's a deep dependency for me.

@localjo
Copy link
Author

localjo commented Jan 15, 2016

Using editorconfig: path.join(__dirname, 'testconfig', '.editorconfig'), I get the same results; the file is found, but the settings in it don't get loaded.

@localjo
Copy link
Author

localjo commented Jan 15, 2016

The problem might be with the editorconfig.parse() function. Trying to get into inception mode to debug.

@schorfES
Copy link
Owner

These lines belong to the editorconfig api. Maybe a security issue in node.js or your OS...?

@localjo
Copy link
Author

localjo commented Jan 15, 2016

Looks like my code ends up calling
editorconfig.parse('/site/assets/scripts/index.js', { config: './testconfig/.editorconfig' });.

@localjo
Copy link
Author

localjo commented Jan 15, 2016

And editorconfig.parse returns {}, so the issue is over there. I'll move the discussion there. Thanks.

@localjo localjo closed this as completed Jan 15, 2016
@localjo localjo reopened this Jan 15, 2016
@localjo
Copy link
Author

localjo commented Jan 15, 2016

Actually, it seems like this might be related to using an outdated version of the editorconfig core: https://github.com/schorfES/node-lintspaces/blob/master/package.json#L48

@schorfES
Copy link
Owner

This version is outdated but currently required. Updating to the latest verison of editorconfig-core will bring breaking changes to the API of lintspaces itself. All in all it won't work synchronous anymore...

@localjo
Copy link
Author

localjo commented Jan 15, 2016

Tracked down my issue specifically to this line;
https://github.com/editorconfig/editorconfig-core-js/blob/50e0dba81b2f7f3e9ea4f701f2c65dd3f482cd4c/editorconfig.js#L68

That's mistakenly(?) appending the .editorconfig's path prefix to the match against the linted file's path. Maybe that's how it's supposed to work, so that an .editorconfig file only applies to the files inside of the same directory as it. In the case of node-lintspaces, or gulp-lintspaces, it seems that the .editorconfig rules should be applied to all of the gulp.src files. But maybe that's a misuse of the editorconfig core. Either way, I'm not sure how to go about resolving the issue. Are you planning to update node-lintspaces to use the latest version of editorconfig-core-js at some point in the future, or will everything I'm suggesting require manual forks?

@schorfES
Copy link
Owner

Yes, I think about updating to the latest version, but as written before it will bring a complete api change to lintspaces which will result in a new major version number. So the new release may take a while...

@localjo
Copy link
Author

localjo commented Jan 18, 2016

Understandable, and no pressure. Just wanted to get an idea of where you see this heading. Thanks for the info, and more importantly, thanks for your work on this project. :)

@schorfES
Copy link
Owner

Thank you for using this project ❤️

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

No branches or pull requests

2 participants