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

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

Merged
merged 2 commits into from
Jan 23, 2019
Merged

Conversation

kondi
Copy link
Contributor

@kondi kondi commented Jan 19, 2019

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 microsoft/typescript-tslint-plugin#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
Copy link

msftclas commented Jan 19, 2019

CLA assistant check
All CLA requirements met.

@mjbvz
Copy link
Contributor

mjbvz commented Jan 23, 2019

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

@mjbvz mjbvz merged commit 747b02e into microsoft:master Jan 23, 2019
mjbvz added a commit that referenced this pull request Jan 23, 2019
@MathiasKandelborg
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants