-
Couldn't load subscription status.
- Fork 75
Add 'fix' command to apply fixes (and unit test) #754
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
base: master
Are you sure you want to change the base?
Add 'fix' command to apply fixes (and unit test) #754
Conversation
a583792 to
3924085
Compare
3c64772 to
7880339
Compare
14a265e to
27342d3
Compare
df6e26c to
bc9e328
Compare
src/FSharpLint.Console/Program.fs
Outdated
|
|
||
| let handleFixResult (ruleName: string) = function | ||
| | LintResult.Success warnings -> | ||
| String.Format(Resources.GetString "ConsoleApplyingSuggestedFixFile", ruleName) |> output.WriteInfo |
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.
@webwarrior-ws ruleName? this is wrong
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.
What should the parameter be then?
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.
Same as what happens when printing "Linting {0}", obviously
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.
Changed to print notification for every warning, with file name as parameter. There may be several warnings per file though.
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 may be several warnings per file though.
Then this was wrong, it should only print once per file. (And it was also wrong even if not adding the $0 element.)
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.
Because I guess that "Linting {0}" only gets printed once per file/project/solution?
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.
The function is not called once per file, but once per rule.
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'm not talking about what the code does now, but what the code should do. And it should do the same as Linting{0}
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.
Changed the logic. Now it is what you meant @knocte ?
a82fe7c to
8beb004
Compare
|
@webwarrior-ws let's rebase this PR when you have time (but beware of recent push from me, to include the last commit; so |
2e3a715 to
918d878
Compare
|
@webwarrior-ws sorry this needs another rebase |
Removed FromText property of SuggestedFix as it duplicates FromRange functionality and is not actually used anywhere.
7ebe1cf to
5a538ef
Compare
Co-authored-by: Parham Saremi <[email protected]> Co-authored-by: webwarrior-ws <[email protected]>
53574ca to
7a893d9
Compare
It was a bit confusing that in some parts of the codebase, the rule's fixes were called "QuickFix" sometimes (more like towards the user) and "SuggestedFix" in others (more API-wise). With this commit we just converge both ways into one, and a much simpler one: fix.
6efbad1 to
7a893d9
Compare
Rebased |
Also removed FromText property of SuggestedFix as it duplicates FromRange functionality and is not actually used anywhere.