-
Notifications
You must be signed in to change notification settings - Fork 14
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
Suggest autofixes for declaration diagnostics #30
Suggest autofixes for declaration diagnostics #30
Conversation
@@ -112,16 +112,16 @@ export const checkOptions = async (opt: Options): Promise<[string[], string[]]> | |||
|
|||
// Check git status if the write flag was provided and the output folder is the same as the project folder | |||
// Do not allow overwriting files with previous changes on them unless --ignoreGitStatus flag was provided | |||
if (opt.write && path.dirname(opt.tsconfig) === opt.outputFolder) { | |||
if (!opt.ignoreGitStatus && opt.write && path.dirname(opt.tsconfig) === opt.outputFolder) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I noticed this one when I tried ts-fix last.
@microsoft-github-policy-service agree company="Airtable" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest running a code formatter over this repo. Too invasive of a change to include in this PR, but very tempting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM though most of the diff is unrelated stuff 😄
Gotta sneak these in somehow 😉 (since typo fix PRs tend to look like spam) |
CI doesn't like this PR, it's probably not your fault, though. I'll look later. |
@MichaelMitchell-at, I tried this branch but still can't get fixes working, what am I missing? |
Have you tried running it across the individual sub-projects? I wouldn't expect ts-fix to follow references. |
Yep, that works! It might be only me but I find this confusing, and there is no warning anywhere either! Thanks anyway! |
Actually the CI problems are real; the tests are not actually typechecked so that's being missed. |
This is to allow autofixes for errors like microsoft/TypeScript#58260.
Test plan:
ts-fix -t <path_to_tsconfig> -e9013 -f add-annotation --write --ignoreGitStatus
and verify that type annotations are automatically added where necessary.Fixes #32