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

CodeAction api change proposal #34664

Closed
mjbvz opened this issue Sep 19, 2017 · 4 comments
Closed

CodeAction api change proposal #34664

mjbvz opened this issue Sep 19, 2017 · 4 comments
Assignees
Labels
api editor-code-actions Editor inplace actions (Ctrl + .) feature-request Request for new features or functionality on-testplan plan-item VS Code - planned item for upcoming under-discussion Issue is under discussion for relevance, priority, approach
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 19, 2017

Follow up on #34597 and #34595, as part of #33555

Problem

#33555 describes a few key issues with using code actions for refactorings. Along with these concerns, we'd also like to be able to do the following:

  • Sort code actions so that fixes for errors show up first
  • Group code actions by type / by the error they fix
  • Provide a better UI for code actions that tells users which actions address which problems in the code

Proposal

This proposal subsumes the proposals of #34597 and #34595 and is based on discussions @kieferrm and I had. It is based on the following observations:

  • The semantics of when we want to show quick fixes vs refactorings is different. The automatic lightbulb does not make as much sense for refactorings
  • However the API and implementation of quick fixes and refactorings will be very similar.

This proposal extends the CodeAction API to let us differentiate between quick fixes and refactorings. Using this, we can then specialized the UX for each of theses

VS Code API Changes

/**
 *  New enum that identifies the type of a code action
 */
enum class CodeActionType {
    // Standard type. Would be used for existing CodeActionProviders that return Commands
    Default = 0 ,

    // Fixes a diagnostic from the CodeActionContext
    QuickFix = 1,    

    // Perform a refactorng
    Refactoring = 2
}

/**
 * New interface that provides additional info about a code action 
 */
interface CodeAction {
    // Command the performs the action
    // In later iterations we could change this to be a `string | WorkspaceEdit`
    action: string
    
    // Title of the action 
    // The reason we don't just use the `Command` type here is that we will likely want
    // to support `WorkspaceEdit` as a future option for action
    title: string

    // Type of the action
    type: CodeActionType

    // List of diagnostics this code action resolves
    addresses?: Diagnostic[]
   
    // Identifier for the action, such as `"extract method"`
    // This would be used for creating keybinding for specific code actions
    id?: string
}

/**
 * Add a new `requestedTypes` field to the `CodeActionContext`
 */
export interface CodeActionContext {
    readonly diagnostics: Diagnostic[]

    // List of types to compute.
    // Any code actions that are returned that are not in the list will also be filtered out by vscode
    readonly requestedTypes: CodeActionType[]
}

/**
 * Update the `CodeActionProvider` to either return `Commands` or `CodeActions`
 */
export interface CodeActionProvider {
	provideCodeActions(document: TextDocument, range: Range, context: CodeActionContext, token: CancellationToken): ProviderResult<Command[] | CodeAction[]>;
}

Code action context menu UX changes
Using the additional information provided by the new CodeAction interface, we would update the code action context menu to display the following:

  • Add a symbol next to next each code action in the context menu. These symbols would use the CodeActionType to differentiate types of code actions.
  • In the case of QuickFix code actions, also use diagnostic severity to display a different symbol for quick fixes for errors and quick fixes for warnings
  • Sort the code actions so that fixes for errors show up first
  • Surface information about the diagnostic that a code action fixes in the menu

Code action trigger changes
Using the CodeActionType, we can now differentiate quick fixes and refactorings. We would then change when we show each type:

  1. The existing lightbulb / editor.action.quickFix would request code actions with CodeActionType.Standard and CodeActionType.QuickFix. Refactoring code actions would be filtered out.

  2. A new editor.action.refactorcommand would request code actions only of CodeActionType.Refactor and display these in the context menu

Keybinding for code actions
Introduce a new editor.action.codeAction command. This command would take an optional list of code action ids.

When the command is triggered, it would request all code actions that are currently available, but only show those that match the requested ids. If only a single code action matches the id, it would be applied automatically

This would allow us to setup keybindings such as:

{
	"key": "cmd+r cmd+e",
	"command": "editor.action.codeAction",
	"args": [
		"extract method"
	]
}
@mjbvz mjbvz added api under-discussion Issue is under discussion for relevance, priority, approach labels Sep 19, 2017
@jrieken jrieken added the editor-code-actions Editor inplace actions (Ctrl + .) label Sep 20, 2017
@mjbvz mjbvz added the plan-item VS Code - planned item for upcoming label Oct 11, 2017
@mjbvz mjbvz added this to the October 2017 milestone Oct 11, 2017
@jrieken jrieken modified the milestones: October 2017, November 2017 Oct 30, 2017
@mjbvz mjbvz added the feature-request Request for new features or functionality label Nov 14, 2017
@jrieken jrieken modified the milestones: November 2017, January 2018 Dec 4, 2017
@alexdima alexdima modified the milestones: February 2018, December 2017 Dec 12, 2017
jrieken added a commit that referenced this issue Dec 13, 2017
@jrieken
Copy link
Member

jrieken commented Dec 13, 2017

@mjbvz Something I thought while writing some jsdoc is if we should just make the edits be a WorkspaceEdit? I think that makes it more explicit and doesn't add a burden for those that want to make 'local' changes only. So instead of

export class CodeAction {
  //...
  edits?: TextEdit[] | WorkspaceEdit;

have this

export class CodeAction {
  //...
  edit: WorkspaceEdit;

Thoughts? Ideas?

@mjbvz
Copy link
Collaborator Author

mjbvz commented Dec 13, 2017

I'm fine with this. I think we should keep edit optional though since sometimes you only want to provide a command. Should I make the change?

jrieken added a commit that referenced this issue Dec 15, 2017
@jrieken
Copy link
Member

jrieken commented Dec 15, 2017

Pushed 966100d with this:

	edit?: WorkspaceEdit;

@jrieken
Copy link
Member

jrieken commented Jan 29, 2018

@mjbvz Can you flesh this out please: #42333

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api editor-code-actions Editor inplace actions (Ctrl + .) feature-request Request for new features or functionality on-testplan plan-item VS Code - planned item for upcoming under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

3 participants