Organize config options (#1042)

* Remove unused docs images * Remove mention of blog post It’s out-of-date, and not actually even running right now. * Rearrange and order settings By default, Atom will sort settings alphabetically, which can result in a really strange, non-logical order. This commit attempts to put more important or commonly used settings up top, while also grouping similar settings together. So, the scopes are together with the lint html setting, the use global and global paths are together, the fixing options are grouped, etc. I’m sure this isn’t perfect, but it’s a start. * Group disabling options together This creates an options group for settings that disable linting in one way or another. Because moving options into groups will require a lot of migrations, this also creates a new file/function to handle those migrations, thus keeping them out of `main.js`. There may be a more elegant way to manage the migrations, but this gets us started. * Streamline config setting migrations For standard migrations, all we really need to do is check if the old setting is defined, then move it to the new setting, then unset the old setting. We can just make a simple loop for this, keeping the old and new setting strings as data, apart from the logic that moves them. * Group “advanced” settings These are settings that are not commonly needed, so we may as well try to avoid confusing new users by indicating that they are “advanced” * Group global eslint options together * Rearrange remaining options * Wording tweaks * Move `eslintrcPath` setting to `global` section * Move eslintRulesDirs to advanced section And also collapse advanced section. * Create an `autofix` section * Do a single `atom.config.get` * Refine migration approach slightly * Avoid object rest spread (we don’t have babel configured for it) * Rename advancedLocalNodeModules to localNodeModules Since it has a prefix of `advanced` now anyways. * Make “called” method of spy return a boolean I believe this is what was intended originally. * Fix migrate-config-options and add specs
AtomLinter · Feb 13, 2018 · 0eb16c4 · 0eb16c4
1 parent 1427e52
commit 0eb16c4
Show file tree

Hide file tree

Showing 14 changed files with 365 additions and 165 deletions.
diff --git a/README.md b/README.md
@@ -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

diff --git a/docs/images/fork.png b/docs/images/fork.png
diff --git a/docs/images/pr-button.png b/docs/images/pr-button.png
diff --git a/docs/images/pr.png b/docs/images/pr.png
diff --git a/docs/images/rule-link.png b/docs/images/rule-link.png
diff --git a/package.json b/package.json
@@ -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",
@@ -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",
+          "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": {
+      "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": {
+          "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
+        },
+        "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": {

diff --git a/spec/linter-eslint-spec.js b/spec/linter-eslint-spec.js
@@ -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')
@@ -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 () => {
@@ -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)
@@ -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)
 
@@ -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)
@@ -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)
@@ -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
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)"
@@ -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'."

diff --git a/spec/make-spy.js b/spec/make-spy.js
@@ -17,6 +17,6 @@ export default (returnValue) => {
   return {
     call,
     calledWith,
-    called: () => calledWith.length
+    called: () => Boolean(calledWith.length)
   }
 }