Skip to content
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

Modernize code action provider #4988

Merged
merged 7 commits into from
Jan 10, 2022

Conversation

winstliu
Copy link
Contributor

@winstliu winstliu commented Jan 2, 2022

Easiest to review if you look at it per-commit, though there aren't too many changes overall so you could look at the overall diff too.

While investigating a pet peeve of mine in the VS Code C# experience (see below), I discovered that the CodeActionProvider class is using a legacy function signature for provideCodeActions. The new signature allows for returning CodeActions rather than Commands and also provides a more straightforward way to differentiate between Selections and Ranges.

Benefits:

  • CodeActions enable the extension to return richer information about the underlying command which can then be reflected in the UI.
  • The Range | Selection type means there's no longer a need to manually query editor state to determine whether there's an active selection.

Things seem to be behaving the same through a quick test I did with a file that had a diagnostic on ReferenceEquals(null, obj).

  • No selection, active file: Displays the "Prefer 'is null'" quick fix, along with some code actions for wrapping arguments
  • Selection on the first "e", active file: Only displays the "Prefer 'is null'" quick fix
  • No selection, inactive file: Same as the active file scenario
  • Selection on the first "e", inactive file: Same as the active file scenario

Note that the behavior I observed is different from what's documented in the VS Code API ("[range] will always be a selection if there is a currently active editor"), so it might be good to double-check me on that one.

I've also taken this opportunity to constify everything, because it seemed like pretty low-hanging fruit.


The pet peeve is that quick fixes aren't displayed in this tooltip right here which misleadingly makes you believe that there's no quick fix support:
No quick fixes available
I've figured out that passing kind: CodeActionKind.QuickFix to each CodeAction will cause them to show up in that tooltip. But to do that, CodeActions have to be returned in the first place ;).

@winstliu winstliu force-pushed the modernize-code-action-provider branch from edddf2d to 068c868 Compare January 3, 2022 17:26
@winstliu winstliu force-pushed the modernize-code-action-provider branch from 068c868 to 1b7d018 Compare January 3, 2022 17:33
Copy link
Member

@JoeRobich JoeRobich left a comment

Choose a reason for hiding this comment

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

Thanks @50Wliu!

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

Successfully merging this pull request may close these issues.

2 participants