Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --fix-to-stdout #53

Merged
merged 4 commits into from
Oct 16, 2016
Merged
Show file tree
Hide file tree
Changes from all 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
38 changes: 38 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ along with an access token in `~/.eslint_d`.

## Editor integration

### Linting

- __Sublime__: Check out [SublimeLinter-contrib-eslint\_d][SublimeLinter].
- __Vim__: Install the [syntastic][] plugin, then make sure this is in your
`.vimrc`:
Expand All @@ -82,6 +84,42 @@ along with an access token in `~/.eslint_d`.

If you're using `eslint_d` in any other editor, please tell me!

### Automatic Fixing

`eslint_d` has an additional flag that `eslint` does not have, `--fix-to-stdout`
which prints the fixed file to stdout. This allows editors to add before save
hooks to automatically fix a file prior to saving. It must be used with
`--stdin`.

- __Emacs__: Add this to your `init.el`, be sure to add hooks for the actual
major modes you use:

```elisp
(defun eslint-fix ()
(interactive)
(setq temp-point (point))
Copy link

Choose a reason for hiding this comment

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

@aaronjensen Why don't you let-bind this variable? It's not a good idea to just setq variables this way.

Copy link

Choose a reason for hiding this comment

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

Actually you could just use save-excursion here, if I'm not mistaken. That'll restore point automatically afterwards. Did you try it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, save-excursion doesn't work w/ shell-command-on-region, that was the first I tried. Good idea on let though.

Copy link

Choose a reason for hiding this comment

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

Ah, too bad, didn't know that.

(shell-command-on-region
;; Region
(point-min)
(point-max)
;; Command
"eslint_d --stdin --fix-to-stdout"
;; Output to current buffer
t
;; Replace buffer
t
;; Error buffer name
"*eslint-fix error*"
;; Display error buffer
t)
;; Refresh syntax highlighting
(font-lock-fontify-buffer)
(goto-char temp-point))

(add-hook 'js-mode-hook
(lambda () (add-hook 'before-save-hook #'eslint-fix)))
```

## Moar speed

If you're really into performance and want the lowest possible latency, talk to
Expand Down
18 changes: 15 additions & 3 deletions lib/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function translateOptions(cliOptions, cwd) {
cache: cliOptions.cache,
cacheFile: cliOptions.cacheFile,
cacheLocation: cliOptions.cacheLocation,
fix: cliOptions.fix,
fix: cliOptions.fix || cliOptions.fixToStdout,
allowInlineConfig: cliOptions.inlineConfig,
printConfig: cliOptions.printConfig,
cwd: cwd
Expand Down Expand Up @@ -49,7 +49,12 @@ module.exports = function (cwd, args, text) {
}))
};
}
var currentOptions = options.parse([0, 0].concat(args));
var currentOptions;
try {
currentOptions = options.parse([0, 0].concat(args));
} catch (e) {
return e.message + '\n# exit 1';
}
cwdDeps.chalk.enabled = currentOptions.color;
var files = currentOptions._;
var stdin = currentOptions.stdin;
Expand All @@ -74,12 +79,19 @@ module.exports = function (cwd, args, text) {

return JSON.stringify(fileConfig, null, ' ');
}
if (currentOptions.fixToStdout && !stdin) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should I check for text here? tbh, the check for text and the fallback to using files below is confusing. It seems stdin should always use stdin, even if the stdin is empty.

Copy link
Owner

Choose a reason for hiding this comment

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

Hm, I see. Checking for text shouldn't be necessary. I agree that the flag should be enough.

return 'The --fix-to-stdout option must be used with --stdin.'
+ '\n# exit 1';
}
var report;
if (stdin && text) {
if (stdin) {
report = engine.executeOnText(text, currentOptions.stdinFilename);
} else {
report = engine.executeOnFiles(files);
}
if (currentOptions.fixToStdout) {
return report.results[0].output || text;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Related to above, there's an edge case here if a user passes --stdin, no text, and more than one file. I should probably at least check for text in the conditional, but that would introduce another edge case... if a user lint fixed a blank file, you'd want it to return a blank file (maybe with a trailing newline? 😄)

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe the new option should only allow a single file if not used with --stdin? This way we don't need to require the --stdin option and one could use the feature to "inspect" what eslint does with --fix without actually modifying the file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is there an easy way to detect a single file? does eslint_d expand globs/directories or does it let eslint do that

Copy link
Owner

Choose a reason for hiding this comment

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

Globs are expanded by eslint. Let's take the easy path then and only support --stdin then. If text is empty, we just do nothing.

}
if (currentOptions.fix) {
cwdDeps.eslint.CLIEngine.outputFixes(report);
}
Expand Down
6 changes: 6 additions & 0 deletions lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@ module.exports = optionator({
default: false,
description: 'Automatically fix problems'
},
{
option: 'fix-to-stdout',
type: 'Boolean',
description: 'Print the fix results to stdout instead of regular output, '
+ 'must be used with --stdin'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should we assume --stdin or just require it?

},
{
option: 'debug',
type: 'Boolean',
Expand Down