Bug 527675 - [code mining] Provide inline annotations support
Summary: [code mining] Provide inline annotations support
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 4.8   Edit
Hardware: All All
: P2 enhancement (vote)
Target Milestone: 4.8 M4   Edit
Assignee: Angelo ZERR CLA
QA Contact:
URL:
Whiteboard:
Keywords: Documentation, noteworthy
Depends on:
Blocks: 527515 527720
  Show dependency tree
 
Reported: 2017-11-23 07:59 EST by Angelo ZERR CLA
Modified: 2018-04-05 17:19 EDT (History)
4 users (show)

See Also:


Attachments
Inline annotations demo (499.70 KB, image/gif)
2017-11-23 07:59 EST, Angelo ZERR CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Angelo ZERR CLA 2017-11-23 07:59:30 EST
Created attachment 271608 [details]
Inline annotations demo

To support CodeMining https://bugs.eclipse.org/bugs/show_bug.cgi?id=527515, we need to provide a support to draw inline annotations:

 * draw inline annotation before a line. This kind of inline annotations can be drawn inside the line spacing (by consuming the new API StyledTextLineSpacingProvider). A sample is for CodeMining to draw references, implementations, etc before a Java class.
 * draw inline annotation inside an existing line. This kind of inline annotations can use GlyphMetrics to take some width place. A sample for CSS color where you can draw a colorized square before the rgb declaration.

You can see those inline annotations in the attached demo.
Comment 1 Eclipse Genie CLA 2017-11-23 11:32:06 EST
New Gerrit change created: https://git.eclipse.org/r/112192
Comment 2 Angelo ZERR CLA 2017-11-23 15:12:02 EST
@Mickael thanks for your review.

Please note some limitations with my gerrit patch:

 * you cannot draw "block" annotation for the first line.
 * not tested when there is apply TextPresentation, I think there will have some trouble since I set the stylerange for GlyphMetrics (I don't merge it).
 * Font changed (like zoom) is not supported, so I think there will have some trouble inside Eclipse IDE when we will use Zoom

I hope those limitations will not block the contribution. I think we could improve this gerrit patch after.
Comment 3 Mickael Istria CLA 2017-11-24 02:10:46 EST
I agree those limitations aren't blockers. Let's try to get the patch merged first in a decent state. In worst case, if we feel the quality isn't there yet for Photon, we'll put the classes in an internal package to leave room for improvement while not blocking early adopters.
Comment 4 Angelo ZERR CLA 2017-11-26 12:55:43 EST
@Mickael, I have continued in my side to develop CodeMining with this inline annotation support and I have tested inside Eclipse IDE (with Generic Editor) and it works very well. CodeMining is based on LineHeaderAnnotation.

However I have tested LineContentAnnotation (with GlyphMetrics) and there is some limitation like when we do a Zoom. I think we can improve too the width compute by caching it (when annotation doesn't change the text).

In otherwise LineHeaderAnnotation works great but LineContentAnnotation have some limitation/improvement to do. I tell me if you wish I try to fix it in this gerrit patch (1) or a new gerrit patch (2) (I will prefer that to speed the contribution)?

> I agree those limitations aren't blockers. 

It seems that you are OK with (2), but I prefer tell you that it will need perhaps a litte changed with GlyphMetrics. Tell me what you wish.
Comment 5 Mickael Istria CLA 2017-11-27 08:46:51 EST
@Angelo: at this point, the code is in a good enough state and the issue you mention are not design issue that would cascade to huge refactoring, but more internal bug fixes. We can treat them as different bugs/patches later.
Comment 6 Angelo ZERR CLA 2017-11-27 08:50:34 EST
@Mickael thanks for your answer.I wanted just to say you those limitation, and I'm glad that we will able to fix in another gerrit patch, that's cool:)
Comment 7 Angelo ZERR CLA 2017-11-29 05:37:00 EST
Guys do you think when you will able to review my gerrit patch? I would like to continue to contribute with other gerrit patch for CodeMining. I'm waiting for the accept of this gerrit patch. Any feedback are welcome, thanks!
Comment 9 Angelo ZERR CLA 2017-11-29 18:18:03 EST
Thanks @Mickael to have merged inlined support!

Now how can we do for JFace text examples? Is it possible for you to create an empty project and after I could add my demo with inlined annotations?

I need this project too for CodeMining (I have a demo too for that).
Comment 10 Mickael Istria CLA 2017-11-30 02:18:07 EST
Sure, if no existing example project is a good host for your example, feel free to creatw a new one. I'd be glad to help you by providing the poms in another commit.
Comment 11 Eclipse Genie CLA 2017-11-30 03:40:06 EST
New Gerrit change created: https://git.eclipse.org/r/112611
Comment 12 Angelo ZERR CLA 2017-11-30 03:41:00 EST
@Mickael please see https://git.eclipse.org/r/112611 which adds the inlined annotation demo.
Comment 14 Angelo ZERR CLA 2017-11-30 07:12:21 EST
Thanks @Mickael!
Comment 15 Eclipse Genie CLA 2017-11-30 08:56:57 EST
New Gerrit change created: https://git.eclipse.org/r/112635
Comment 16 Eclipse Genie CLA 2017-11-30 09:00:23 EST
New Gerrit change created: https://git.eclipse.org/r/112637
Comment 18 Noopur Gupta CLA 2017-11-30 09:25:25 EST
(In reply to Eclipse Genie from comment #13)
> Gerrit change https://git.eclipse.org/r/112611 was merged to [master].
This causes API version errors at two locations: Invalid @since tag '3.13.0' on org.eclipse.jface.text.source.inlined.Positions; expecting two fragments.

Fixed with comment #17.
Comment 20 Dani Megert CLA 2017-11-30 10:34:11 EST
I get API Tools errors with the committed changes PLEASE(!!!) make sure set up your environment with the correct API baseline. This is especially required by committers. Please fix or revert.
Comment 21 Eclipse Genie CLA 2017-11-30 10:49:13 EST
New Gerrit change created: https://git.eclipse.org/r/112645
Comment 22 Noopur Gupta CLA 2017-11-30 10:55:51 EST
(In reply to Dani Megert from comment #20)
> I get API Tools errors with the committed changes PLEASE(!!!) make sure set
> up your environment with the correct API baseline. This is especially
> required by committers. Please fix or revert.

API tools initially showed only two errors which I fixed in comment #18. A re-build now showed 3 more errors (fixed in comment #21).

Angelo, please set your API baseline to the latest available M-build for 4.7.2 and check if any other issues are missed.
Comment 24 Eclipse Genie CLA 2017-11-30 11:06:22 EST
New Gerrit change created: https://git.eclipse.org/r/112648
Comment 26 Mickael Istria CLA 2017-12-01 01:33:42 EST
@Angelo: can you please provide a note with a screnshot of the example in the N&N?
Comment 27 Eclipse Genie CLA 2017-12-01 12:34:47 EST
New Gerrit change created: https://git.eclipse.org/r/112726
Comment 28 Angelo ZERR CLA 2017-12-01 12:35:32 EST
@Mickael, please see my N&N at https://git.eclipse.org/r/#/c/112726/ 
Hope it will be OK.
Comment 30 Mickael Istria CLA 2017-12-01 13:28:37 EST
Thanks and congrats to Angelo for this important patch that enables a new era of UXin the IDE!