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

Support isPreferred from CodeAction #829

Open
BoykoAlex opened this issue Sep 26, 2023 · 3 comments
Open

Support isPreferred from CodeAction #829

BoykoAlex opened this issue Sep 26, 2023 · 3 comments

Comments

@BoykoAlex
Copy link
Contributor

BoykoAlex commented Sep 26, 2023

I only have in mind showing a quickfix (marker resolution) created based on CodeAction marked as "preferred" above other quickfixes.

Eclipse sorts quick fixes alphabetically which may not be desirable... For example there are 3 quick fixes proposed to a problem:

  1. Fix the problem in the current code snippet
  2. Fix all such problems in the current java file
  3. Fix all such problems in the corresponding java project

The preferred fix would the current code snippet but it is challenging to push it at the top of the list due to Eclipse's alphabetical sorting. Either label becomes ugly and inconsistent or the order is reversed.

Currently Eclipse sorts the list of quick fixes (together with assists and refactorings) in alphabetical order with org.eclipse.jdt.ui.text.java.CompletionProposalComparator.CompletionProposalComparator. It is not meant to sort alphabetically since the fOrderAlphabetically is false but it falls through to alphabetical order based on resolution labels due to the type of marker resolution object type, i.e. no "relevance" property. The "relevance" property is only available in IJavaCompletionProposal subtypes. However, org.eclipse.lsp4e.operations.codeactions.CodeActionMarkerResolution cannot implement IJavaCompletionProposal because it should be independent from JDT.

What would be your thoughts on this @mickaelistria @rubenporras @angelozerr ?

@mickaelistria
Copy link
Contributor

This seems to be the case for LSP4E integrated in JDT; is it the same in the generic editor (eg using eclipseide-jdtls) ? Does the generic editor sort the code actions?
If in JDT only, is it possible to override or configure the CompletionProposalComparator ?

@BoykoAlex
Copy link
Contributor Author

BoykoAlex commented Sep 27, 2023

@mickaelistria I think you're right about the sorting done for marker resolutions in Java editor only. Thanks for pointing this out as it seemed to push me in the right direction to address the sorting on our end.

  1. I have created a new marker type for Boot LS (LS extension point)
  2. I have implemented LSPBootCodeActionMarkerResolution a subclass of LSPCodeActionMarkerResolution and associated it with the marker type via extension point
  3. The LSPBootCodeActionMarkerResolution creates DelegateMarkerResolution in getResolutions(IMarker), where DelegateMarkerResolution wraps IMarkerResolution from super class LSPCodeActionMarkerResolution and has a relevance field that is assigned based on the index in the array of resolutions. DelegateMarkerResolution implements IJavaCompletionProposal and has implementation for the icon image and relevance, the rest of the methods throws unsupported op.

The only thing i don't like in my solution is warnings about extending non-API classes...

I'm not sure which part of this could be moved to LSP4E... Perhaps, move LSPBootCodeActionMarkerResolution and call it LSPJavaCodeActionMarkerResolution Move DelegateMarkerResolution and call it JavaMarkerResolution. Allow for custom icon for each marker resolution perhaps via some API. Make LSPJavaCodeActionMarkerResolution an API class and let people use it in org.eclipse.ui.ide.markerResolution extension. How does this sound?

@mickaelistria
Copy link
Contributor

If this seems generic enough to you, you could start by moving as much as possible of this into the org.eclipse.lsp4e.jdt bundle, so it becomes independent from the Boot type; and then, in a next step, we can more easily discuss what can go in org.eclipse.lsp4e directly to serve other cases than Java one.

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

No branches or pull requests

2 participants