Fix #6661. Configurable multiple linters.

[busykai]

Something I started at the hackathon and finished offline.

This PR fixes #6661. It introduces an option to control execution of linters through a configuration file. It uses LanguageLayer implemented in #7889 (#7889 should be merged first). No UI impact.

For any given language languageID, the following set of options can be defined:

"language": {
    "languageID": {
        "linting.prefer": (comma-separated-list-of-linters),
        "linting.preferredOnly": (boolean)
    }
}

linting.prefer is the list of code inspection providers (does not matter whether they are installed or not, does not matter if it is complete or not). if preferredOnly is specified, then only providers from prefer list will be used (those which are installed, of course). preferredOnly will have no effect if prefer is not specified. Test cases are provided which are probably the best documentation.

One could think of prefer as "the list of providers I would like to see first" and usePreferredOnly as of "I would only like to see the providers I prefer".

For example, the following configuration is used in this PR for Brackets:

"language": {
    "javascript": {
        "linting.prefer": "JSLint, JSHint",
        "linting.preferredOnly": true
    }
}

Which would enable both JSLint and JSHint (they will be used in that order).

• remove JSLint override on a JS provider registration
• merge when Async linting lands in master
• change brackets' own .brackets.json to always prefer JSLint

See #6661, #6662

CC: @dangoor, @peterflynn, @ingorichter

[marcelgerber]

I wonder if this should be done via the LanguageLayer

[dangoor]

@SAplayer That's a good point. Since these are fundamentally language-related, maybe it makes sense to group them that way.

The LanguageLayer would be simple to resurrect. What do you think, @busykai?

[marcelgerber]

This won't work with prefer = null and firstOnly = true or sth like that.

[busykai]

well, that's a good point -- i don't think i've added a test case for that, but it should work. when prefer is falsy, list of installed providers will use used (line 180), and then only the first will be picked. i'll add a test case for this (and fix if it does not work), thanks!

[marcelgerber]

Ah, you're right. But a pretty uncommon combination could still break:
prefer = [unavailable provider] (this could cause problems either way...)
preferredOnly = true (so providers is empty)
firstOnly = true (split on an empty object)

[busykai]

Great! This one needs a test and a fix too.

[busykai]

Actually, both work. Just added testcases. Thank you for the idea.

[marcelgerber]

What's the difference between a preferred provider and the others (if preferredOnly = false)? Just the order?

[ingorichter]

You can register multiple inspector for the same language, but if you specify prefer, then you are able to pick from the list of inspectors and only use the preferred ones. If preferredOnly === false, then all registered inspectors will be used.

Currently we have this special handling for javascript inspectors. Once you register a new javascript inspector, the JSLint inspector will be removed. Using the new prefer preference, you will be able to keep JSLint in addition to the other inspectors, but you are able to exclude JSLint by not mentioning in the list of preferred inspectors.

[busykai]

@SAplayer, @dangoor: wrt LanguageLayer -- good idea. if i remember it correctly, it should work well with the prefixed pref system. but... do you want me to restore language layer as part of this PR?

[marcelgerber]

I'd say, we should split the work and first re-implement the LanguageLayer in a separate PR. When it's done, you can merge the changes and do work on this PR.

[busykai]

Ok. Makes sense, there's time. This one should go after async linting anyways. @dangoor, I'll assign resurrection of LanguageLayer to myself then, OK?

[dangoor]

@busykai Sounds good, thanks!

[ingorichter]

@busykai What is the status of this PR? What needs to be done to finish this PR? Thanks!

[busykai]

