Bug 533194 - [code mining] Cannot position the cursor on the first letter
Summary: [code mining] Cannot position the cursor on the first letter
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.7.1   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.12 M1   Edit
Assignee: Mickael Istria CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 529011
  Show dependency tree
 
Reported: 2018-04-04 04:26 EDT by Lars Vogel CLA
Modified: 2019-03-22 06:04 EDT (History)
5 users (show)

See Also:


Attachments
Screencast as animated gif (1.92 MB, image/gif)
2018-04-04 04:26 EDT, Lars Vogel CLA
no flags Details
CSS inlined annotation in VSCode (81.63 KB, image/gif)
2019-03-19 04:57 EDT, Angelo ZERR CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2018-04-04 04:26:29 EDT
Created attachment 273412 [details]
Screencast as animated gif

I wanted to add a prefix "Lars" to a method. Unfortunately with activated code minining for JDT this is not possible. I can only position cursor:

1.) After the first letter
2.) Before the coding minining information. 

See animated gif.
Comment 1 Angelo ZERR CLA 2018-04-04 04:34:32 EDT
I think this issue will be fixed when https://bugs.eclipse.org/bugs/show_bug.cgi?id=531769 will be fixed.

Indeed, to takes some place, we apply GlyphMetrics width on the offset of the character where mining must be drawn. As GlyphMetrics replace character, we need to redraw the character and you have the cursor problem, you cannot select the character etc. 

The idea of https://bugs.eclipse.org/bugs/show_bug.cgi?id=531769 is to give the capability to apply GlyphMetrics width without replacing the character. It requires update of TextLayout for each OS (see my gerrit patch that I have done for Windows OS).

We need people who work on this TextLayout for each OS. For the moment this issue cannot be fixed.
Comment 2 Mickael Istria CLA 2018-04-04 05:59:52 EDT
(In reply to Angelo ZERR from comment #1)
> I think this issue will be fixed when
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=531769 will be fixed.

Right, it's still the same family of issues about allowing some GlyphMetrics to be interpreted as "non-payload" space in various situation (computing actual cursor size or position is one of them)
Comment 3 Lars Vogel CLA 2018-04-09 09:01:39 EDT
This also prevents me from copying statements. For example, I have:

assertNotNull(part.getElementId(), part.getIconURI());

I cannot select "part.getElementId()" only "art.getElementId()".
Comment 4 Mickael Istria CLA 2019-03-15 19:51:48 EDT
(In reply to Lars Vogel from comment #3)
> I cannot select "part.getElementId()" only "art.getElementId()".

You need to also include the codemining annotation if you want to select the character that "hosts" the code mining. This is more usually achieved when you select from right to left (not saying it's a pleasant thing to do, but maybe it can help until we manage to improve this.

The big issue here is that there cannot be a curser just before and just after code mining with the current implementation. The codemining is really part of the character, and the caret moves from a character to another. There is no concept of code mining from caret POV, so it cannot be either before or after the code mining, it can only be before the character (code mining + letter) or after the letter.
That's the current fundamental cause of all troubles.
Comment 5 Mickael Istria CLA 2019-03-15 19:53:36 EDT
But I'll investigate. It seems to me that if we print the code mining at the right of the previous character, then the behavior would feel a bit better.
Comment 6 Mickael Istria CLA 2019-03-18 12:04:12 EDT
Maybe as a workaround we can just hide the code mining annotation when the caret is on the character where the code mining is attached. What do you think?
Comment 7 Angelo ZERR CLA 2019-03-18 12:06:34 EDT
@Mickael I kno wit's an hard issue, but I think really https://bugs.eclipse.org/bugs/show_bug.cgi?id=531769 should be fixed and will fix all issues with line content annotation. There are so many bugs with this kind of annotation.
Comment 8 Mickael Istria CLA 2019-03-18 12:17:16 EDT
(In reply to Angelo ZERR from comment #7)
> @Mickael I kno wit's an hard issue, but I think really
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=531769 should be fixed and
> will fix all issues with line content annotation. There are so many bugs
> with this kind of annotation.

How do you think fixing bug 531769 would fix this issue?
The root issue here is conceptual: there is only one cursor and 2 candidate positions that make sense (before the annotation -just after the parenthesis in the gif- or after the annotation before the character -quote in the gif-). This is not really depending on an implementation detail, it's conceptually a new issue which will occur whichever implementation we keep to render the code mining, and an issue that vanishes when we don't draw the mining.
Comment 9 Angelo ZERR CLA 2019-03-18 12:32:34 EDT
> How do you think fixing bug 531769 would fix this issue?

I think (but I'm not sure)

The main problem is that line content annotation redraws the first character. In the demo it's the 'P' character of "PerspectiveBuilder" string which is redrawn. So there is none way to select this 'P' character.

When bug 531769 will be fixed, we will able to select the 'P' character and left move of cursor from right 'P' will set the cursor before the 'P':

---------------------------
([ key: ]P|
---------------------------

when we move cursor to left, we will have that:

---------------------------
([ key: ]|P
---------------------------

and not like today

---------------------------
(|[ key: ]P
---------------------------

This case occurs because 'P' is redrawn and we cannot select it.
Comment 10 Angelo ZERR CLA 2019-03-19 04:57:29 EDT
Created attachment 277924 [details]
CSS inlined annotation in VSCode

Here a demo with CSS Language server in VSCode with inlined annotation. It should be really cool if Eclipse CodeMining could have this same behaviour.
Comment 11 Mickael Istria CLA 2019-03-19 05:02:04 EDT
(In reply to Angelo ZERR from comment #10)
> Created attachment 277924 [details]
> CSS inlined annotation in VSCode
> 
> Here a demo with CSS Language server in VSCode with inlined annotation. It
> should be really cool if Eclipse CodeMining could have this same behaviour.

It's the exact same issue, but the other way round, we cannot put the caret just after `:` ;)
We could easily tweak the annotation rendered to behave the same if that seems good to everyone.
@Lars: would the same behavior as Angelo shows in VSCode be a good resolution to you or do you feel the need for being able to place it after the `(` (your gif) or `:` (Angelo's gif) ?
Comment 12 Eclipse Genie CLA 2019-03-21 07:34:59 EDT
New Gerrit change created: https://git.eclipse.org/r/139219
Comment 13 Lars Vogel CLA 2019-03-21 07:45:57 EDT
Vscode behaviour looks better compared to the current situation.
Comment 14 Mickael Istria CLA 2019-03-21 08:08:16 EDT
@Lars: can you please try the Gerrit patch?
Bug 531769 would help in many ways, but a workaround is possible ;)
Comment 16 Lars Vogel CLA 2019-03-22 06:04:26 EDT
(In reply to Mickael Istria from comment #14)
> @Lars: can you please try the Gerrit patch?

Will test in one of the next i-builds.