Add basic tslint.autoFixOnSave support (#2)

microsoft / vscode-typescript-tslint-plugin

VS Code extension that provides TSLint support using the typescript-tslint-plugin

https://marketplace.visualstudio.com/items?itemName=ms-vscode.vscode-typescript-tslint-plugin

MIT License

188 stars 34 forks source link

Add basic tslint.autoFixOnSave support (#2) #43

Closed kondi closed 5 years ago

kondi commented 5 years ago

This pull request replicates the basic boolean version of tslint.autoFixOnSave config option of vscode-tslint extension.

Quite ugly solution which finds the code action having the exact 'Fix all auto-fixable tslint failures' title. As I understand the current API does not allow a more sophisticated solution, but I am open for implementing other proposal if you have one. At least this one is working now. I have opened a PR in https://github.com/Microsoft/typescript-tslint-plugin/pull/46, if accepted, there will be a nicer solution and will be possible to implement the array version of tslint.autoFixOnSave option too.

Related issues:

msftclas commented 5 years ago

All CLA requirements met.

mjbvz commented 5 years ago

Change looks good overall. Thanks for your work!

However I'd use the Source.fixAll purposed VS Code api instead of onWillSaveTextDocument for a few reasons:

Fix all actions can be triggered without saves.
Users can setup keybindings for fix all actions
We'd have build-in VS Code setting for this instead of having to provide our own:
```
"editor.codeActionsOnSave": {
    "source.autoFix.tslint": true
}
```
codeActionsOnSave runs in a more standard order relative to other on save actions

The downside of fixAll is that it will only work properly in VS Code 1.31+ but I think the trade offs are worth it

I'm going to merge this PR in as-is and just refactor the onWillSaveTextDocument to use Source.fixAll instead

MathiasKandelborg commented 5 years ago

Came here to say how happy I am this is fixed! Note to self: I'll be sure to remember to report bugs & issues on VSCode extensions in the future