@ingorichter, it depends on language layer which i'm planning to finish up this week and re-work this one to use it. we can close it so it wouldn't bother (but please don't delete the branch).

[ingorichter]

@busykai I'd like to get this feature into Brackets soon. This would make things so much easier to use for me (working on projects that mainly use JSHint instead of JSLint). I don't always want to install und uninstall JSHint before working on Brackets again. I'm afraid that once we close this PR, we will forget about it. ;-)

[busykai]

@ingorichter, yes. I do want the same thing. We'll make it to Release 42.

[MiguelCastillo]

@TomMalbran Yeah that's my understanding as well. I just think that being explicit about what you want will lead to cleaner/clearer configurations. I would prefer to just write prefer: ['JSLint', 'JSHint', 'ESLint']. And even if the provider does not exist, it can be ignored.

[Mark-Simulacrum]

I also prefer @MiguelCastillo's explicit definition, if defining at all. When not specifying anything (not giving a prefer preference), all linters/providers should be used.

[busykai]

I also think that having a disabled list might help. We now have JSLint installed as a default extension, but if I didn't wanted to use it for any project, I would need to list in the preferred list all the providers that I want to see, making it harder to install new ones, instead of being able to just disable 1.

@TomMalbran, good idea. It could be built on top of this on top of this though.

For the most cases, provider is an extension and can be uninstalled. JSLint is special. Now that we have pre-installed extension, it should probably be done as such as opposed to being a default one. Out of scope of this PR, of course.

I just think that being explicit about what you want will lead to cleaner/clearer configurations. I would prefer to just write prefer: ['JSLint', 'JSHint', 'ESLint']. And even if the provider does not exist, it can be ignored.

@MiguelCastillo, The only case not covered by this one preference scenario is installing a new provider. If we had just prefer and it was in user scope, then a user would not see a newly installed linter being executed. This is not what one would expect, it is also not how it's done now.

You, in turn, can accomplish the desired behaviour by using prefer and userPreferredOnly: true. Just don't forget to update the list when you try a new linter. :)

[MiguelCastillo]

@busykai So your use case of a newly installed extension cleared up for me why you are using the flag. The only thing that the flag does not cover is order in cases where there is more than one provider that is not in the list.

I can actually work on putting together an extension to provide a UI to configure this stuff. I know @Mark-Simulacrum would help me :D Right buddy? It would be a simple dialog listing all providers with a simple checkbox to toggle them on and off... Maybe group them by language. I am not sure about ordering though; dragging would be a nice fancy way to do it, but we can add that later.

[TomMalbran]

@TomMalbran, good idea. It could be built on top of this on top of this though.
For the most cases, provider is an extension and can be uninstalled. JSLint is special. Now that we have pre-installed extension, it should probably be done as such as opposed to being a default one. Out of scope of this PR, of course.

Yes I was thinking of it as a new preference, with lower priority to prefer. So if I have prefer: ["JSLint"], usePreferredOnly: true, disable: ["JSLint"], JSLint will still be the only provider. I was also thinking of the case where you are working on a project that, for example, uses "ESLint" but you don't want to use it for any of your other projects. With this preference you can enable it for that project and disable it for the rest.

The only case not covered by this one preference scenario is installing a new provider. If we had just prefer and it was in user scope, then a user would not see a newly installed linter being executed. This is not what one would expect, it is also not how it's done now.
You, in turn, can accomplish the desired behaviour by using prefer and userPreferredOnly. Just don't forget to update the list when you try a new linter. :)

You can only do that if you intend to use the preference just for sorting, which I don't think that will be the main use for this preference. So if you set usePreferredOnly: true you will not see that anymore. Anyway is strange for people to install many linters for the same language, and maybe even forget that they set a preference for them. If you want to support that scenario, maybe it might be better to have a showAllProviders, which only applies if prefer is not empty and defaults to false. If we change that, I think that enabled might be a better name than prefer.

[busykai]

I would really like to come to a decision on this. I think we can stick to current two-preference set without any harm being done going forward (all the ideas could be done as additional settings). Let's help @ingorichter decide. @TomMalbran, @MiguelCastillo would you agree this PR is 1) mergable, 2) serves its purpose and 3) you don't see something fundamentally flawed that cannot be be extended later on? Simple yes/no does it.

[MiguelCastillo]

I am good with this.

Putting a UI in front of this will definitely help users having to understand the details behind the usePreferedOnly, which is the only point of contention.

[TomMalbran]

1. It is mergeable.
2. It does serve its purpose.
3. The issue is if we want to change the preferences added here later, which is not easy. So it is best if we do them right on the first time. I would like to hear what other people think of the use of the usePreferedOnly preference.

[busykai]

@TomMalbran, in order to help the decision, you need to mention "other people" whose opinion you'd like to hear to invite them and express it. Would you agree?

[TomMalbran]

Yes. It would be nice to hear if others agree with this preferences or prefer others, since I am still not convinced. The current preferences are fine, but maybe we can improve them.

[busykai]

Updated PR description. Here is the modified table based on @ingorichter's above

Preferences

prefer: "<Linter 1>, <Linter 2>, <Linter n>"
usePreferredOnly: [true|false]

prefer	usePreferredOnly	Result
""	false	use every linter registered with Brackets
""	true	use every linter registered with Brackets
"JSHint"	false	use every available linter In Brackets, JSHint will be reported first
"JSHint"	true	use only JSHint. If JSHint is not installed linting would appear with "No linter..." message
"JSHint,AnotherLinter"	false	use every linter in Brackets, JSHint and AnotherLinter first, in that order
"JSHint,AnotherLinter"	true	use only JSHint and AnotherLinter, if any is installed. If none of these two is installed, "No linter..." message
"NotExistLinter"	false	Use every available linter in Brackets
"NotExistLinter"	true	"No linter is available"
"NotExistLinter1,NotExistLinter2"	false	Use every available linter in Brackets
"NotExistLinter1,NotExistLinter2"	true	"No linter is available"

[ingorichter]

I'm glad to see you all improving this feature with your questions, ideas and the constructive feedback. IMHO, this is a great improvement over the existing feature and will give our users control over which linting providers will be used. Even if we don't feel that this is a perfect solution right now, let's get it merged into master and iterate on it when necessary.

[busykai]

Yay! No more suffering over multiple projects with different linters! And less "Fix JSHint error" in Brackets codebase! 👍

[EvHaus]

I'm having issues with this change. After upgrading to 1.1, all Javascript files only execute JSLint, instead of the other linter extensions I have installed.

I put this into my preferences:

"language": {
        "javascript": {
            "linting.prefer": "JSXHint, JSCS",
            "linting.preferredOnly": true
        }
    }

But it seems to have no effect.

Is there a page documenting this change, and how I can configure my preferences to return Brackets to its 1.0 behavior (disabling JSLint and using custom linting extensions)?

[TomMalbran]

@globexdesigns Notice that you need to use an array for the prefer preference:

"language": {
        "javascript": {
            "linting.prefer": ["JSXHint", "JSCS"],
            "linting.usePreferredOnly": true
        }
    }

[MiguelCastillo]

@TomMalbran beat to it :) The problem seems to be with usePreferredOnly

[TomMalbran]

I guess that we should update the description of this issue which doesn't use the final preferences.

[EvHaus]

That did it. Thanks @TomMalbran . It would still be nice to document this somewhere as I expect many others will come here with the same question. Especially since this update changes default behaviour.

[busykai]

It is documented and an example is provided.