Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Organize config options #1042

Merged
merged 18 commits into from
Feb 13, 2018
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ eslint --init
```

Alternatively you can create the `.eslintrc` file by yourself. It is a good
idea to have a look at the [Get Started With ESLint](http://devnull.guru/get-started-with-eslint/)
blog post by [IanVS](https://github.com/IanVS) and [the ESLint documentation](http://eslint.org/docs/user-guide/configuring),
idea to have a look at the [ESLint documentation](http://eslint.org/docs/user-guide/configuring),
including the [list of rules](http://eslint.org/docs/rules/).

## A Note About Settings
Expand Down
Binary file removed docs/images/fork.png
Binary file not shown.
Binary file removed docs/images/pr-button.png
Binary file not shown.
Binary file removed docs/images/pr.png
Binary file not shown.
Binary file removed docs/images/rule-link.png
Binary file not shown.
213 changes: 128 additions & 85 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,72 +9,6 @@
"atom": ">=1.13.0 <2.0.0"
},
"configSchema": {
"lintHtmlFiles": {
"title": "Lint HTML Files",
"description": "You should also add `eslint-plugin-html` to your .eslintrc plugins",
"type": "boolean",
"default": false
},
"useGlobalEslint": {
"title": "Use global ESLint installation",
"description": "Make sure you have it in your $PATH",
"type": "boolean",
"default": false
},
"showRuleIdInMessage": {
"title": "Show Rule ID in Messages",
"type": "boolean",
"default": true
},
"disableWhenNoEslintConfig": {
"title": "Disable when no ESLint config is found (in package.json or .eslintrc)",
"type": "boolean",
"default": true
},
"eslintrcPath": {
"title": ".eslintrc Path",
"description": "It will only be used when there's no config file in project",
"type": "string",
"default": ""
},
"globalNodePath": {
"title": "Global Node Installation Path",
"description": "Write the value of `npm get prefix` here",
"type": "string",
"default": ""
},
"advancedLocalNodeModules": {
"title": "Path to the local node_modules folder",
"description": "Optionally specify the path to the local node_modules folder",
"type": "string",
"default": ""
},
"eslintRulesDirs": {
"title": "ESLint Rules Directories",
"description": "Specify a comma separated list of directories for ESLint to load rules from.",
"type": "array",
"default": [],
"items": {
"type": "string"
}
},
"disableEslintIgnore": {
"title": "Don't use .eslintignore files",
"type": "boolean",
"default": false
},
"disableFSCache": {
"title": "Disable FileSystem Cache",
"description": "Paths of node_modules, .eslintignore and others are cached",
"type": "boolean",
"default": false
},
"fixOnSave": {
"title": "Fix errors on save",
"description": "Have eslint attempt to fix some errors automatically when saving the file.",
"type": "boolean",
"default": false
},
"scopes": {
"title": "List of scopes to run ESLint on, run `Editor: Log Cursor Scope` to determine the scopes for a file.",
"type": "array",
Expand All @@ -87,31 +21,140 @@
],
"items": {
"type": "string"
},
"order": 1
},
"lintHtmlFiles": {
"title": "Lint HTML Files",
"description": "You should also add `eslint-plugin-html` to your .eslintrc plugins",
"type": "boolean",
"default": false,
"order": 2
},
"autofix": {
"type": "object",
"order": 3,
"title": "Automatic Fixes",
"properties": {
"fixOnSave": {
"title": "Fix errors on save",
"description": "Have eslint attempt to fix some errors automatically when saving the file.",
"type": "boolean",
"default": false,
"order": 1
},
"ignoreFixableRulesWhileTyping": {
"title": "Ignore fixable rules while typing",
"description": "Have the linter ignore all fixable rules during linting when editing a document. The list is automatically updated on each lint job, and requires at least one run to be populated. Only supported when using ESLint v4+.",
"type": "boolean",
"default": false,
"order": 2
},
"rulesToDisableWhileFixing": {
"title": "Disable specific rules from fixes",
"description": "Prevent rules from being auto-fixed by ESLint. Applies to fixes made during saves as well as when running the `Linter Eslint: Fix File` command.",
"type": "array",
"default": [],
"items": {
"type": "string"
},
"order": 3
}
}
},
"rulesToSilenceWhileTyping": {
"title": "Silence specific rules while typing",
"description": "Useful when Atom fixes errors on save like `no-trailing-spaces` or `eol-last`.",
"type": "array",
"default": [],
"items": {
"type": "string"
"global": {
"type": "object",
"order": 4,
"title": "Global ESLint",
"properties": {
"useGlobalEslint": {
"title": "Use global ESLint installation",
"description": "Make sure you have it in your $PATH",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add something here to emphasize that local installation is the suggested usage?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned to @Arcanemagus in one of the previous comments, I'd like to save wording tweaks for another PR. There is a lot that I would like to do to help explain and clarify our settings, but this PR is really just about reorganizing them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. 👍

"type": "boolean",
"default": false,
"order": 1
},
"eslintrcPath": {
"title": ".eslintrc Path",
"description": "It will only be used when there's no config file in project",
"type": "string",
"default": "",
"order": 4
},
"globalNodePath": {
"title": "Global Node Installation Path",
"description": "Write the value of `npm get prefix` here",
"type": "string",
"default": "",
"order": 2
}
}
},
"rulesToDisableWhileFixing": {
"title": "Disable specific rules from fixes",
"description": "Prevent rules from being auto-fixed by ESLint. Applies to fixes made during saves as well as when running the `Linter Eslint: Fix File` command.",
"type": "array",
"default": [],
"items": {
"type": "string"
"disabling": {
"type": "object",
"order": 5,
"properties": {
"disableWhenNoEslintConfig": {
"title": "Disable when no ESLint config is found (in package.json or .eslintrc)",
"type": "boolean",
"default": true,
"order": 1
},
"rulesToSilenceWhileTyping": {
"title": "Silence specific rules while typing",
"description": "Useful when Atom fixes errors on save like `no-trailing-spaces` or `eol-last`.",
"type": "array",
"default": [],
"items": {
"type": "string"
},
"order": 2
}
}
},
"ignoreFixableRulesWhileTyping": {
"title": "Ignore fixable rules while typing",
"description": "Have the linter ignore all fixable rules during linting when editing a document. The list is automatically updated on each lint job, and requires at least one run to be populated. Only supported when using ESLint v4+.",
"type": "boolean",
"default": false
"advanced": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add a collapsed attribute to make this section collapsed by default.

Although it doesn't seem to be documented, it's tested in the specs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed atom/atom#16030 about getting it officially documented 😉.

"type": "object",
"collapsed": true,
"title": "Uncommon",
"order": 6,
"properties": {
"disableEslintIgnore": {
"title": "Don't use .eslintignore files",
"type": "boolean",
"default": false,
"order": 1
},
"disableFSCache": {
"title": "Disable FileSystem Cache",
"description": "Paths of node_modules, .eslintignore and others are normally cached",
"type": "boolean",
"default": false,
"order": 2
},
"showRuleIdInMessage": {
Copy link
Member

@Arcanemagus Arcanemagus Oct 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗒 One could make the argument this should go in the "general uncategorized" area, but I think it's fine leaving this down here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really understand why someone would want to turn that off. Do you know off-hand the reason for the option to exist at all?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make the output cleaner and easier to read at the cost of being much harder to look up rules in the docs or write inline disable rules. I would never turn this off, but I can see the motivation. I think it's reasonable to argue for removing it entirely.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it was added in 72cc3cb, I was hoping for a bit more context as to why, but no luck.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly he wanted to remove it completely when doing the rewrite, but I made him keep it around (this was before the rule ID even linked to the relevant page, so it was very helpful for googleing an error). The compromise was an option that defaulted enabled.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I said you could argue to remove it, I meant you could remove the setting. Removing the rule id for everybody would be incredibly mean.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh personally I would be for removing the setting, but there are people out there that don't like the "noise".

"title": "Show Rule ID in Messages",
"type": "boolean",
"default": true,
"order": 3
},
"eslintRulesDirs": {
"title": "ESLint Rules Directories",
"description": "Specify a comma separated list of directories for ESLint to load rules from.",
"type": "array",
"default": [],
"items": {
"type": "string"
},
"order": 4
},
"advancedLocalNodeModules": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Since we are already migrating this to a new key, what about renaming it to just localNodeModules?

"title": "Path to the local node_modules folder",
"description": "Optionally specify the path to the local node_modules folder",
"type": "string",
"default": "",
"order": 5
}
}
}
},
"scripts": {
Expand Down
34 changes: 17 additions & 17 deletions spec/linter-eslint-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ describe('The eslint provider for Linter', () => {
const { lint } = linterProvider

beforeEach(async () => {
atom.config.set('linter-eslint.disableFSCache', false)
atom.config.set('linter-eslint.disableEslintIgnore', true)
atom.config.set('linter-eslint.advanced.disableFSCache', false)
atom.config.set('linter-eslint.advanced.disableEslintIgnore', true)

// Activate the JavaScript language so Atom knows what the files are
await atom.packages.activatePackage('language-javascript')
Expand Down Expand Up @@ -206,7 +206,7 @@ describe('The eslint provider for Linter', () => {

describe('when a file is specified in an .eslintignore file', () => {
beforeEach(() => {
atom.config.set('linter-eslint.disableEslintIgnore', false)
atom.config.set('linter-eslint.advanced.disableEslintIgnore', false)
})

it('will not give warnings when linting the file', async () => {
Expand All @@ -233,7 +233,7 @@ describe('The eslint provider for Linter', () => {
const tempDir = path.dirname(tempPath)

const editor = await atom.workspace.open(tempPath)
atom.config.set('linter-eslint.disableEslintIgnore', false)
atom.config.set('linter-eslint.advanced.disableEslintIgnore', false)
await copyFileToDir(path.join(paths.eslintignoreDir, '.eslintrc.yaml'), tempDir)

const messages = await lint(editor)
Expand All @@ -244,7 +244,7 @@ describe('The eslint provider for Linter', () => {

describe('when a file is specified in an eslintIgnore key in package.json', () => {
it('will still lint the file if an .eslintignore file is present', async () => {
atom.config.set('linter-eslint.disableEslintIgnore', false)
atom.config.set('linter-eslint.advanced.disableEslintIgnore', false)
const editor = await atom.workspace.open(path.join(paths.eslintIgnoreKeyDir, 'ignored.js'))
const messages = await lint(editor)

Expand All @@ -256,7 +256,7 @@ describe('The eslint provider for Linter', () => {
const tempDir = path.dirname(tempPath)

const editor = await atom.workspace.open(tempPath)
atom.config.set('linter-eslint.disableEslintIgnore', false)
atom.config.set('linter-eslint.advanced.disableEslintIgnore', false)
await copyFileToDir(path.join(paths.eslintIgnoreKeyDir, 'package.json'), tempDir)

const messages = await lint(editor)
Expand Down Expand Up @@ -298,7 +298,7 @@ describe('The eslint provider for Linter', () => {
})

it('should not fix linting errors for rules that are disabled with rulesToDisableWhileFixing', async () => {
atom.config.set('linter-eslint.rulesToDisableWhileFixing', ['semi'])
atom.config.set('linter-eslint.autofix.rulesToDisableWhileFixing', ['semi'])

await firstLint(editor)
await makeFixes(editor)
Expand Down Expand Up @@ -382,8 +382,8 @@ describe('The eslint provider for Linter', () => {
}

it('does nothing on saved files', async () => {
atom.config.set('linter-eslint.rulesToSilenceWhileTyping', ['no-trailing-spaces'])
atom.config.set('linter-eslint.ignoreFixableRulesWhileTyping', true)
atom.config.set('linter-eslint.disabling.rulesToSilenceWhileTyping', ['no-trailing-spaces'])
atom.config.set('linter-eslint.autofix.ignoreFixableRulesWhileTyping', true)
expectedPath = paths.modifiedIgnoreSpace
const editor = await atom.workspace.open(expectedPath)
// Run once to populate the fixable rules list
Expand All @@ -409,7 +409,7 @@ describe('The eslint provider for Linter', () => {
checkNew(messages)

// Enable the option under test
atom.config.set('linter-eslint.rulesToSilenceWhileTyping', ['no-trailing-spaces'])
atom.config.set('linter-eslint.disabling.rulesToSilenceWhileTyping', ['no-trailing-spaces'])

// Check the lint results
const newMessages = await lint(editor)
Expand All @@ -433,7 +433,7 @@ describe('The eslint provider for Linter', () => {

// Enable the option under test
// NOTE: Depends on no-trailing-spaces being marked as fixable by ESLint
atom.config.set('linter-eslint.ignoreFixableRulesWhileTyping', true)
atom.config.set('linter-eslint.autofix.ignoreFixableRulesWhileTyping', true)

// Check the lint results
const newMessages = await lint(editor)
Expand All @@ -459,7 +459,7 @@ describe('The eslint provider for Linter', () => {

// Enable the option under test
// NOTE: Depends on mport/newline-after-import rule being marked as fixable
atom.config.set('linter-eslint.ignoreFixableRulesWhileTyping', true)
atom.config.set('linter-eslint.autofix.ignoreFixableRulesWhileTyping', true)

// Check the lint results
const newMessages = await lint(editor)
Expand Down Expand Up @@ -516,7 +516,7 @@ describe('The eslint provider for Linter', () => {
let tempFixtureDir

beforeEach(async () => {
atom.config.set('linter-eslint.disableWhenNoEslintConfig', false)
atom.config.set('linter-eslint.disabling.disableWhenNoEslintConfig', false)

tempFilePath = await copyFileToTempDir(paths.badInline)
editor = await atom.workspace.open(tempFilePath)
Expand Down Expand Up @@ -549,7 +549,7 @@ describe('The eslint provider for Linter', () => {
let tempFixtureDir

beforeEach(async () => {
atom.config.set('linter-eslint.disableWhenNoEslintConfig', true)
atom.config.set('linter-eslint.disabling.disableWhenNoEslintConfig', true)

const tempFilePath = await copyFileToTempDir(paths.badInline)
editor = await atom.workspace.open(tempFilePath)
Expand All @@ -571,7 +571,7 @@ describe('The eslint provider for Linter', () => {
it('works when the cache fails', async () => {
// Ensure the cache is enabled, since we will be taking advantage of
// a failing in it's operation
atom.config.set('linter-eslint.disableFSCache', false)
atom.config.set('linter-eslint.advanced.disableFSCache', false)
const fooPath = path.join(paths.badCache, 'temp', 'foo.js')
const newConfigPath = path.join(paths.badCache, 'temp', '.eslintrc.js')
const editor = await atom.workspace.open(fooPath)
Expand Down Expand Up @@ -658,7 +658,7 @@ describe('The eslint provider for Linter', () => {
const expectedUrl = 'https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-unresolved.md'

it('shows the rule ID when enabled', async () => {
atom.config.set('linter-eslint.showRuleIdInMessage', true)
atom.config.set('linter-eslint.advanced.showRuleIdInMessage', true)
const editor = await atom.workspace.open(paths.badImport)
const messages = await lint(editor)
const expected = "Unable to resolve path to module '../nonexistent'. (import/no-unresolved)"
Expand All @@ -673,7 +673,7 @@ describe('The eslint provider for Linter', () => {
})

it("doesn't show the rule ID when disabled", async () => {
atom.config.set('linter-eslint.showRuleIdInMessage', false)
atom.config.set('linter-eslint.advanced.showRuleIdInMessage', false)
const editor = await atom.workspace.open(paths.badImport)
const messages = await lint(editor)
const expected = "Unable to resolve path to module '../nonexistent'."
Expand Down
Loading