Community
Participate
Working Groups
An extension point should be provided to register ICodeMiningProviders for a given text editor (JDT Editor, GenericEditor). Here how we could register codemining providers for JDT to support 2 codemining providers: * codemining for references/implementations/main (run debug) : * codemining for JUnit test (run debug) : ----------------------------------------------------------------------------- <extension point="org.eclipse.ui.workbench.texteditor.codeMiningProviders"> <codeMiningProvider id="java" activate="true" class="JavaCodeMiningProvider" name="Java CodeMining" targetId="java.codeminings.target"> </codeMiningProvider> </extension> <extension point="org.eclipse.ui.workbench.texteditor.codeMiningProviders"> <codeMiningProvider id="junit" activate="true" class="JUnitCodeMiningProvider" name="JUnit CodeMining" targetId="java.codeminings.target"> </codeMiningProvider> </extension> <extension point="org.eclipse.ui.workbench.texteditor.codeMiningProviderTargets"> <target id="java.codeminings.target" name="Java Editor"> </target> </extension> ----------------------------------------------------------------------------- Each codemining providers are linked to a target id "java.codeminings.target" that JDT Java Editor should register. It works exactly like hyperlink extension point.
An another proposition is to provide one extension point "org.eclipse.ui.workbench.texteditor.codeMinings" (1) instead of 2 extension point "org.eclipse.ui.workbench.texteditor.codeMiningProviders" & "org.eclipse.ui.workbench.texteditor.codeMiningProviderTargets"(2) like: ----------------------------------------------------------------------------- <extension point="org.eclipse.ui.workbench.texteditor.codeMinings"> <provider id="java" activate="true" class="JavaCodeMiningProvider" name="Java CodeMining" targetId="java.codeminings.target"> </provider> <provider id="junit" activate="true" class="JUnitCodeMiningProvider" name="JUnit CodeMining" targetId="java.codeminings.target"> </provider> <target id="java.codeminings.target" name="Java Editor"> </target> </extension> ----------------------------------------------------------------------------- What do you prefer? (1) or (2) ?
I'm not a big fan of this `target` pattern in general. I prefer activation/enablement based on content-type or enabledWhen/activeWhen expression that give much more flexibility. Do you think an approach based on enabledWhen is doable here?
> I'm not a big fan of this `target` pattern in general. Me too:) But I though to keep the same logic than hyperlink. > I prefer activation/enablement based on content-type or enabledWhen/activeWhen expression that give much more flexibility. Is it a good idea to use content type? For instance with lsp codelens, the condition is not about content type but with support of codelens language server. My first idea was to manage that with a property test class that you imlement as you wish. What do you think about that?
(In reply to Angelo ZERR from comment #3) > Is it a good idea to use content type? Content-type may not be enough, but it'd be nice if we could include this as a condition on an enableWhen or such. That said, I don't think we do need it at the moment > For instance with lsp codelens, the > condition is not about content type but with support of codelens language > server. My first idea was to manage that with a property test class that you > imlement as you wish. > What do you think about that? I think it's good. Moreover, activeWhen/enableWhen interpreters can very well handle thos property test class, so it seems like the most flexible and powerful approach.
@Mickael, have you a sample code with activeWhen/enableWhen interpreters (extension point declaration and Java code which load this extension point)?
IConfigurationElement activeWhen = activeWhenElements[0]; IConfigurationElement[] activeWhenChildren = activeWhen.getChildren(); if (activeWhenChildren.length == 1) { try { Expression expression = this.expressionConverter.perform(activeWhen.getChildren()[0]); EvaluationContext context = new EvaluationContext(null, defaultEvaluationObject); isActive = expression.evaluate(context).equals(EvaluationResult.TRUE); } } The hardest part is to properly create the EvaluationContext with relevant data.
(In reply to Mickael Istria from comment #6) > [...] With this.expressionConverter = new ExpressionConverter(new ElementHandler[] { ElementHandler.getDefault(), // other custom element handlers that can make sense here });
Thanks @Mickael for your feedback. I have never done that, I must study it. I don't know how many time I could do that. If it's so hard for me, perhaps I could create a gerrit patch without enableWhen at first?
Ok. However, I'd rather avoid introducing the "target" in the extension point if we plan to replace it with something better. A first iteration should skip it. Also, I don't get what's the "activate" attibute. Can you please explain?
> However, I'd rather avoid introducing the "target" in the extension point if we plan to replace it with something better. A first iteration should skip it. Yes sure. > Also, I don't get what's the "activate" attibute. Can you please explain? It's like hyperlink, activate - an attribute that tells whether to activate the contributing plug-in when hyperlink detection takes place in the given target (not sure we need it?)
(In reply to Angelo ZERR from comment #10) > It's like hyperlink, activate - an attribute that tells whether to activate > the contributing plug-in when hyperlink detection takes place in the given > target (not sure we need it?) No, I don't think we need it. We should use the typical rule of extension: plugin loads automatically when contributed class is loaded. If we need some earlier loading, we'll add it later; but as the vast majority of extension do work well without such `activate`, I don't see why we need it here more than anywhere else.
> The hardest part is to properly create the EvaluationContext with relevant data. I think I understand more what you mean. The data that I can have is ISourceViewer (because I'm in TextSourceViewerConfiguration to retrieve ICodeMiningProvider extension point). For LSP it will work since LanguageServiceAccessor.getLSPDocumentInfosFor works with a IDocument. But what about with other case? I think other editors will need IResource or ITextEditor, but it's not possible to get in TextSourceViewerConfiguration. Hyperlink works in other editor because they register their editor in their context by overriding #getHyperlinkDetectorTargets (like GenericEditor does). Have you an idea @Mickael how I could do it? Is IsourceViewer data is enough?
Having an ISourceViewer may be enough, because from the viewer we should be able to get document, and from the document the content-type... I believe there may be some issues about the initialization sequence which may make that from the first call, the viewer won't contain reference to the document and is not so useful. What you can do to work around that is to initiatilize the codeMiningProviders as part of the documentChanged(...) in the TextEditor.
All features (content assist,hyperlink, etc) are initialized in SourceViewer#configure(SourceViewerConfiguration configuration) but at this step document is null. So if we wish to use enableWhen, we should do that in other place (like documentChanged for instance). I could do that but it will differ from other features. Is it not annoying?
(In reply to Angelo ZERR from comment #14) > Is it not annoying? Not to me, especially if it works better and give much more powerful enablement possibilities.
> Not to me, especially if it works better and give much more powerful enablement possibilities. Perhaps initialize of CodeMining should be done with a CompletableFuture in order to avoid freezing the editor. I think about lsp which can take time to initialize language server. No?
(In reply to Angelo ZERR from comment #16) > Perhaps initialize of CodeMining should be done with a CompletableFuture in > order to avoid freezing the editor. I think about lsp which can take time to > initialize language server. No? I don't think it's necessary. At this point, I think it's up to the provider to make the initialization fast and deal with it. For example, LSP4E would contribute the codemining provider anyway, and this codemining provider would simply not return anything until the LS is started, so the delay to start server happens later (during the computation of codemining)
New Gerrit change created: https://git.eclipse.org/r/113331
@Mickael, please see my gerrit patch https://git.eclipse.org/r/#/c/113331/ which provides codemining provider extension point with <enabledWhen>. You can see a sample with the codeMiningProviders.exsd: ----------------------------------------------------- <extension point="org.eclipse.ui.workbench.texteditor.codeMiningProviders"> <codeMiningProvider class="org.eclipse.jdt.internal.ui.codemining.JavaReferencesCodeMiningProvider" id="org.eclipse.jdt.ui.codemining.references" label="Java References"> <enabledWhen> <with variable="editor"> <instanceof value="org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor"/> </with> </enabledWhen> </codeMiningProvider> </extension> ----------------------------------------------------- Here 2 important information (I don't know how to set it, in exsd?): - enabledWhen : available variables are "editor" & "viewer" and default variable is the editor. I tell me if "editor" should be enough, because we can retrieve the viewer from the editor, no? - the extension point don't update codeminings, it just register ICodeMiningProviders. Where can I add this information? - in other words, the update is not done, developer must call SourceViewer#updateCodeMinings() in a reconciler or other things. My question is about update on focus ? Perhaps AbstractTextEditor#setFocus should update codeminings? I think about this case when a java editor changes some content and when you go at in an another editor, codeminings should be updated.
@Mickael I would like to provide a sample with CodeMining extension point. My proposition is to display number of referenced project inside .project by using the org.eclipse.ui.genericeditor.examples which supports .project editor with GenericEditor. Take a sample with a "test" project and a "test2" project which references "test" project. When you will open .project you will see that: * test/.project ----------------------------------------- <?xml version="1.0" encoding="UTF-8"?> <projectDescription> 1 reference <name>test</name> ... ----------------------------------------- * test2/.project ----------------------------------------- <?xml version="1.0" encoding="UTF-8"?> <projectDescription> 0 reference <name>test2</name> ... ----------------------------------------- Do you like this idea?
(In reply to Angelo ZERR from comment #20) > My proposition is to display number of referenced project inside .project > [...] > Do you like this idea? Yes, a lot!
Created attachment 271909 [details] Demo with CodeMining & dotproject GenericEditor Here a demo that I have done for .project inside org.eclipse.ui.genericeditor.examples Because of this demo, I have added "editorInput" in the expression context to check content type (see <with variable="editorInput">): -------------------------------------------------- <extension point="org.eclipse.ui.workbench.texteditor.codeMiningProviders"> <codeMiningProvider class="org.eclipse.ui.genericeditor.examples.dotproject.codemining.ProjectReferencesCodeMiningProvider" id="org.eclipse.ui.genericeditor.examples.dotproject.codemining.references" label="Project references"> <enabledWhen> <with variable="editorInput"> <adapt type="org.eclipse.core.resources.IFile"> <test property="org.eclipse.core.resources.contentTypeId" value="org.eclipse.ui.genericeditor.examples.dotproject" /> </adapt> </with> </enabledWhen> </codeMiningProvider> </extension> -------------------------------------------------- In other words, today we have 2 variables "editor", "editorInput" and "viewer". A problem that I have seen with this demo, is that minings is not updated when editor takes focus. (it's annoying if you change project references in project2 (remove project1 as reference) and you go at .project of project1. So I tell me, if minings should be updated on focus (in AbstractTextEditor#setFocus) An another things that I have seen is that AbstractCodeMiningProvider is not helpfull. Perhaps we should remove it?
All that seems really great! Being able to use the editor input is incredibly practical and powerful! That looks like, once again, a great piece of work! Before merging the patch, I'd like to be able to look at the example you implemented. Would you mind pushing another Gerrit with the example to ease review?
New Gerrit change created: https://git.eclipse.org/r/113429
> All that seems really great! Cool!!! > Being able to use the editor input is incredibly practical and powerful! That looks like, once again, a great piece of work! Wow, thanks! But it was your idea to provide "enabledWhen" which was a so great idea:) > Before merging the patch, I'd like to be able to look at the example you implemented. Would you mind pushing another Gerrit with the example to ease review? done with https://git.eclipse.org/r/113429 Please note that I have added a comment at https://git.eclipse.org/r/#/c/113429/1/org.eclipse.ui.genericeditor.examples/src/org/eclipse/ui/genericeditor/examples/dotproject/codemining/ProjectCodeMiningStrategy.java about focus.
Gerrit change https://git.eclipse.org/r/113331 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=079815ecaf4c3f5aea1bb04692048e955daea8c4
New Gerrit change created: https://git.eclipse.org/r/113502
@Mickael please review my gerrit patch https://git.eclipse.org/r/#/c/113502/ for N&N. Thanks!
Gerrit change https://git.eclipse.org/r/113502 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=cad81db866a025b4b3241359ecb27bd301d882b0
