steelbrain / linter

A Base Linter with Cow Powers http://steelbrain.me/linter/
MIT License
1.1k stars 178 forks source link

Add Option to Enable/Disable a Specific Linter #1165

Closed fny closed 7 years ago

fny commented 8 years ago

It would be immensely to be able to toggle specific linters manually when dealing with JSLint/JSHint/ESLint/JSCS across different projects. In my opinion, SublimeLinter has a good reference implementation:

Select enable or disable from the menu:

screen shot 2016-05-30 at 9 14 34 am

Select linter to disable:

screen shot 2016-05-30 at 9 19 16 am
steelbrain commented 8 years ago

Unfortunately, Atom linter can't do that, mostly because the name property is optional and it won't know how to distinguish between linters without it :(

fny commented 8 years ago

Is this something worth pushing for as a requirement for v2? I wrote a quick script just to see how many packages don't provide a name, and it turns out to be about 50% percent. Full output is below. I could post issues to all the repositories that are missing the value. Also, couldn't we always just list the linters without names as "Unknown 1", "Unknown 2" or include the grammar scope as well to better identify the linter ("Unknown 1: source.java")

Found for linter-ansible-linting:       name: 'Ansible',
Found for linter-ansible-syntax:       name: 'Ansible',
No linter name specified for linter-liferay
No linter name specified for linter-shellcheck
Found for linter-bootlint:       name: 'bootlint',
Found for linter-clang:       name: "clang",
Found for linter-cppcheck:       name: 'Cppcheck',
Found for linter-gcc:       name: 'GCC'
No linter name specified for linter-moose
Found for linter-emscripten:       name: "Emscripten",
Found for linter-clang:       name: "clang",
Found for linter-cppcheck:       name: 'Cppcheck',
Found for linter-gcc:       name: 'GCC'
No linter name specified for linter-moose
Found for linter-emscripten:       name: "Emscripten",
No linter name specified for linter-cspm
Found for linter-csslint:       name: 'CSSLint'
No linter name specified for linter-liferay
Found for linter-stylelint:     name: 'stylelint',
Found for linter-stylelint:         codeFilename: filePath,
No linter name specified for linter-clojure
No linter name specified for linter-coffeelint
No linter name specified for linter-coffeescript
Found for linter-cflint:       name: 'cflint',
No linter name specified for linter-crystal
Found for linter-elixirc:       name: 'Elixir'
No linter name specified for linter-elm-make
Found for linter-erb:       name: 'ERB',
No linter name specified for linter-erlang
Found for linter-gfortran:             name: "gfortran",
Found for linter-glsl:     baseFilename: baseFilename,
Found for linter-glsl:     outFilename: outFilename,
Found for linter-glsl:     fullFilename: shaderFilename
Found for linter-glsl:       name: "glsl",
Found for linter-golinter:       name: 'Golint'
Found for linter-tidy:       name: 'tidy'
Found for linter-htmlhint:     name: 'htmlhint',
No linter name specified for linter-liferay
Found for linter-vnu:     name: "v.Nu",
No linter name specified for linter-haml
No linter name specified for linter-handlebars
No linter name specified for linter-liferay
No linter name specified for linter-hlint
No linter name specified for linter-hdevtools
Found for linter-jsonlint:     name: 'JSON Lint',
No linter name specified for linter-package-json-validator
No linter name specified for linter-jade
No linter name specified for linter-javac
Found for linter-jshint:       name: 'JSHint',
Found for linter-jscs:       name: 'JSCS',
Found for linter-eslint:       name: 'ESLint',
Found for flow-ide:       name: 'Flow IDE',
Found for linter-flow:       name: 'Flow',
No linter name specified for linter-js-standard
No linter name specified for linter-liferay
Found for linter-gjslint:       name: 'gjslint'
No linter name specified for linter-jolie
Found for linter-chktex:       name: 'chktex'
No linter name specified for linter-lsc
No linter name specified for linter-lua
No linter name specified for linter-luacheck
No linter name specified for linter-lua-findglobals
No linter name specified for linter-glua
No linter name specified for linter-luaparse
No linter name specified for linter-matlab
Found for linter-markdownlint:       name: 'markdownlint'
Found for linter-markdown:     name: 'remark-lint',
No linter name specified for linter-node-markdownlint
No linter name specified for linter-moonscript
Found for linter-clang:       name: "clang",
Found for linter-emscripten:       name: "Emscripten",
Found for linter-clang:       name: "clang",
Found for linter-emscripten:       name: "Emscripten",
Found for linter-php:       name: 'PHP'
Found for linter-phpcs:       name: 'PHPCS'
Found for linter-phpmd:       name: 'PHPMD'
Found for linter-perlcritic:       name: 'perlcritic'
No linter name specified for linter-perl
No linter name specified for linter-processing
No linter name specified for linter-prolog
No linter name specified for linter-protocol-buffer
Found for linter-puppet-lint:       name: 'Puppet-Lint',
No linter name specified for linter-puppet-parse
No linter name specified for linter-puppet-parser
Found for linter-puppet-parsing:       name: 'Puppet',
Found for linter-pylint:       name: 'Pylint'
Found for linter-pep8:       name: 'pep8'
No linter name specified for linter-pydocstyle
Found for linter-flake8:       name: 'Flake8'
No linter name specified for linter-pylama
Found for linter-python:             name: 'Python Linter',
Found for linter-lintr:       name: 'lintr'
Found for linter-rubocop:   name: 'RuboCop'
Found for linter-ruby:       name: "Ruby",
Found for linter-reek:       name: 'reek'
No linter name specified for linter-ruby-reek
Found for linter-erb:       name: 'ERB',
Found for linter-rust:     cargoManifestFilename:
Found for linter-rust:       name: 'Rust'
Found for linter-scss-lint:       name: 'scss-lint'
No linter name specified for linter-liferay
Found for linter-sass-lint:       name: 'sass-lint'
Found for linter-sass-lint:               filename: filePath
Found for linter-scss-lint:       name: 'scss-lint'
No linter name specified for linter-9e-sass
Found for linter-sass-lint:       name: 'sass-lint'
Found for linter-sass-lint:               filename: filePath
Found for linter-scalac:       name: 'scalac',
Found for linter-scalastyle:       name: 'scalastyle',
No linter name specified for linter-swiftc
No linter name specified for linter-swiftlint
Found for linter-swift-package-manager:       name: 'linter-swift-package-manager'
Found for linter-twig:       name: 'Twig'
No linter name specified for linter-tslint
Found for linter-xmllint:       name: 'xmllint'
Found for linter-jing:       name: 'Jing',
Found for linter-js-yaml:       name: 'Js-YAML',
Found for linter-js-yaml:             filename: path.basename(filePath),
No linter name specified for linter-less
No linter name specified for linter-stylint
steelbrain commented 8 years ago

Sounds fair for a v2

steelbrain commented 8 years ago

Also just for clarification, I meant requiring a name property sounds good for v2, I am still not sure why you wouldn't want to just disable the package that provides that linter and want linter to disable it for you

6zz commented 7 years ago

current workaround is going to packcage settings enable and disable the different linters I guess?

steelbrain commented 7 years ago

@6zz anything wrong with disabling the package? I personally think it's pretty fair given the fact that you want to disable a linter, a linter is a package so you basically disable a package.

Arcanemagus commented 7 years ago

Also, all of the linters mentioned in the original request at least have the option to not run when they can't find a configuration file. Enable that and I'd imagine that solves most cases this would be for.

6zz commented 7 years ago

@steelbrain the downside with that is when I switch to a project that requires eslint, I'd have to make sure I remember to go re-enable it. And @Arcanemagus just said what I wanted to say. It would be fantastic if a project contains .jshintrc, then linter-jshint automatically kicks in, and if .eslintrc exists then linter-eslint kicks in.

Arcanemagus commented 7 years ago

@6zz linter-eslint and linter-jshint already have options for that, just enable them and you should be fine.

6zz commented 7 years ago

what?! I miss read what you said earlier. Thanks. This request now is no longer necessary.

6zz commented 7 years ago

@Arcanemagus I have checked "Disable when no ESLint config is found (in package.json or .eslintrc) but it is still running it. It appears if I have enabled "use global ESLint installation" it negates the prior config

Arcanemagus commented 7 years ago

@6zz it's likely you have an eslint configuration in some parent directory: It will search up to the drive root, combining any configurations it finds. If you aren't able to figure out what is going on please file an issue over there.

6zz commented 7 years ago

@Arcanemagus thanks. yes I do have a global .eslintrc. so I have to disable "use global ESLint installation" :(

dmytrokyrychuk commented 7 years ago

@steelbrain

a linter is a package so you basically disable a package

This may not be true for all packages. A linter may be just one of the services that a package provides. Hypothetically, there might be a package that provides a useful toolbar and autocomplete services, but a very noisy linter service. Say, a user just got assigned to a legacy project that was not written with any style guide in mind, so that the linter service becomes noisy due to a number of, for instance, common code style violations, but other services provided by the package (toolbar and autocomplete) would still be helpful during the project development, so the user would like to disable just this one noisy linter service temporarily, so he could see other (probably more valuable) linter messages, but still have access to the toolbar and autocomplete.

steelbrain commented 7 years ago

@orgkhnargh as someone who has recently written a package that does autocompletion, linting and shows code coverage in the status bar, I know where you're coming from

The ideal solution that I followed or facebook follows with some of their packages (that I agree with) is that the package that provides all of these services has configurations, configurations like Enable linter support, Enable autocomplete support that allow the user to specifically enable what they want from the package.

But if you still insist on there should be an option to disable individual linter providers (I want to do this) then please do recommend a way to handle the UI. We cannot put the UI in settings view because it expects static options and the list of linter providers is ever-growing, I wasn't able to find a good user-friendly alternative way to provide this configuration

dmytrokyrychuk commented 7 years ago

@steelbrain don't get me wrong, I don't insist on this feature to be implemented. As a fellow developer I realize that every new feature increases the cost of maintenance. I just thought a valid use case might help when deciding whether to implement this feature.

I see nothing wrong with using the settings UI for this (at least at first; custom UI may be implemented later). You only need to store a list of providers names. It is possible to store them in atom contig as an array of strings. Atom itself does that for:

Alternatively, a UI similar to Command Palette could be used. That could be a pop-up with a searchable list of providers, with a checkbox near every provider. This is close to what @fny has shown on the Sublime Text screenshots above.

Another idea is to make it similar to the Installed Packages section of the Settings view, where each provider would have a card with its name, version, author picture, description, etc. This one would require too much work, I don't suppose someone would be willing to implement it.

steelbrain commented 7 years ago

@orgkhnargh I didn't intend to come off as hostile, I've thought about it for some time. We've already made name a required field in linter v2 so that's done

We cannot make it work in settings view for the reasons I mentioned above (it expects static settings from external packages). The way I see it, we have two possible options to deal with this

  1. Introduce a string settings where users can specify comma-separated list of packages to disable, we won't be able to assist the users in it by autocompletion of package names or anything, kinda like eslint, flow-ide, linter-puppet-lint

  2. We use a horrible looking but working command pallete like view to show the users list of packages that they can choose to enable/disable from (thanks for the idea!)

I'll let the community vote on which one it wants because I am fine with either (though the 1st one is a little bit inconvenient and has a better error ratio because the name of the package isn't necessarily the name of the exported linter

steelbrain commented 7 years ago

Just so we are on the same page, does the community want the packages to be disabled using the command palette per project or global?

steelbrain commented 7 years ago

I've implemented this in #1341. Here's how it looks like in action, had to reduce gif quality because of max filesize limits on GitHub

Linter Toggle in action