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

Code actions contribution point #82718

Closed
3 of 5 tasks
mjbvz opened this issue Oct 16, 2019 · 21 comments
Closed
3 of 5 tasks

Code actions contribution point #82718

mjbvz opened this issue Oct 16, 2019 · 21 comments
Assignees
Labels
api api-proposal editor-code-actions Editor inplace actions (Ctrl + .) feature-request Request for new features or functionality *out-of-scope Posted issue is not in scope of VS Code
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 16, 2019

Problem

For feature requests such as #52846 or any improved refactoring UX, VS Code needs to know all the code actions that are available for a language. Some information about available code actions is provided by CodeActionProviderMetadata but only if an extension has been activated and the provider has been registered

Proposal

To support this, we likely want a new contribution point that lets extensions statically tell us about the general classes of the code actions that they provide:

{
  "contributions": {
    "codeActions": [
      {
        "languages": [
          "javascript",
          "javascriptreact",
          "typescript",
          "typescriptreact"
        ],
        "actions": [
          {
            "kind": "refactor.extract.constant",
            "title": "Extract constant",
            "description": "Extract expression to a constant."
          }
        ]
      }
    ]
  }
}

Potential use cases for this info

/cc @kieferrm

@mjbvz mjbvz added the api label Oct 16, 2019
@mjbvz mjbvz added this to the November 2019 milestone Oct 16, 2019
@mjbvz mjbvz self-assigned this Oct 16, 2019
@mjbvz

This comment has been minimized.

@mjbvz mjbvz added the editor-code-actions Editor inplace actions (Ctrl + .) label Oct 18, 2019
mjbvz added a commit that referenced this issue Nov 6, 2019
For #82718
Fixes #52846

This adds a newly proposed codeActions contribution point. For details, see #82718

This change also makes the intellisense for the `editor.codeActionsOnSave` property dynamic by using the new contribution point
mjbvz added a commit that referenced this issue Nov 6, 2019
Fixes #84033

Enables intellisene in the keybindings json editor for code actions and refactorings. Uses the new contribution point from #82718
@mjbvz
Copy link
Collaborator Author

mjbvz commented Nov 6, 2019

I have checked in a prototype of the static codeActions contribution point and have hooked up a few examples of how this information could be used by us, such as providing intellisense for editor.codeActionsOnSave and when configuring code action keybindings. I think that a static contribution point makes sense for use cases like these, although it means that extensions need to generate those static lists for us, see: microsoft/TypeScript#34859

@jrieken brought up that not all extensions may be able to generate these lists ahead of time, such as those using Rosyln. For some use cases, we may also want to dynamically enabled/disable which code actions are shown to the user. If you are using an old TypeScript version for example, refactoring is not supported. An example of where code actions would be enabled/disabled would a improved refactor context menu in the editor

My current thinking on static vs dynamic contribution points is that we likely want both:

  • The static info would document all possible code actions that may be available. Having this makes sense IMO when you are configuring keybindings. It also does not require activating extensions.

  • The dynamic info about what code actions are really available in the current file would be useful for the refactoring menu in the editor. The CodeActionProviderMetadata api already lets extensions provide this info to us

@mjbvz

This comment has been minimized.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Nov 6, 2019

Copying a few extension authors for feedback on contribution point and possible use cases

@DanTup for Dart
@DonJayamanne for Python
@DustinCampbell for C#

mjbvz added a commit that referenced this issue Nov 7, 2019
- Adds a description
- Remove schama
- Moves the language to the top level so we don't need to duplicate so much info for each code action
@mjbvz
Copy link
Collaborator Author

mjbvz commented Nov 7, 2019

Based on trying this for JS/TS, I've added a description field and changed the schema to the following:

 "codeActions": [
      {
        "languages": [
          "javascript",
          "javascriptreact",
          "typescript",
          "typescriptreact"
        ],
        "actions": [
          {
            "kind": "refactor.extract.constant",
            "title": "%codeActions.refactor.extract.constant.title%",
            "description": "%codeActions.refactor.extract.constant.description%"
          },
          ....
        ]
      }
    ]

@DonJayamanne
Copy link
Contributor

Are these code actions context aware (visa a callback)?
I.e. can I have them show up in one file and not the other. Or one line and not the other?

@mjbvz
Copy link
Collaborator Author

mjbvz commented Nov 7, 2019

This proposal does not effect the existing CodeActionProvider api (which lets your extension control which actual code actions are shown to the user in the editor). The static contribution point proposed here aims to supports some of the use cases outlined above without needing to activate any extensions

@DonJayamanne
Copy link
Contributor

The static info would document all possible code actions that may be available. Having this makes sense IMO when you are configuring keybindings. It also does not require activating extensions.

Missed this.

@DanTup
Copy link
Contributor

DanTup commented Nov 7, 2019

The code actions in Dart are all defined in the language server, so we can't really provide this ahead-of-time either. This is similar to a request I made a while ago for keybinds:

#45383

I expected them to only show up when the extension is activated though - not ideal, but supports the case where they might change across different versions of SDKs that you can't possibly know ahead-of-time.

@jrieken
Copy link
Member

jrieken commented Nov 7, 2019

