Community
Participate
Working Groups
This issue is just to update JFace Platforme CodeMinings support inside Java Editor by using IJavaReconcilingListener feature: in other words update CodeMinings when compilation unit changed. Please note this issue doesn't declare some Java CodeMining provider, so it will change nothing about the current Java Editor (StyledText continue to be fast if none Java CodeMinings are registered with extension point).
New Gerrit change created: https://git.eclipse.org/r/116845
New Gerrit change created: https://git.eclipse.org/r/116847
@Dani, @Sarika Please review my simple gerrit patch which updates CodeMinings (if there are CodeMinings registered with extension point codeMiningProvider). My gerrit patch doesn't affect the performance of Java Editor since there are none registered CodeMining providers.
@noopur is there any chance that you accept my gerrit patch. I need it for my other gerrit patch with CodeMining: * Java References/Implementations CodeMinings https://bugs.eclipse.org/bugs/show_bug.cgi?id=529127 * JUnit Status/Run/Debug CodeMinings https://bugs.eclipse.org/bugs/show_bug.cgi?id=529316 * Allow to show the type parameter names in method calls via inlined annotation support https://bugs.eclipse.org/bugs/show_bug.cgi?id=529011 Please note my gerrit patch on this issue doesn't affect performance of JDT Java Editor because CodeMinings is activated only if there one codemining provider registered. @Mickael Istria and me are working on this performance problem with StyledText and here a well gerrit patch https://git.eclipse.org/r/#/c/116962/ which improves performance with big file like StyledText.java. It's not perfect again, but I have other ideas to improve StyledText. Thanks for your review @noopur !
(In reply to Angelo ZERR from comment #4) > @noopur is there any chance that you accept my gerrit patch. I need it for > my other gerrit patch with CodeMining: I don't think I will have time until M6 so I can't commit to looking at it. It will be good if someone else can do the review. In general, it may be better if you create a new beta branch in jdt.ui and put all your changes in that. It will be easier to manage dependencies b/w patches and to test, review and push all the changes together.
> I don't think I will have time until M6 so I can't commit to looking at it. It will be good if someone else can do the review. Ok thanks for your answer. > In general, it may be better if you create a new beta branch in jdt.ui and put all your changes in that. It will be easier to manage dependencies b/w patches and to test, review and push all the changes together. It should really fantastic if this gerit patch of this issue could be merged in M6. The gerit patch is very simple and there are none impact with performances. If you accept this gerrit patch, I could create an external plugin outside JDT project which will provide my Java CodeMinings. Photon users could benefit with Java CodeMinings by installing my external plugin and we could benefit from feedback to know if my external plugin could be integrated inside the JDT project. What do you think about this idea?
(In reply to Angelo ZERR from comment #6) > What do you think about this idea? Sounds good. I will try to get back to you on this Gerrit patch by next week. I hope that's fine.
> Sounds good. Cool! > I will try to get back to you on this Gerrit patch by next week. I hope that's fine. Yes sure! If my gerrit patch is included in Photon, it's perfect! Thanks!
This patch always adds a code mining reconcile listener to the CU editor which may not be required. The listener should be added only when code mining is enabled and other checks can also be added later. The implementation can be similar to SemanticHighlightingReconciler listener i.e. not part of the CompilationUnitEditor itself.
> This patch always adds a code mining reconcile listener to the CU editor which may not be required. The listener should be added only when code mining is enabled and other checks can also be added later. What is the problem with that? If there are none CodeMinings, listener does nothing. The call of updateCodeMinings does nothing. To support your idea, we need to support in JFace Text Platform, listener when CodeMining providers is added, removed and activate and add/remove a "code mining reconcile listener" in this case. I'm afraid that it's a little hard. Today calling updateCodeMinings every time work for any case (when there are none CodeMining, when CodeMining will be added/removed). @Mickael, what do you think about my comment? Do you think we need to support CodeMining listeners?
(In reply to Angelo ZERR from comment #10) > > This patch always adds a code mining reconcile listener to the CU editor which may not be required. The listener should be added only when code mining is enabled and other checks can also be added later. > > > What is the problem with that? If there are none CodeMinings, listener does > nothing. The call of updateCodeMinings does nothing. This is to have a clean solution which avoids cluttering the internal CompilationUnitEditor class. I haven't checked the code but a new reconcile listener similar to SemanticHighlightingReconciler with minimal checks (e.g. for the preference enabling the code minings feature) and functionality for the first cut should be fine here.
(In reply to Noopur Gupta from comment #9) > This patch always adds a code mining reconcile listener to the CU editor > which may not be required. The listener should be added only when code > mining is enabled. by "when code mining is enabled", you mean "when at least one code mining provider is enabled in preferences", don't you? (In reply to Angelo ZERR from comment #10) > Do you think we need to support CodeMining listeners? My interpretation of Noopur's comment is more that we need a preference listener, and depending on whether code mining providers are enabled or not, we invoke CompilationUnitEditor.add/removeReconcilingListeners. This involves creating a "JavaCodeMiningIReconcilingListener implements IJavaReconcilingListener". Noopur's hint about SemanticHighlightingManager is a good one, this class and its usages are probably the best examples to follow.
> My interpretation of Noopur's comment is more that we need a preference listener, Ok I understand more. The question is which preference key we can use inside AbstractDecoratedTextEditor#handlePreferenceStoreChanged to track changed of codemining providers. With https://bugs.eclipse.org/bugs/show_bug.cgi?id=528728 I have tried to manage CodeMinings with a generic mean. So it was easy to refresh codemining, because the preference key was the same for the whole editor. But it seems that you prefer managing CodeMining preferences inside JDT, for instance with the Java / Editor/ CodeMinings preference page: ---------------------- ----- Java ----- [v] Show references [v] Show implementations [v] Show parameters of callee methods ----- JUnit ----- [v] Show status [ ] Show run icon [ ] Show debug icon ---------------------- How to store those preferences? I think the better solution is to have a preference key per "show" mining like this: ---------------------- java.codeminings.references=true java.codeminings.implementations=true java.codeminings.callee_methods=true java.codeminings.junit.status=true java.codeminings.junit.run=false java.codeminings.junit.debug=false ---------------------- With this solution you can easily use IPeferenceStore#getBoolean to know if "java.codeminings.references" and/or "java.codeminings.implementations" managed by JavaCodeMiningProvider must be shown or not. But the hard part is about which preferences key we must track to add/remove IJavaReconcilingListener. We could try to track the whole preferences key "java.codeminings.references", "java.codeminings.implementations", etc but it's not a generic mean since external plugin could define CodeMinings for the Java editor. So I tell me, that we could have a non persistent preference like "java.codeminings.enabled" which is computed in the CodeMinings preference page by checking that there is at least one "show" which is checked. The CompilationUnitEditor#handlePreferenceStoreChanged could track this non persistent preference "java.codeminings.enabled" to add/remove IJavaReconcilingListener which updates code minings. The codemining providers list should be recomputed again, so I think we should update too ISourceViewerExtension5 to add a new method: -------------------------------------------------------- ISourceViewerExtension5#installCodeMinigProviders() -------------------------------------------------------- which is today a private method inside SourceViewer#installCodeMinigProviders') @Mickael, @Noopur what do you think about that?
@Mickael, @Noopur please see my new gerrit patch ay https://git.eclipse.org/r/#/c/116847/ which follows the same idea than semantic highlight manager. Please note my gerrit patch fails (compilation error) because it need this JFace Text gerrit patch https://git.eclipse.org/r/#/c/117806/
Do we really need preference for each ? java.codeminings.references=true java.codeminings.implementations=true java.codeminings.callee_methods=true java.codeminings.junit.status=true java.codeminings.junit.run=false java.codeminings.junit.debug=false I think one preference to enable code mining in JDT editor should be enough, isn't it? At most one for Java and one for JUnit I guess.
> Do we really need preference for each ? IMHO I think it's good to to that. User can check which code minings he wishes to display in the Java Editor. For instance for JUnit, some user would like to show only status of test and other users would like to display status of test and the debug icon. VSCode Java (https://marketplace.visualstudio.com/items?itemName=redhat.java) configures their references and implementations CodeLens with 2 preferences too: ----------------------------------------------------------- java.referencesCodeLens.enabled : Enable/disable the references code lenses. java.implementationsCodeLens.enabled : Enable/disable the implementations code lenses. -----------------------------------------------------------
I agree with Angelo. As a user, I'd like to disable the references count which I find not so profitable and taking a lot of space, but I'd love to get the parameter names, and the JUnit actions/reports for instance.
As a user I will like the flexibility, but then there should be a quick way to disable/enable all the Code Mining Preferences as well.
Created attachment 272792 [details] Java CodeMining & preferences @Sarika here a demo with my current work with Java CodeMining and preferences. This preference page gives the capability to enable/disable each CodeMining easily. Hope you will like it.
(In reply to Angelo ZERR from comment #19) > Created attachment 272792 [details] > Java CodeMining & preferences > > @Sarika here a demo with my current work with Java CodeMining and > preferences. This preference page gives the capability to enable/disable > each CodeMining easily. Hope you will like it. Thanks !!
I would be highly interested in this and would love to implement my own code mining extension for the Java editor. Is there a chance to get this into Photon somehow? Anything I can help with?
> I would be highly interested in this and would love to implement my own code mining extension for the Java editor. Is there a chance to get this into Photon somehow? Anything I can help with? Glad mining could interested you. If you wish to start implementing a Java JDT CodeMining (without applying this patch), I suggest you that you import in your workspace the 2 plugins from https://github.com/angelozerr/jdt-codemining You can see an example at https://github.com/angelozerr/jdt-codemining/blob/master/org.eclipse.jdt.codemining/plugin.xml#L15 Hope it will help you.
(In reply to Angelo ZERR from comment #14) > @Mickael, @Noopur please see my new gerrit patch ay > https://git.eclipse.org/r/#/c/116847/ which follows the same idea than > semantic highlight manager. I have not tried the patch to see if it works as Sarika has already tested that. In the code review, I have made some changes in the patch like added some instanceof and null checks, fixed Javadocs, formatting, method names etc. The patch is good to go once the additional comment regarding preference prefix is handled.
Gerrit change https://git.eclipse.org/r/116847 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=1332a4312cbbf390f92cc692ec6eae2904899c43
Thanks a lot @Noopur! It's a very great news, now anybody can register their own codemining provider for JDT Editor, it's a very cool news! Do you know where I could write some doc "how to integrate codemining in Java Editor"? For instance I think it's important to speak about codemining preferences which must start with "java.codemining." to refresh the Java Editor when preferences changed 'Show, References, etc)
Sorry to spoil the party but this commit: (In reply to Eclipse Genie from comment #24) > Gerrit change https://git.eclipse.org/r/116847 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > ?id=1332a4312cbbf390f92cc692ec6eae2904899c43 seems to cause this failure in official builds: [ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:1.1.0:compile (default-compile) on project org.eclipse.jdt.ui: Compilation failure: Compilation failure: [ERROR] /opt/public/eclipse/builds/4I/gitCache/eclipse.platform.releng.aggregator/eclipse.jdt.ui/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/JavaEditor.java:[4330] [ERROR] installCodeMinigProviders(); [ERROR] ^^^^^^^^^^^^^^^^^^^^^^^^^ [ERROR] The method installCodeMinigProviders() is undefined for the type JavaEditor See http://download.eclipse.org/eclipse/downloads/drops4/I20180321-2000/buildlogs/mb060_run-maven-build_output.txt
(In reply to Stephan Herrmann from comment #26) > Sorry to spoil the party but this commit: > > (In reply to Eclipse Genie from comment #24) > > Gerrit change https://git.eclipse.org/r/116847 was merged to [master]. > > Commit: > > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/ > > ?id=1332a4312cbbf390f92cc692ec6eae2904899c43 > > seems to cause this failure in official builds: > > [ERROR] Failed to execute goal > org.eclipse.tycho:tycho-compiler-plugin:1.1.0:compile (default-compile) on > project org.eclipse.jdt.ui: Compilation failure: Compilation failure: > [ERROR] > /opt/public/eclipse/builds/4I/gitCache/eclipse.platform.releng.aggregator/eclipse.jdt.ui/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/JavaEditor.java:[4330] > > [ERROR] installCodeMinigProviders(); > [ERROR] ^^^^^^^^^^^^^^^^^^^^^^^^^ > [ERROR] The method installCodeMinigProviders() is undefined for the type > JavaEditor > > See > http://download.eclipse.org/eclipse/downloads/drops4/I20180321-2000/buildlogs/mb060_run-maven-build_output.txt > This is due to bug 532703.
(In reply to Angelo ZERR from comment #25) > Thanks a lot @Noopur! It's a very great news, now anybody can register their > own codemining provider for JDT Editor, it's a very cool news! > > Do you know where I could write some doc "how to integrate codemining in > Java Editor"? For instance I think it's important to speak about codemining > preferences which must start with "java.codemining." to refresh the Java > Editor when preferences changed 'Show, References, etc) You can check the eclipse.platform.common repo to add Platform and JDT documentation.
(In reply to Noopur Gupta from comment #28) > (In reply to Angelo ZERR from comment #25) > > Thanks a lot @Noopur! It's a very great news, now anybody can register their > > own codemining provider for JDT Editor, it's a very cool news! > > > > Do you know where I could write some doc "how to integrate codemining in > > Java Editor"? For instance I think it's important to speak about codemining > > preferences which must start with "java.codemining." to refresh the Java > > Editor when preferences changed 'Show, References, etc) > > You can check the eclipse.platform.common repo to add Platform and JDT > documentation. To be concrete, it should be added to the Programmer's Guide in org.eclipse.platform.doc.isv and org.eclipse.jdt.doc.isv.
Note that this added API after the freeze. Next time, please request approval from the PMC before submitting the change.
