Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Conversation

@busykai
Copy link
Contributor

@busykai busykai commented Jan 27, 2014

This pull request introduces an option to keep JSLint active. By default it will not be enabled when there's another linter for JavaScript.

Please, please @peterflynn, @ingorichter take a look at this. It will make life so easier for working on projects which uses different linting requirements!

@busykai
Copy link
Contributor Author

busykai commented Jan 27, 2014

Link to the issue report it fixes: #6661.

@busykai
Copy link
Contributor Author

busykai commented Jan 27, 2014

P.S. @dangoor new preferences is a pandora's box.

@dangoor
Copy link
Contributor

dangoor commented Jan 27, 2014

@busykai I'd argue that unless you're building a tool for a very prescriptive environment, preferences are very important whereas is most typical apps they're a pandora's box. We'll see if I'm right :)

Regarding this change, it seems like it would be better if the user could explicitly state that they want to disable (or, instead, enable) some linter or set of linters. JSLint should not be treated as special, I think.

@busykai
Copy link
Contributor Author

busykai commented Jan 27, 2014

@dangoor, i agree completely it should not be treated specially, but it was already... and having to uninstall jshint and reload each time you need to work on brackets code (or any other using JSLint) is unbearable.

my intention is to have it in place until the definitive solution to this is found. basically, make it liveable for a duration of the next sprint.

@busykai
Copy link
Contributor Author

busykai commented Jan 27, 2014

perhaps it makes sense instead of treating JSLint as a special case, enable (or disable) providers by name. always keeping the registered providers, but using only the ones enabled based on settings. it sounds like a nicer option and could potentially stay as a long-term solution. what do you think?

@peterflynn
Copy link
Member

@busykai Yeah, I agree with @dangoor -- and I think we should avoid exposing new preferences to users if we plan to remove them shortly afterward. I like the idea of allowing enabling/disabling all lint providers generally via preferences first, and waiting to add UI for it later.

One small issue there is that lint providers currently provide only a user-visible name, which is localization-sensitive. We should probably add an id field to the registration object, and only use name for backwards compatibility when it's not specified.

We should also specify a best practice for how extensions like JSHint will still disable the built-in JSLint by default (since presumably most users won't want both going at once). Maybe the rule should be that, if no enablement pref is explicitly set for JSLint, we enable it iff no other JS linter is registered.

@busykai
Copy link
Contributor Author

busykai commented Jan 28, 2014

@peterflynn @dangoor makes sense. thanks for pointing out the potential localization problem.

i think it will be a nice way to manage which linter to run. i like the idea of disabling default providers w/o persisting it in absence of explicit enabling rule. that would mean, though that there should be "enable" rule and not disable.

if you agree that's the long-term plan on how multiple providers should be handled, then i'll make the change. this one can be closed. would you agree?

@busykai
Copy link
Contributor Author

busykai commented Jan 28, 2014

closing this one, another PR is to follow soon as per discussion.

@busykai busykai closed this Jan 28, 2014
@dangoor
Copy link
Contributor

dangoor commented Jan 28, 2014

Perhaps linters should explicitly register with CodeInspection which other linters should be disabled unless otherwise set by a pref. I mention this because JSHint recently split in two: one project for error checking and another for style checking. So, it's quite reasonable to want both of those to work together, but not with JSLint.

@busykai
Copy link
Contributor Author

busykai commented Jan 28, 2014

I would rather not delegate this to linters. I was thinking the following:

  • Brackets provides certain set of linters (currently it's just JSLint) by default.
  • Default linters are not manageable
  • User can install linters as extensions
  • User can manage the set of extensions he/she installs.

With that in mind, I was thinking the configuration stategy should be the following: specify preferred provider list and specify whether to use the first provider only (it will use first available from the list) and whether others registered providers should be supressed.

It would preserve default behavior and will allow greater flexibility as to per-project configuration, e.g. for Brackets the configuration will be (property names are not illustrative):

{
    "linting.providers": {
        "javascript": {
            "prefer": "JSLint,JSHint",
            "useFirstOnly" true,
            "supressOthers": true
        }
    }
}

@busykai
Copy link
Contributor Author

busykai commented Jan 28, 2014

@peterflynn @dangoor and thinking it a little bit further all these stunts are due to the fact that JSLint is "built-in" in Brackets. taking it off would pretty much simplify the configuration options. i understand it is there for a reason, but it might be worth considering distributing it as a stand-alone extension.

it would allow to simply configuration to just options prefer and useFirstFound. In the majority of cases (given useFirstFound defaults to true), it will be prefer only.

BTW, JHint is split into two starting 3.x series, I believe. Brackets JSHint extension did not yet switch to it.

@dangoor
Copy link
Contributor

dangoor commented Jan 28, 2014

I like your configuration example, because that would still support the case of using JSHint+JSCS.

What I was suggesting was that linter extensions could help provide sane default behavior. Using JSLint and JSHint together does not make much sense as default behavior and everyone would be forced to do some configuration to get what they want. It's reasonable to assume that if someone installs JSHint then they're not also looking to use JSLint.

Of course, the user can still configure as you suggest, which would allow them to use JSLint on some projects and not on others.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants