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

Inlay hints schedule improve #125995

Closed
Kingwl opened this issue Jun 11, 2021 · 12 comments · Fixed by #133668
Closed

Inlay hints schedule improve #125995

Kingwl opened this issue Jun 11, 2021 · 12 comments · Fixed by #133668
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug inlay-hints insiders-released Patch has been released in VS Code Insiders polish Cleanup and polish issue verified Verification succeeded

Comments

@Kingwl
Copy link
Contributor

Kingwl commented Jun 11, 2021

Currently, the inlay hints will clean and shows again as scheduler's strategy.

It's might not friendly if you are editing some line who has many hints.

I have an idea:
Clean all hints where the cursor’s line when we are editing the code and append new hints by the scheduler.

I think It's might improve the experience about inlay hints.

@vscodebot
Copy link

vscodebot bot commented Jun 11, 2021

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@gjsjohnmurray
Copy link
Contributor

/gifPlease

@Kingwl
Copy link
Contributor Author

Kingwl commented Jun 11, 2021

hints_exp

@hediet
Copy link
Member

hediet commented Jul 14, 2021

I think inlay hints should disappear immediately if the surrounding text is deleted.
Afaik, currently, when text gets deleted, decorations inside of that text get collapsed but are not deleted.

@jrieken
Copy link
Member

jrieken commented Jul 15, 2021

Yes, I agree that hints should disappear immediately when their "anchor" position is removed. Maybe it's not so easy to know what remove actually means in this case. Arguably this is an editor bug as I was expecting this to work?

@hediet
Copy link
Member

hediet commented Jul 16, 2021

Arguably this is an editor bug as I was expecting this to work?

I wouldn't call this a bug - decorations are never removed automatically.

Alex suggests (I asked him before his vacation):

yes, the inlay hints could listen to change events and remove them if they become collapsed

I also suggest to extend the inlay hints API so that a hint refers to a range. The hint itself then can either be after or before the range.
The range allows to better predict what should happen with the hint if the text changes.

With injected text, we now have two cursor stops around inlay hints - both refer to the same model position. Maybe the move behavior of inlay text should depend on the view position where the text change was caused from?

I suggest to think about how we can better predict synchronously how a text change affects inlay hints.

@jrieken
Copy link
Member

jrieken commented Jul 16, 2021

I also suggest to extend the inlay hints API so that a hint refers to a range. The hint itself then can either be after or before the range. The range allows to better predict what should happen with the hint if the text changes.

This is how we started, mostly to optionally support overlay hints, and everyone was confused about ranges for inlay hints. Using a range with before/after screams "position" and I don't want to leak an implementation detail into the API. We would also still have the case of empty ranges... I'd rather create "fake ranges" from positions so that I can use the notion of collapsed ranges.

@jrieken
Copy link
Member

jrieken commented Sep 20, 2021

I have pushed a change (00d20b9) to use the range of touching words of inlay positions (when possible). In that case showIfCollapsed will be false and my expectation was that decorations hide/disappear when removing all their text.. @alexdima @hediet is that a bug in decorations land or am I using this wrong? Should I listen to decoration changes and "manually" remove them?

@hediet
Copy link
Member

hediet commented Sep 20, 2021

Should I listen to decoration changes and "manually" remove them?

I think so, decorations are not removed automatically.

@jrieken
Copy link
Member

jrieken commented Sep 20, 2021

But what does showIfCollapsed mean then?

@hediet
Copy link
Member

hediet commented Sep 20, 2021

Ah, maybe I forgot to consider that flag for injected text, I'll look into it tomorrow.

@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Sep 23, 2021
@lramos15 lramos15 added the verified Verification succeeded label Sep 30, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug inlay-hints insiders-released Patch has been released in VS Code Insiders polish Cleanup and polish issue verified Verification succeeded
Projects
None yet
6 participants
@jrieken @hediet @lramos15 @gjsjohnmurray @Kingwl and others