-
Notifications
You must be signed in to change notification settings - Fork 419
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
Handle nested code actions #753
Handle nested code actions #753
Conversation
My next PR will stop "Remove Unnecessary Usings" from showing here. |
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.
👍
await CollectRefactoringActions(refactoringContext.Value); | ||
} | ||
|
||
// TODO: Determine good way to order code actions. | ||
actions.Reverse(); |
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.
Any thoughts on this (besides just reversing the order like we do today?)
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.
We have some notion of ordering in Roslyn using attributes but it's messy. At some point, I intend to order them similarly in OmniSharp, but it's down the priority list a bit. For now, I just wanted to add a TODO to indicate that it wasn't ideal. 😄
protected BaseCodeActionService(OmniSharpWorkspace workspace, CodeActionHelper helper, IEnumerable<ICodeActionProvider> providers, ILogger logger) | ||
{ | ||
this.Workspace = workspace; | ||
this.Providers = providers; | ||
this.Logger = logger; | ||
this._helper = helper; | ||
|
||
// Sadly, the CodeAction.NestedCodeActions property is still internal. |
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.
👎 (at it still being internal)
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.
We discussed making it public awhile back, but we tabled the issue because folks didn't like the word "Nested" or "Composite" because it's really about grouping related code actions together rather than making up a larger code action out of little ones. I need to bring this back up with the team again.
Thanks for the approval! Burning down some bugs. 😄 |
Fixes dotnet/vscode-csharp#302
This change digs into the first level of an action's "nested" code actions. This is a feature added more recently to Roslyn which is used to support code actions being hidden under submenus in VS. We can't assume submenu support in OmniSharp, so we expand the nested actions in place.