--fix doesn't fix all problems

[davidjb]

The message from #576 suggests that using --fix will automatically fix problems but not everything gets fixed:

$ standard --version
7.1.2
$ standard WebScreen.js
standard: Use JavaScript Standard Style (http://standardjs.com) Try `standard --fix` to automatically fix problems.
  /path/toApp/Containers/WebScreen.js:2:1: Expected consistent spacing
  /path/toApp/Containers/WebScreen.js:2:31: Multiple spaces found before '}'.
  /path/toApp/Containers/WebScreen.js:3:21: 'NavigationActions' is defined but never used

Running --fix solves the spacing issue, but continues to suggest running --fix to solve the unused import:

$ standard --fix Containers/WebScreen.js
standard: Use JavaScript Standard Style (http://standardjs.com) Try `standard --fix` to automatically fix problems.
  /path/to/App/Containers/WebScreen.js:3:21: 'NavigationActions' is defined but never used

I thought I recall reading somewhere here (another issue?) that not all issues get fixed. So, if fixing unused imports is out-of-scope, the message should avoid suggesting to run --fix -- that was confusing. Alternatively, if the unused import should be removed from my file, then that's not currently happening. Whichever way, the option to remove unused definitions would help even further.

Either way, thanks - the --fix feature is awesome.

[feross]

@davidjb Good suggestion!

I just sent a PR to implement this behavior: standard/standard-engine#122

[davidjb]

@feross thanks for that, that's a great help.

The other bit I've thought about is if someone runs Standard and sees errors, they're told to run --fix. So, it's very likely they'll expect all the issues outputted to get fixed. For example, imagine I'm a first-time user, unfamiliar with Standard and its linting; from the output above:

$ standard WebScreen.js
standard: Use JavaScript Standard Style (http://standardjs.com) Try `standard --fix` to automatically fix problems.
  /path/toApp/Containers/WebScreen.js:2:1: Expected consistent spacing
  /path/toApp/Containers/WebScreen.js:2:31: Multiple spaces found before '}'.
  /path/toApp/Containers/WebScreen.js:3:21: 'NavigationActions' is defined but never used

because of the suggestion given, I'm now expecting all the listed issues would be fixed with --fix.

What about a change in wording (eg Run --fix to automatically fix most problems. For a list of problems that require manual changes see https://standardjs.com/#manual-changes) and docs on the site as to what will/won't be fixed.

Fwiw, at the moment I'm not sure what would or wouldn't be fixed; I can guess a bit from experience and there's http://standardjs.com/index.html#is-there-an-automatic-formatter but that's for a related package. I might have missed something, though so let me know.

[feross]

Good suggestion on the wording, but I don't want to maintain a list of which rules are fixable. It's changing all the time and constantly improving thanks to the ESLint project (which standard uses under-the-hood).

http://standardjs.com/index.html#is-there-an-automatic-formatter

This readme section will be changed once standard v8 is released. You're running a beta version right now.

[davidjb]

Okay thanks. Does eslint have a list that you could link to? Even if it's
the source code they're using to make the choice between fixable and won't
fix would help reduce confusion. Cheers
On Sat, 13 Aug 2016 at 16:50 Feross Aboukhadijeh notifications@github.com
wrote:

Good suggestion on the wording, but I don't want to maintain a list of
which rules are fixable. It's changing all the time and constantly
improving thanks to the ESLint project (which standard uses
under-the-hood).

http://standardjs.com/index.html#is-there-an-automatic-formatter

This readme section will be changed once standard v8 is released. You're
running a beta version right now.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#587 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA9NO8ZvQnI7UoOaD2j0R5hdb6SdOzOLks5qfWlPgaJpZM4Jiuhr
.

[feross]

The fixable rules are on this page, with a wrench icon next their name: http://eslint.org/docs/rules/ Note: Standard doesn't enable every ESLint rule.

[davidjb]

Awesome thanks, that's really helpful for using Standard.

On Sat, 13 Aug 2016 at 17:12 Feross Aboukhadijeh notifications@github.com
wrote:

The fixable rules are on this page, with a wrench icon next their name:
http://eslint.org/docs/rules/ Note: Standard doesn't enable every ESLint
rule.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#587 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA9NOyvwSUKaIUcw-S0b74zKp5o2q5rQks5qfW5tgaJpZM4Jiuhr
.