Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cursor stops around inlay hints #127703

Open
mjbvz opened this issue Jun 30, 2021 · 15 comments
Open

Add cursor stops around inlay hints #127703

mjbvz opened this issue Jun 30, 2021 · 15 comments
Assignees
Labels
feature-request Request for new features or functionality inlay-hints
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Jun 30, 2021

This is some generic feedback on the editor inline hints implementation, not this TS-specific support, but I wasn’t sure where else to share it. I found the experience of splitting up a long function call with parameter name hints extremely disorienting, because I wanted to put my cursor before the hint to move it down a line with the argument. But the cursor position only exists after the hint, so the impression you get as you make a refactor like this is that you’re actually inserting a newline between the label and the argument it’s labeling. And indeed, there is a moment of delay before the label disappears and reappears on the new line:

Kapture.2021-06-17.at.09.59.18.mp4

I obviously understand what’s happening and how I need to adjust for it—I’m exaggerating my struggle in this screen cap to demonstrate the nature of why it feels unintuitive—but it really is kind of jarring. It seems to me that you’d either want to duplicate the cursor position on either side of these labels, so it appears as if there are two cursor positions, even though they’re actually the same position in the source text, or you’d want an API that allows these labels to be “sticky” on either the start or end, which would control which side of the label you can place the cursor. Prefix labels like these would be sticky on their end side, such that you can’t separate the label from the argument that follows it. This would result not only in a more intuitive editing experience, but also fewer reflows of label position mid-edit like you see in every newline insertion here.

Originally posted by @andrewbranch in #113412 (comment)

@jrieken
Copy link
Member

jrieken commented Jul 1, 2021

. It seems to me that you’d either want to duplicate the cursor position on either side of these labels, so it appears as if there are two cursor positions, even though they’re actually the same position in the source text

💯 that's basically why we haven't moved forward with this. Today, inlay hints are implemented with ::before/after pseudo elements and that has all kind of funny and annoying issues. @hediet and @alexdima work on true support for inline text and that should resolve these kind of problems

@hediet hediet self-assigned this Jul 1, 2021
@alexdima alexdima changed the title Feedback on the editor inline hints implementation Add cursor stop around inlay hints Jul 1, 2021
@alexdima alexdima changed the title Add cursor stop around inlay hints Add cursor stops around inlay hints Jul 1, 2021
@alexdima alexdima self-assigned this Jul 1, 2021
@mjbvz mjbvz removed their assignment Jul 7, 2021
@Kingwl
Copy link
Contributor

Kingwl commented Jul 8, 2021

Very interested in this, please let me know how I can help If I may.

@jrieken jrieken added the feature-request Request for new features or functionality label Jul 8, 2021
@hediet
Copy link
Member

hediet commented Jul 8, 2021

We are already working on something, but thanks @Kingwl!
It should land in VS Code Insiders in the next couple of days.

@hediet hediet added this to the July 2021 milestone Jul 8, 2021
@Kingwl
Copy link
Contributor

Kingwl commented Jul 8, 2021

Cool! Thanks!

@Kingwl
Copy link
Contributor

Kingwl commented Jul 8, 2021

I guess it's https://github.com/microsoft/vscode/commits/hediet/injected-text-position-normalization this branch? Very looking forward on it.

@Kingwl
Copy link
Contributor

Kingwl commented Jul 9, 2021

I guess #128140 has fixed this?

@hediet
Copy link
Member

hediet commented Jul 9, 2021

not yet, the new API needs to be used. I'm on it.

@yume-chan
Copy link
Contributor

image

TypeScript inlay hints has switched to the new "true element" mode!

It seems to me that you’d either want to duplicate the cursor position on either side of these labels, so it appears as if there are two cursor positions, even though they’re actually the same position in the source text,

Now there ARE cursor stops on both sides of inlay hints, but when moving cursor with arrow keys, it only stops at one of them depending on which direction the cursor is moving.

1.mp4

I think it should stop on both, as if these inlay hints are real text in editors.

@hediet
Copy link
Member

hediet commented Jul 20, 2021

I think it should stop on both, as if these inlay hints are real text in editors.

IntelliJ does this.
There are some problems though:

  • When using multiple cursors, moving left/right would not move the text position of all cursors if some of them are before/after inlay hints. This might be unexpected.
  • Sometimes inlay hints need some time to be computed and appear only after some delay. This could happen while the users navigates in the line and could be annoying.

The current behavior ensures that cursor navigation without and with inlay hints is the same.

However, I'm open to change this.

@jrieken
Copy link
Member

jrieken commented Jul 22, 2021

However, I'm open to change this.

Yeah, I think that should be tried. I believe there is desire to have these virtual stops before and after the inlay text, at least as an option

@jrieken jrieken modified the milestones: July 2021, August 2021 Jul 22, 2021
@hediet
Copy link
Member

hediet commented Jul 22, 2021

What about line wrapping points?
When a line is wrapped, there are also two cursor stops that point to the same model position (last column in the first view line of the wrapped line and first column in the second view line).
Currently, we skip one of those when navigating right/left (which is important when using multiple cursors).

@seansfkelley
Copy link

seansfkelley commented Oct 2, 2021

The multiple stop points is nice, but I find myself doing this more often than I'd like:

Screen.Recording.2021-10-02.at.17.38.44.mov

If it's not clear: I'm arrowing over from the right side of a function return type inlay hint in an attempt to get to the left side of the inlay so I can add an explicit type annotation (: boolean, in this case). But when you come from the right side, it stops only at the stop point to the right of the inlay. I would either have to stop there and write : boolean (which feels wrong, because there's a thing in between the cursor and the ) which I am trying to append my changes to) or go past the desired insertion point and approach the hint from the left side (which I don't think to do, since I'm coming from the right side).

I normally use a block cursor, though this screen recording doesn't show it because I took it on an unconfigured Insiders build (to see if it behaved differently). For what it's worth, this could possibly be resolved for us block-cursor users without even requiring multiple stop points, if the hint was considered part of the character after it for the purposes of block cursor sizing.

As it stands, the block cursor only highlights the first character of the inlay hint when you approach from the left:

image

Oversize block cursors are awkward-looking, but they're not unheard of, indeed, there's one right next to this:

image

Here's my crude mockup of how it could look with only one stop point and an oversize cursor:

image

@OldStarchy
Copy link

I think using a single stop, and having it highlight the hint like @seansfkelley's mockup (regardless of cursor type) makes it more intuitive.

If you do move your cursor to that position, intuition tells me that you're likely about to do something that is going to invalidate the hint anyway (again see video above). Typing then would remove the inlay hint and it wouldn't return until you stop typing (#133730).

Doing this over having two "virtual" stops naturally avoids the issues mentioned above.

@ulugbekna
Copy link
Contributor

Another instance of the problem, which is a bit worse even with a block cursor -- note, how I can't put cursor on r and d in default is actually r:

Screen.Recording.2023-06-26.at.16.23.22.mov

@KurtGokhan
Copy link

KurtGokhan commented Aug 5, 2023

Is there a consensus on this? If not, I would like to provide my thoughts.

Usually, inlay hints are either a prefix, or suffix. For instance, when it is a suffix, you usually want to put cursor stop on the start. I think having an option like this would please everyone.

  • auto (or directional, The current behavior. The cursor stop is skipped depending on the direction of cursor navigation)
  • both (There is a cursor stop on both sides and they are not skipped)
  • start (Cursor stop is always on start. End is always skipped)
  • end(Opposite of start)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality inlay-hints
Projects
None yet
Development

No branches or pull requests

10 participants