Add completion handler extension point#3738
Conversation
|
Can one of the admins verify this patch? |
|
@fbricon could you take a quick look here? Is this change going in the right direction, or you had something else in mind? #3719 (comment) I've started with the most simple part of what we want to change, the code completions. If this is the right direction, I have some questions as well:
If plug-ins are contributing completion handlers, is it expected that this usage is also covered? |
trancexpress
force-pushed
the
completion-handler-extension
branch
3 times, most recently
from
f992c28 to
4ef603e
Compare
|
@trancexpress from a cursory look, I think you're on the right track. Indeed we need to figure out how to handle default items, and how to also connect completion resolution, I'll try to give it more time later today |
|
add to whitelist |
|
Thank you @fbricon ! There is no rush here, it will take some time before I can try the changes out. Though I think I've gotten the PR to a state where there is not much room for error when hooking our code... |
trancexpress
force-pushed
the
completion-handler-extension
branch
3 times, most recently
from
fd200fa to
bc644c0
Compare
| * @author Simeon Andreev | ||
| * | ||
| */ | ||
| public class ExtensionRepository<T> implements IRegistryEventListener { |
There was a problem hiding this comment.
I think this will be useful for other extension points.
trancexpress
force-pushed
the
completion-handler-extension
branch
3 times, most recently
from
8956ce9 to
184727b
Compare
|
I'll check if it makes sense to add more tests, otherwise I think this PR is ready for a review. |
|
The test job failed with: I see no failures when I run those test cases locally. When I run all tests locally (select 
I ran tests also on command line:
|
|
I know some errors fail locally because they require running Eclipse with lombok. Errors on the main branch might be caused by some recent updates to the Eclipse 4.40 bits. You can try to reload your target platform to check. |
I do have the latest platform set. |
@fbricon @trancexpress You may want to take a look at #3739 |
|
Thank you @snjeza ! I'll rebase when your PR is merged. |
|
I did try cherry-picking f2b053e, locally the tests continue to fail. Maybe it will be better in the Jenkins job. |
trancexpress
force-pushed
the
completion-handler-extension
branch
from
184727b to
2047609
Compare
|
Even more fails now: Running the tests locally from Eclipse, I see only these fails: I'll try with disabling the new test, maybe its corrupting the extension registry somehow... |
|
new test failures are caused by upstream JDT Core now targeting Java 26 for preview features. #3740 should fix that |
trancexpress
force-pushed
the
completion-handler-extension
branch
from
3eb0be2 to
a669cd6
Compare
|
Adding the completion handler extension on-the-fly would've been nice, but maybe it causes problems. I've switched to a plain |
trancexpress
force-pushed
the
completion-handler-extension
branch
from
a669cd6 to
c4ad50e
Compare
|
@fbricon this is ready for a review, tests are green now. |
fbricon
left a comment
fbricon
left a comment
There was a problem hiding this comment.
I'm pondering whether we should allow users to disable specific completion providers via a setting, in case one of them misbehaves and hangs completion entirely.
| isIncomplete |= c.isIncomplete(); | ||
| if (i == 0) { | ||
| // TODO: how to merge defaults? | ||
| defaults = c.getItemDefaults(); |
There was a problem hiding this comment.
I don't think there's a way to merge the defaults.
I think we should first exclude the itemdefaults not supported by the client
Then just pick the 1st one, so you end up with something like :
if (defaults == null && isClientSupportedItemDefaults(c.getItemDefaults()) {
defaults = c.getItemDefaults();
}
There was a problem hiding this comment.
Sorry I've missed the notifications for the comments here.
I don't understand what isClientSupportedItemDefaults() is here, I've adjusted to ask the preferences for isCompletionListItemDefaultSupport(). Is this what you meant?
Potentially this causes some of the fails seen in the latest build:
org.eclipse.jdt.ls.core.internal.handlers.CompletionHandlerLazyResolveTest.testSnippet_while_itemDefaults_enabled_generic_snippets
org.eclipse.jdt.ls.core.internal.handlers.CompletionHandlerTest.testSnippet_inner_class_itemDefaults_enabled_type_definition
org.eclipse.jdt.ls.core.internal.handlers.CompletionHandlerTest.testCompletion_lombok2
org.eclipse.jdt.ls.core.internal.handlers.CompletionHandlerTest.testCompletion_field_itemDefaults_enabled
org.eclipse.jdt.ls.core.internal.handlers.PostfixCompletionTest.test_sysout_itemDefaults_enabled
org.eclipse.jdt.ls.core.internal.handlers.ImportNewProjectsTest.testImportNewGradleProjects
org.eclipse.jdt.ls.core.internal.handlers.ImportNewProjectsTest.testImportNewMavenProjects
This change adds an extension point
which can be used to contribute a completion handler.
A completion handler can be contributed as follows:
<plugin>
<extension
id="contributedCompletionHandler"
point="org.eclipse.jdt.ls.core.completionHandler">
<completionHandler
id="testHandler"
class="my.test.ContributedCompletionHandler" />
</extension>
</plugin>
The handler must implement the interface:
org.eclipse.jdt.ls.core.internal.handlers.ICompletionHandler
Offered code completions are the aggregated completions of
the JDT LS CompletionHandler and the contributed completion handlers.
The handler which generated a completion is also the handler
which is asked to complete it, if the completion is selected.
trancexpress
force-pushed
the
completion-handler-extension
branch
from
c4ad50e to
74f70aa
Compare
I'm not sure, if this was just Eclipse I would suggest a dialog and a time limit preference, something like "Completion Handler XYZ is unresponsive, disable it?" When its VS Code on top to show such UIs, I'm not sure how to do that. If the preference would allow the user to disable a specific provider, the user would need to know what to disable - we would have to tell the user somehow, which provider is being slow. Maybe this should be up to the maintainers of the product, not including plug-ins that slow down content assist? Resp. to fix that slowness. For maintainers a preference like that would be simple, of course. Alternatively we can show a preference in VS Code that defaults to all contributed completion provider IDs, the user can then edit and eventually land at the provider that is being slow. Likely |
4 participants
This change adds an extension point which can be used to contribute a completion handler.
A completion handler can be contributed as follows:
The handler must implement the interface:
org.eclipse.jdt.ls.core.internal.handlers.ICompletionHandlerOffered code completions are the aggregated completions of the JDT LS
CompletionHandlerand the contributed completion handlers.The handler which generated a completion is also the handler which is asked to complete it, if the completion is selected.