@mjbvz Maybe something more pragmatic. Once a code action provider registered (dynamically) we store its code action kinds (for configurations) and revalidate them whenever registration happens again or when an extension cannot be found anymore. That would work better with dynamic code action lists and still yield in a better user experience - except the first time before the first activation.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Nov 7, 2019

@DanTup This is the case for TS as well. There should be some way for you to extract this list, either manually or with automation. Again the idea is that the extension provides the complete set of all possible refactorings that users can configure—regardless of what specific language version a user is on—and that this does not any impact on what actions are actually shown in the editor

I'm not opposed to allowing the same metadata to be contributed dynamically as well, but this would also be new information that extensions would have to provide to VS Code (i.e. through a new api). The current CodeActionProviderMetadata is not sufficient

@jrieken Managing a cache of metadata about code actions will get very complicated. TS for example only contributes a refactor provider if you are using TS 2.8, and user can explicitly disable some code actions like organize imports through settings

@DanTup
Copy link
Contributor

DanTup commented Nov 7, 2019

@DanTup This is the case for TS as well. There should be some way for you to extract this list, either manually or with automation.

Sure, it just means having some script to update/maintain it I was hoping to avoid :-)

and user can explicitly disable some code actions like organize imports through settings

Doesn't that affect a static list in the same way (there may be things in the static list that have been disabled)?

@orta
Copy link
Contributor

orta commented Nov 7, 2019

I made a script which generates all of TypeScripts microsoft/TypeScript#34859 (comment) , but the tricky thing here is that it won't account for a workspace's version of typescript (only vscode's) - which could end up asking for refactors that don't exist. That could end up being a tricky mismatch.

We wondered about making it something the TSServer could provide? but that kinda undermines the point of having it beforehand.

@akaroml
Copy link
Member

akaroml commented Nov 8, 2019

While we do Java language support in VS Code, we always hoped there could be some way to fully expose all the capabilities. The problem with the dynamic way is that the options are contextual. So we constantly hear from the users asking for some refactoring features that are actually there. They just couldn't find them or figure out how to trigger them.

That's why I really like this idea of having this new contribution point, which introduces the static way of exposing all possible code actions. The extension's contribution will make it even better by showing all the options in one place. Power users would want to set hot keys to specific actions, and this will also be possible and convenient.

One feedback is still regarding discoverability. IntelliJ has this top menu entry to expose all refactoring options. It shows the full picture even when some options are not available at the moment. It would be great to have some equivalent in VS Code because refactoring is especially important for languages like Java, C#.
image

@jrieken
Copy link
Member

jrieken commented Nov 8, 2019

Managing a cache of metadata about code actions will get very complicated. TS for example only contributes a refactor provider if you are using TS 2.8, and user can explicitly disable some code actions like organize imports through settings

Yes, but applying the same logic how it is then better to "cache" them at compile time, e.g writing them upfront into package.json without knowing anything about the user or her/his settings nor being able to change that list? What I am suggesting is to cache them only between the time of deactivation and activation.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Nov 9, 2019

In our existing keybindings UX, we let users configure keybindings for any command. Many of these commands are only actually enabled when the user has enabled a setting or switched their language run time, but that does not prevent users from setting up the keybinding.

I believe that is also the behavior we want when configure keybindings for code actions or refactors: I should always be able to configure a keybinding for extract constant if we know some extension provides it. A static contribution point enables that

@mjbvz mjbvz removed this from the November 2019 milestone Dec 2, 2019
@mjbvz mjbvz added this to the December 2019 milestone Dec 2, 2019
@mjbvz mjbvz modified the milestones: January 2020, Backlog Jan 9, 2020
@DanTup
Copy link
Contributor

DanTup commented Apr 20, 2020

@mjbvz what's the status of this? If I want my code actions to show up in the keybinding list, should I be statically contributing them in package.json like this, or just providing them via documentation (like #89388)? It's not clear if documentation also includes this?

@DanTup
Copy link
Contributor

DanTup commented Apr 20, 2020

Strangely, although some code actions show up in the keybinding list, if I try to add my own based on the info above, I get this warning:

Screenshot 2020-04-20 at 12 38 43

If I remove the array (?!), then it breaks the intellisense for the existing ones, because it throws an error trying to call slice on the actions object.

@DanTup
Copy link
Contributor

DanTup commented Apr 20, 2020

(oh, ignoring the warning seems like the right thing - my code action then shows up! 🤨)

@heejaechang
Copy link

Pylance team wants to expose all individuals fix all code actions to users so they can configure which fix all code actions run on file save. currently there seems no way to add those individual action names in editor.codeActionsOnSave schema. and no way to provide default value for code actions on save for each fix all code actions.

I think that makes it very hard for users to discover each fix all actions and make default such as source.fixAll: true very intrusive since updating extension without knowing that it just added new fix all code action can change users code unexpectedly. furthermore, in current design, it will be very hard to find out which extension actually did it.

Please, provide a way to configure this by each fix all code action provider

Copy link

We closed this issue because we don't plan to address it in the foreseeable future. If you disagree and feel that this issue is crucial: we are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding, and happy coding!

@vs-code-engineering vs-code-engineering bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api api-proposal editor-code-actions Editor inplace actions (Ctrl + .) feature-request Request for new features or functionality *out-of-scope Posted issue is not in scope of VS Code
Projects
None yet
Development

No branches or pull requests

7 participants