-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: show full type in tooltips for hints #19640
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
base: master
Are you sure you want to change the base?
Conversation
crates/ide/src/inlay_hints.rs
Outdated
pub struct InlayHintLabel { | ||
pub parts: SmallVec<[InlayHintLabelPart; 1]>, | ||
pub tooltip: Option<LazyProperty<String>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be needed here, we should instead record the hover tooltip for the ...
parts (to make that work, we probably need to add a function to HirWrite
that is called when the truncation string gets emitted).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so, should we concatenate the types at the end? 👀 I tried adding a tooltip only for ...
, but it was quite inconvenient. Displaying the tooltip for the entire type directly is a better choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature itself shouldn't require changes to our inlay hint structure here (and thus also no changes in to_proto
). Associating a tooltip with all parts doesnt make sense when every part can track tooltips themselves.
I would expect that if HirWrite
had a function like fn start_truncated(&mut self)
(and end) that gets called when the formatting infra emits a ...
we could have that implemented as:
fn start_truncated(&mut self) {
self.make_new_part();
// render full type into the new part tooltip here / (lazily)
}
fn end_truncated(&mut self) {
self.make_new_part();
}
and probably needs InlayHintLabelBuilder
to get the full type as an additional field or so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I implemented a prototype (the unit tests haven't been updated yet and the code needs refactoring), but I find it somewhat tricky. I would like to discuss potential improvements or alternative implementation ideas. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think generally this feels right, couple improvements I could think of:
start_truncated()
should probably return a bool / boolish type to mark whether the writer is interested in receiving an end marker or not (allows us to short circuit as before). If it returns true, we continue rendering and emit the end_truncated
marker, if it returns false, we bail out and write the ...
to the writer (so we don't need to fixup other uses besides inlay hints of the formatting API).
THen to make the api a bit simpler, we could approach the truncation stuff with closures, f.maybe_truncate(|f| { ... })
that only gets invoked if it won't get truncated, then we can pair the start end calls around the closure invocation in maybe_truncate
.
d741cd3
to
b9d0401
Compare
self.last_part = "…".to_owned(); | ||
LazyProperty::Lazy | ||
} else { | ||
// TODO: tricky |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is tricky here, swapping the buffer out seems correct to me?
fix #19615.