Isn't this essentially just duplicating the overriding of the Global Node Path setting, but for non-standard installations? If you are using eslint in a project, you should have it installed as a dependency of the project, not using some sub-dependency of a dependency.

I guess I would be okay with this functionality being in here though if somebody else in @AtomLinter/linter-eslint agrees with this being necessary.

Isn't this essentially just duplicating the overriding of the Global Node Path setting, but for non-standard installations? If you are using eslint in a project, you should have it installed as a dependency of the project, not using some sub-dependency of a dependency.

Not at all. Large projects often consist of many modules (not node modules) and each of these modules can have it's own set of dependencies. In such projects a node_modules folder in the project root makes no sense because each module has it's own requirements i.e. it's own node_modules directory.

This setting is very useful when you have a larger project opened in Atom, where only one of the subcomponents is Javascript code, and when you want to avoid installing eslint globally.

The other components can be e.g. Python code and having a package.json and node_modules is not appropriate at top-level.

@jsnajdr, @MikeRatcliffe: What sort of structure are you talking about exactly? This works just fine currently:
linter-eslint_GH585.zip

There are literally hundreds of modules in our tree. A simplified example of the tree is below.

Even though each module is in the same project it has an element of independence and may require it's own set of node modules with specific version numbers. To allow this independence we put the node_modules folder inside each modules folder. There is no way to guarantee the depth etc. because it all depends where the js files are in that module. e.g. our Loop module may use a different version of eslint than our DevTools module... this is fine because the node_modules folders are inside each module.

If we had a node_modules folder at the root of the tree then there would be cross contamination from all of the modules in our tree and we couldn't count on a specific version of a specific node module. This would mean that to upgrade a node module for e.g. our networking module we would need to test the module on hundreds of other parts of our tree and fix all affected modules, which is completely unnecessary.

• /networking/
  • /networking/module/ (cpp)
    • file.cpp
      • /networking/module/engine/
        • file.js
        • node_modules... unique set of node modules with specific version numbers incompatible with other modules.
        • ...
• /parser/
  • /parser/module/ (js)
    • file.js
    • node_modules... unique set of node modules with specific version numbers incompatible with other modules.
  • ...
• /testing/
  • /testing/eslint/
  • /testing/eslint/node_modules/
• /WebGL/
  • /WebGL/module/ (cpp + js)
    • /WebGL/module/cpp/
      • file.cpp
      • /WebGL/module/js/
        • file.js
        • node_modules... unique set of node modules with specific version numbers incompatible with other modules.
        • ...
• /layout/
  • /layout/module/ (js)
    • file.js
    • node_modules... unique set of node modules with specific version numbers incompatible with other modules.
• /mathml/
  • /mathml/module/ (js)
    • file.js
    • node_modules... unique set of node modules with specific version numbers incompatible with other modules.

From the discussion over in #584, I can see your use case. Add some specs testing that it works properly and we can get this merged, assuming @IanVS has no issues with it. 😉

@IanVS: Do you have any issues with this? I can add a fix for the failing .eslintignore test if you would like so that all the specs pass.

I don't have issues with the concept, no. I think we will need to make sure it's very clear what the option is for, because frankly it's a bit non-standard and confusing. I'd like to see an update to the README explaining what it is and the use cases it's intended to address.

@Arcanemagus We should actually probably add a section to the readme for all of the options, what do you think? Perhaps @MikeRatcliffe can start it off with this one, and you and I can expand on the rest afterwards.

I'm using Mike's patched version for Firefox development already and it works as it should. So far so good.

But the user experience, the understanding what the different options are good for and how they interact, is very bad.

First, the package settings are displayed in alphabetical, i.e., essentially random order. It's not obvious that Global Node Installation Path is a customization sub-option for Use Global Installation. The Path to the local node folder is inactive when Use Global is turned on and should be disabled. But it isn't.

Path to the local node_modules folder - should it be a path relative to the project, or absolute? Should it include the node_modules part or not? I needed to figure this out by trial and error. Also, it's a project specific option, but it's global and affects all other projects.

To get a full understanding of the lookup rules, one has to read the source:

1. If Use Global is on, use the global ESLint. The path to it is figured out by running npm get prefix. If this fails for any reason, you can set the global path manually in Global Node Installation Path.
2. If Use Global is off, try to find a local installation in the project folder. Look if there's ESLint in ${PROJECT_ROOT}/node_modules and use it if found.
3. If there is no local installation, use the built-in ESLint in the linter-eslint package itself.

The new option adds a second step to rule 2: in addition to the ${PROJECT_ROOT}/node_modules directory, do lookup in a custom subdirectory - in Firefox case, it's testing/eslint. Therefore, it would make sense if the config.localNodeModules variable was a relative subdirectory, without the node_modules part. The lookup path is then ${PROJECT_ROOT}/${config.localNodeModules}/node_modules. The variable's default value is .

If designed and documented this way, the rules start to make sense and are possible to understand.

@IanVS The documentation could definitely use some work, and as you suggest we might as well start here.

@jsnajdr Atom doesn't support disabling options based on the values of other options, it does support grouping the options and ordering them though, can you file a new issue with your notes on that so it can be dealt with separately?

Your comments on this affecting all projects is one of the reasons I don't really like this, but there is obviously a use for it so it's getting added for the people that need it.

1. Isn't quite that simple, but that works well enough (it actually searches for a node_modules/eslint directory in any parent folder).

I like your idea of making it relative to the project root, @MikeRatcliffe you can get what Atom considers to be the project root from atom.project.relativizePath(filePath)[0] in the provideLinter function. This will return null if the file isn't opened as part of a Project in Atom though so that will need to be handled. This would need to be passed to the worker as part of the job configuration. You could alternatively use the findCached function to try to locate it that way.

I like your idea of making it relative to the project root

I could do that but how about making this option more generic as it will be useful for more users?

I suggest that we change the option to "Ignore other settings and specify the path to eslint's cli.js e.g. /usr/local/lib/node_modules/eslint/lib/cli.js"

I guess we could even disable the other options to make it clear what is happening.

This way eslint can be anywhere on a users system, installed locally or globally, standard or non-standard and the user can still use it. That way if the current system fails to find the path the user still has a way of using it.

@Arcanemagus What do you think?

I guess we could even disable the other options to make it clear what is happening.

Like I said, Atom doesn't support disabling an option. It's either there, or not.

Wouldn't that be keeping it as an absolute path? @jsnajdr made some good points as to why it should be relative to the project, but I suppose a simple call to path.isAbsolute() would tell us if we need to do anything special to make it work if a user enters a relative path.

I don't know this use case well enough to say which approach is better if it can't be made to work with both relative and absolute paths.

Closing, continued in #609.