oak-team/oak-gpui
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Debounce refresh of inlay hints on buffer edits (#8282)

Browse Source 
I think this makes it less chaotic to edit text when the inlay hints are
on.

It's for cases where you're editing to the right side of an inlay hint.
Example:

```rust
for name in names.iter().map(|item| item.len()) {
    println!("{:?}", name);
}
```

We display a `usize` inlay hint right next to `name`.

But as soon as you remove that `.` in `names.iter` your cursor jumps
around because the inlay hint has been removed.

With this change we now have a 700ms debounce before we update the inlay
hints.

VS Code seems to have an even longer debounce, I think somewhere around
~1s.

Release Notes:

- Added debouncing to make it easier to edit text when inlay hints are
enabled and to save rendering of inlay hints when scrolling. Both
debounce durations can be configured with `{"inlay_hints":
{"edit_debounce_ms": 700}}` (default) and `{"inlay_hints":
{"scroll_debounce_ms": 50}}`. Set a value to `0` to turn off the
debouncing.


### Before


https://github.com/zed-industries/zed/assets/1185253/3afbe548-dcfb-45a3-ab9f-cce14c04a148



### After



https://github.com/zed-industries/zed/assets/1185253/7ea90e42-bca6-4f6c-995e-83324669ab43

---------

Co-authored-by: Kirill <kirill@zed.dev>
This commit is contained in:
Thorsten Ball
2024-02-27 11:18:13 +01:00
committed by GitHub
co-authored by Kirill
parent cbdc07dcd0
commit ddca6a3fb7
8 changed files with 137 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/editor/src/editor.rs
+8
View File
@@ -1360,6 +1360,7 @@ enum InlayHintRefreshReason {
RefreshRequested,
ExcerptsRemoved(Vec<ExcerptId>),
}
impl InlayHintRefreshReason {
fn description(&self) -> &'static str {
match self {
@@ -3029,6 +3030,12 @@ impl Editor {
}
let reason_description = reason.description();
let ignore_debounce = matches!(
reason,
InlayHintRefreshReason::SettingsChange(_)
| InlayHintRefreshReason::Toggle(_)
| InlayHintRefreshReason::ExcerptsRemoved(_)
);
let (invalidate_cache, required_languages) = match reason {
InlayHintRefreshReason::Toggle(enabled) => {
self.inlay_hint_cache.enabled = enabled;
@@ -3091,6 +3098,7 @@ impl Editor {
reason_description,
self.excerpts_for_inlay_hints_query(required_languages.as_ref(), cx),
invalidate_cache,
ignore_debounce,
cx,
) {
self.splice_inlay_hints(to_remove, to_insert, cx);
crates/editor/src/hover_links.rs
+2
View File
@@ -984,6 +984,8 @@ mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
crates/editor/src/hover_popover.rs
+2
View File
@@ -1066,6 +1066,8 @@ mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
crates/editor/src/inlay_hint_cache.rs
+82 -17
View File
@@ -37,6 +37,9 @@ pub struct InlayHintCache {
version: usize,
pub(super) enabled: bool,
update_tasks: HashMap<ExcerptId, TasksForRanges>,
refresh_task: Option<Task<()>>,
invalidate_debounce: Option<Duration>,
append_debounce: Option<Duration>,
lsp_request_limiter: Arc<Semaphore>,
}
@@ -267,6 +270,9 @@ impl InlayHintCache {
enabled: inlay_hint_settings.enabled,
hints: HashMap::default(),
update_tasks: HashMap::default(),
refresh_task: None,
invalidate_debounce: debounce_value(inlay_hint_settings.edit_debounce_ms),
append_debounce: debounce_value(inlay_hint_settings.scroll_debounce_ms),
version: 0,
lsp_request_limiter: Arc::new(Semaphore::new(MAX_CONCURRENT_LSP_REQUESTS)),
}
@@ -282,6 +288,8 @@ impl InlayHintCache {
visible_hints: Vec<Inlay>,
cx: &mut ViewContext<Editor>,
) -> ControlFlow<Option<InlaySplice>> {
self.invalidate_debounce = debounce_value(new_hint_settings.edit_debounce_ms);
self.append_debounce = debounce_value(new_hint_settings.scroll_debounce_ms);
let new_allowed_hint_kinds = new_hint_settings.enabled_inlay_hint_kinds();
match (self.enabled, new_hint_settings.enabled) {
(false, false) => {
@@ -332,15 +340,15 @@ impl InlayHintCache {
/// This way, concequent refresh invocations are less likely to trigger LSP queries for the invisible ranges.
pub(super) fn spawn_hint_refresh(
&mut self,
reason: &'static str,
reason_description: &'static str,
excerpts_to_query: HashMap<ExcerptId, (Model<Buffer>, Global, Range<usize>)>,
invalidate: InvalidationStrategy,
ignore_debounce: bool,
cx: &mut ViewContext<Editor>,
) -> Option<InlaySplice> {
if !self.enabled {
return None;
}
let mut invalidated_hints = Vec::new();
if invalidate.should_invalidate() {
self.update_tasks
@@ -358,12 +366,23 @@ impl InlayHintCache {
}
let cache_version = self.version + 1;
cx.spawn(|editor, mut cx| async move {
let debounce_duration = if ignore_debounce {
None
} else if invalidate.should_invalidate() {
self.invalidate_debounce
} else {
self.append_debounce
};
self.refresh_task = Some(cx.spawn(|editor, mut cx| async move {
if let Some(debounce_duration) = debounce_duration {
cx.background_executor().timer(debounce_duration).await;
}
editor
.update(&mut cx, |editor, cx| {
spawn_new_update_tasks(
editor,
reason,
reason_description,
excerpts_to_query,
invalidate,
cache_version,
@@ -371,8 +390,7 @@ impl InlayHintCache {
)
})
.ok();
})
.detach();
}));
if invalidated_hints.is_empty() {
None
@@ -612,6 +630,14 @@ impl InlayHintCache {
}
}
fn debounce_value(debounce_ms: u64) -> Option<Duration> {
if debounce_ms > 0 {
Some(Duration::from_millis(debounce_ms))
} else {
None
}
}
fn spawn_new_update_tasks(
editor: &mut Editor,
reason: &'static str,
@@ -1259,6 +1285,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)),
show_other_hints: allowed_hint_kinds.contains(&None),
@@ -1389,6 +1417,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
@@ -1506,6 +1536,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
@@ -1734,6 +1766,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)),
show_other_hints: allowed_hint_kinds.contains(&None),
@@ -1895,6 +1929,8 @@ pub mod tests {
update_test_language_settings(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: new_allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
show_parameter_hints: new_allowed_hint_kinds
.contains(&Some(InlayHintKind::Parameter)),
@@ -1939,6 +1975,8 @@ pub mod tests {
update_test_language_settings(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: false,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: another_allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
show_parameter_hints: another_allowed_hint_kinds
.contains(&Some(InlayHintKind::Parameter)),
@@ -1997,6 +2035,8 @@ pub mod tests {
update_test_language_settings(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: final_allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
show_parameter_hints: final_allowed_hint_kinds
.contains(&Some(InlayHintKind::Parameter)),
@@ -2071,6 +2111,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
@@ -2203,6 +2245,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
@@ -2361,6 +2405,11 @@ pub mod tests {
editor
.update(cx, |editor, cx| {
editor.scroll_screen(&ScrollAmount::Page(1.0), cx);
})
.unwrap();
cx.executor().run_until_parked();
editor
.update(cx, |editor, cx| {
editor.scroll_screen(&ScrollAmount::Page(1.0), cx);
})
.unwrap();
@@ -2497,6 +2546,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
@@ -2782,6 +2833,9 @@ pub mod tests {
});
})
.unwrap();
cx.executor().advance_clock(Duration::from_millis(
INVISIBLE_RANGES_HINTS_REQUEST_DELAY_MILLIS + 100,
));
cx.executor().run_until_parked();
editor.update(cx, |editor, cx| {
let expected_hints = vec![
@@ -2816,12 +2870,12 @@ pub mod tests {
cx.executor().run_until_parked();
editor.update(cx, |editor, cx| {
let expected_hints = vec![
"main hint(edited) #0".to_string(),
"main hint(edited) #1".to_string(),
"main hint(edited) #2".to_string(),
"main hint(edited) #3".to_string(),
"main hint(edited) #4".to_string(),
"main hint(edited) #5".to_string(),
"main hint #0".to_string(),
"main hint #1".to_string(),
"main hint #2".to_string(),
"main hint #3".to_string(),
"main hint #4".to_string(),
"main hint #5".to_string(),
"other hint(edited) #0".to_string(),
"other hint(edited) #1".to_string(),
];
@@ -2834,11 +2888,12 @@ pub mod tests {
assert_eq!(expected_hints, visible_hint_labels(editor, cx));
let current_cache_version = editor.inlay_hint_cache().version;
let expected_version = last_scroll_update_version + expected_hints.len();
assert!(
current_cache_version == expected_version || current_cache_version == expected_version + 1 ,
// TODO we sometimes get an extra cache version bump, why?
"We should have updated cache N times == N of new hints arrived (separately from each excerpt), or hit a bug and do that one extra time"
// We expect two new hints for the excerpts from `other.rs`:
let expected_version = last_scroll_update_version + 2;
assert_eq!(
current_cache_version,
expected_version,
"We should have updated cache N times == N of new hints arrived (separately from each edited excerpt)"
);
}).unwrap();
}
@@ -2848,6 +2903,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: false,
show_parameter_hints: false,
show_other_hints: false,
@@ -3049,6 +3106,8 @@ pub mod tests {
update_test_language_settings(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
@@ -3082,6 +3141,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
@@ -3180,6 +3241,8 @@ pub mod tests {
init_test(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: false,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,
@@ -3258,6 +3321,8 @@ pub mod tests {
update_test_language_settings(cx, |settings| {
settings.defaults.inlay_hints = Some(InlayHintSettings {
enabled: true,
edit_debounce_ms: 0,
scroll_debounce_ms: 0,
show_type_hints: true,
show_parameter_hints: true,
show_other_hints: true,