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

Implement disabled, isPreferred and documentation fields for code actions #10777

Merged

Conversation

lucas-koehler
Copy link
Contributor

Fixes #9989
Closes #10073 - this builds on #10073 but also hands in the documentation metadata into monaco.

Contributed on behalf of STMicroelectronics

Signed-off-by: Lucas Koehler lkoehler@eclipsesource.com

What it does

Fixes #9989

  • Adds support for the disabled and isPreferred fields to the CodeAction interface
  • Adds support for the documentation field to CodeActionProviderMetaData interface.

How to test

  • Include my code actions test plugin. For reference, it's code can be found here
  • Start the application and open a markdown file
  • Type :)
  • Lightbulb should be blue to indicate a preferred fix
  • Open quick fix menu and look at the entries
    • The poop emoji entry is hidden (present on master).
    • There should be an entry Learn more about Emojis... which opens the emoji wikipedia page when selected. This is the documentation entry

Review checklist

Reminder for reviewers

Archie27376 and others added 2 commits February 18, 2022 12:41
Contributed on behalf of STMicroelectronics

Signed-off-by: Lucas Koehler <lkoehler@eclipsesource.com>
@vince-fugnitto vince-fugnitto added the vscode issues related to VSCode compatibility label Feb 18, 2022
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed that functionally the changes work very well 👍

  • confirmed that disabled items are in fact disabled compared to master
  • confirmed that documentation entries work well

if (metadata && metadata.documentation) {
disposables = new DisposableCollection();
documentation = metadata.documentation.map(doc => ({
kind: doc.kind.value!,
Copy link
Member

Choose a reason for hiding this comment

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

@lucas-koehler I believe that value is non-optional in vscode, should we align?

Contributed on behalf of STMicroelectronics

Signed-off-by: Lucas Koehler <lkoehler@eclipsesource.com>
@lucas-koehler
Copy link
Contributor Author

@vince-fugnitto Thanks for your review :) I aligned CodeActionKind's value property to be non-optional and removed the now obsolete ! operators.

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

The changes are looking good to me!

  • Preferred code actions are highlighted
  • Clicking the documentation code actions brings up the pages in the browser

I noticed that a separator is missing in Theia compared to VSCode:

image

Theia:

image

I assume that's unrelated to your changes. Or do you think you can "simply" add that as well?

@lucas-koehler
Copy link
Contributor Author

lucas-koehler commented Feb 22, 2022

I noticed that a separator is missing in Theia compared to VSCode:
[...]
I assume that's unrelated to your changes. Or do you think you can "simply" add that as well?

@msujew Unfortunately, as this menu comes from monaco, I do not know how to add this in a simple way.

@msujew
Copy link
Member

msujew commented Feb 22, 2022

@lucas-koehler Completely fine with me. Since Colin is working on updating Monaco anyway, it's not that important 👍

@JonasHelming
Copy link
Contributor

@vince-fugnitto : Fine with merging this?

@vince-fugnitto vince-fugnitto merged commit 786c1ba into eclipse-theia:master Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement missing CodeAction fields
5 participants