Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

add refactoring and add code action setting support#1734

Merged
heejaechang merged 13 commits into
microsoft:masterfrom
heejaechang:refactoringsupport
Nov 8, 2019
Merged

add refactoring and add code action setting support#1734
heejaechang merged 13 commits into
microsoft:masterfrom
heejaechang:refactoringsupport

Conversation

@heejaechang
Copy link
Copy Markdown

there is no refactoring provider yet. just plumbing code to enable it on vscode/lsp.

added a plumbing code for codeaction settings.

one can add in settings.json

"python.refactoring.xxx" or "python.quickfix.xxx" and use CodeActionSettings.ReadOption("xxx") to read value.

added hidden "python.quickfix.addimports" for add import feature.

Comment thread src/LanguageServer/Impl/LanguageServer.Configuration.cs Outdated
@heejaechang
Copy link
Copy Markdown
Author

ping @jakebailey @MikhailArkhipov @bschnurr @judej @RaymonGulati1 can you do code review for this PR?

var quickfix = new Dictionary<string, object>();

var refactoringToken = pythonSection["refactoring"];
var quickfixToken = pythonSection["quickfix"];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

quickfix or quickFix? Where are these settings coming from?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

it is coming from settings.json.

ex) "python.quickfix.addimport": false

no UI. just a way to pass setting

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, I was just wondering if @Anapo14 had looked at the naming ahead of time.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Per this: https://github.com/microsoft/vscode/blob/434662c6cb11b4cad41bd4bf80c23812aeb319d6/src/vs/editor/contrib/codeAction/types.ts#L13-L19

quickfix is probably fine. I just wanted to ensure we had a standardized name.

Comment thread src/LanguageServer/Impl/CodeActions/CodeActionSettings.cs Outdated
Comment thread src/LanguageServer/Test/MissingImportCodeActionTests.cs
Comment thread src/LanguageServer/Impl/Sources/RefactoringCodeActionSource.cs Outdated
Comment thread src/LanguageServer/Impl/Definitions/IQuickFixCodeActionProvider.cs Outdated
Comment thread src/LanguageServer/Impl/Definitions/IQuickFixCodeActionProvider.cs Outdated
Comment thread src/LanguageServer/Impl/Definitions/IQuickFixCodeActionProvider.cs Outdated
Comment thread src/LanguageServer/Impl/Definitions/IRefactoringCodeActionProvider.cs Outdated
Comment thread src/LanguageServer/Impl/LanguageServer.Configuration.cs
@MikhailArkhipov
Copy link
Copy Markdown

LGTM, @jakebailey ?

Comment thread src/Analysis/Ast/Test/AnalysisTestBase.cs
@heejaechang
Copy link
Copy Markdown
Author

@MikhailArkhipov so you okay except the option name?

@heejaechang
Copy link
Copy Markdown
Author

any feedback from other people?

@heejaechang
Copy link
Copy Markdown
Author

thank you!

@heejaechang heejaechang merged commit 75cecfb into microsoft:master Nov 8, 2019
@heejaechang heejaechang deleted the refactoringsupport branch November 8, 2019 20:59
MikhailArkhipov pushed a commit that referenced this pull request Jun 15, 2020
* add refactoring and add code action setting support

* some clean up

* added test on add import being turned off

* removed comment not relevant anymore

* removed empty trailing spaces

* changed code to use quickFix. user facing setting is left as quickfix since that is what LSP uses.

* removed using try/catch

* cancellation check inside of the loop

* added timeout

* updated doc comment

* more doc comment update

* more doc comment update

(cherry picked from commit 75cecfb)
MikhailArkhipov pushed a commit that referenced this pull request Jun 15, 2020
* add refactoring and add code action setting support

* some clean up

* added test on add import being turned off

* removed comment not relevant anymore

* removed empty trailing spaces

* changed code to use quickFix. user facing setting is left as quickfix since that is what LSP uses.

* removed using try/catch

* cancellation check inside of the loop

* added timeout

* updated doc comment

* more doc comment update

* more doc comment update

(cherry picked from commit 75cecfb)
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