Add a way to toggle inlay hints with modifiers #2 (#25766)

Kirill Bulatov created

https://github.com/zed-industries/zed/pull/25752 with fixes on top

* Ensures no flickering happens for all modifiers `: false` case
* Dismisses the toggled state on focus out
* Reworks cache state so that "enabled" and "toggled by modifiers" are
different states with their own lifecycle

Release Notes:

- N/A

Change summary

assets/settings/default.json             | 11 ++++
crates/collab/src/tests/editor_tests.rs  |  4 +
crates/editor/src/editor.rs              | 56 +++++++++++++++++++------
crates/editor/src/element.rs             | 29 +++++++++++-
crates/editor/src/hover_links.rs         |  1 
crates/editor/src/hover_popover.rs       |  1 
crates/editor/src/inlay_hint_cache.rs    | 52 +++++++++++++++++++++++
crates/gpui/src/platform/keystroke.rs    | 10 +++
crates/language/src/language_settings.rs |  9 +++
docs/src/configuring-zed.md              | 19 ++++++++
10 files changed, 166 insertions(+), 26 deletions(-)

Detailed changes

assets/settings/default.json 🔗

@@ -401,7 +401,16 @@
     "edit_debounce_ms": 700,
     // Time to wait after scrolling the buffer, before requesting the hints,
     // set to 0 to disable debouncing.
-    "scroll_debounce_ms": 50
+    "scroll_debounce_ms": 50,
+    /// A set of modifiers which, when pressed, will toggle the visibility of inlay hints.
+    /// If the set if empty or not all the modifiers specified are pressed, inlay hints will not be toggled.
+    "toggle_on_modifiers_press": {
+      "control": false,
+      "shift": false,
+      "alt": false,
+      "platform": false,
+      "function": false
+    }
   },
   "project_panel": {
     // Whether to show the project panel button in the status bar

crates/collab/src/tests/editor_tests.rs 🔗

@@ -1537,6 +1537,7 @@ async fn test_mutual_editor_inlay_hint_cache_update(
                     show_parameter_hints: false,
                     show_other_hints: true,
                     show_background: false,
+                    toggle_on_modifiers_press: None,
                 })
             });
         });
@@ -1552,6 +1553,7 @@ async fn test_mutual_editor_inlay_hint_cache_update(
                     show_parameter_hints: false,
                     show_other_hints: true,
                     show_background: false,
+                    toggle_on_modifiers_press: None,
                 })
             });
         });
@@ -1770,6 +1772,7 @@ async fn test_inlay_hint_refresh_is_forwarded(
                     show_parameter_hints: false,
                     show_other_hints: false,
                     show_background: false,
+                    toggle_on_modifiers_press: None,
                 })
             });
         });
@@ -1785,6 +1788,7 @@ async fn test_inlay_hint_refresh_is_forwarded(
                     show_parameter_hints: true,
                     show_other_hints: true,
                     show_background: false,
+                    toggle_on_modifiers_press: None,
                 })
             });
         });

crates/editor/src/editor.rs 🔗

@@ -1031,6 +1031,7 @@ pub enum GotoDefinitionKind {
 
 #[derive(Debug, Clone)]
 enum InlayHintRefreshReason {
+    ModifiersChanged(bool),
     Toggle(bool),
     SettingsChange(InlayHintSettings),
     NewLinesShown,
@@ -1042,6 +1043,7 @@ enum InlayHintRefreshReason {
 impl InlayHintRefreshReason {
     fn description(&self) -> &'static str {
         match self {
+            Self::ModifiersChanged(_) => "modifiers changed",
             Self::Toggle(_) => "toggle",
             Self::SettingsChange(_) => "settings change",
             Self::NewLinesShown => "new lines shown",
@@ -3668,7 +3670,7 @@ impl Editor {
         cx: &mut Context<Self>,
     ) {
         self.refresh_inlay_hints(
-            InlayHintRefreshReason::Toggle(!self.inlay_hint_cache.enabled),
+            InlayHintRefreshReason::Toggle(!self.inlay_hints_enabled()),
             cx,
         );
     }
@@ -3690,21 +3692,44 @@ impl Editor {
                 | InlayHintRefreshReason::ExcerptsRemoved(_)
         );
         let (invalidate_cache, required_languages) = match reason {
+            InlayHintRefreshReason::ModifiersChanged(enabled) => {
+                match self.inlay_hint_cache.modifiers_override(enabled) {
+                    Some(enabled) => {
+                        if enabled {
+                            (InvalidationStrategy::RefreshRequested, None)
+                        } else {
+                            self.splice_inlays(
+                                &self
+                                    .visible_inlay_hints(cx)
+                                    .iter()
+                                    .map(|inlay| inlay.id)
+                                    .collect::<Vec<InlayId>>(),
+                                Vec::new(),
+                                cx,
+                            );
+                            return;
+                        }
+                    }
+                    None => return,
+                }
+            }
             InlayHintRefreshReason::Toggle(enabled) => {
-                self.inlay_hint_cache.enabled = enabled;
-                if enabled {
-                    (InvalidationStrategy::RefreshRequested, None)
+                if self.inlay_hint_cache.toggle(enabled) {
+                    if enabled {
+                        (InvalidationStrategy::RefreshRequested, None)
+                    } else {
+                        self.splice_inlays(
+                            &self
+                                .visible_inlay_hints(cx)
+                                .iter()
+                                .map(|inlay| inlay.id)
+                                .collect::<Vec<InlayId>>(),
+                            Vec::new(),
+                            cx,
+                        );
+                        return;
+                    }
                 } else {
-                    self.inlay_hint_cache.clear();
-                    self.splice_inlays(
-                        &self
-                            .visible_inlay_hints(cx)
-                            .iter()
-                            .map(|inlay| inlay.id)
-                            .collect::<Vec<InlayId>>(),
-                        Vec::new(),
-                        cx,
-                    );
                     return;
                 }
             }
@@ -15839,11 +15864,12 @@ impl Editor {
         &mut self,
         event: FocusOutEvent,
         _window: &mut Window,
-        _cx: &mut Context<Self>,
+        cx: &mut Context<Self>,
     ) {
         if event.blurred != self.focus_handle {
             self.last_focused_descendant = Some(event.blurred);
         }
+        self.refresh_inlay_hints(InlayHintRefreshReason::ModifiersChanged(false), cx);
     }
 
     pub fn handle_blur(&mut self, window: &mut Window, cx: &mut Context<Self>) {

crates/editor/src/element.rs 🔗

@@ -12,6 +12,7 @@ use crate::{
     hover_popover::{
         self, hover_at, HOVER_POPOVER_GAP, MIN_POPOVER_CHARACTER_WIDTH, MIN_POPOVER_LINE_HEIGHT,
     },
+    inlay_hint_settings,
     items::BufferSearchHighlights,
     mouse_context_menu::{self, MenuPosition, MouseContextMenu},
     scroll::{axis_pair, scroll_amount::ScrollAmount, AxisPair},
@@ -19,10 +20,11 @@ use crate::{
     DisplayRow, DocumentHighlightRead, DocumentHighlightWrite, EditDisplayMode, Editor, EditorMode,
     EditorSettings, EditorSnapshot, EditorStyle, ExpandExcerpts, FocusedBlock, GoToHunk,
     GoToPrevHunk, GutterDimensions, HalfPageDown, HalfPageUp, HandleInput, HoveredCursor,
-    InlineCompletion, JumpData, LineDown, LineUp, OpenExcerpts, PageDown, PageUp, Point, RowExt,
-    RowRangeExt, SelectPhase, SelectedTextHighlight, Selection, SoftWrap, StickyHeaderExcerpt,
-    ToPoint, ToggleFold, COLUMNAR_SELECTION_MODIFIERS, CURSORS_VISIBLE_FOR, FILE_HEADER_HEIGHT,
-    GIT_BLAME_MAX_AUTHOR_CHARS_DISPLAYED, MAX_LINE_LEN, MULTI_BUFFER_EXCERPT_HEADER_HEIGHT,
+    InlayHintRefreshReason, InlineCompletion, JumpData, LineDown, LineUp, OpenExcerpts, PageDown,
+    PageUp, Point, RowExt, RowRangeExt, SelectPhase, SelectedTextHighlight, Selection, SoftWrap,
+    StickyHeaderExcerpt, ToPoint, ToggleFold, COLUMNAR_SELECTION_MODIFIERS, CURSORS_VISIBLE_FOR,
+    FILE_HEADER_HEIGHT, GIT_BLAME_MAX_AUTHOR_CHARS_DISPLAYED, MAX_LINE_LEN,
+    MULTI_BUFFER_EXCERPT_HEADER_HEIGHT,
 };
 use buffer_diff::{DiffHunkSecondaryStatus, DiffHunkStatus, DiffHunkStatusKind};
 use client::ParticipantIndex;
@@ -505,6 +507,25 @@ impl EditorElement {
                     return;
                 }
                 editor.update(cx, |editor, cx| {
+                    let inlay_hint_settings = inlay_hint_settings(
+                        editor.selections.newest_anchor().head(),
+                        &editor.buffer.read(cx).snapshot(cx),
+                        cx,
+                    );
+
+                    if let Some(inlay_modifiers) = inlay_hint_settings
+                        .toggle_on_modifiers_press
+                        .as_ref()
+                        .filter(|modifiers| modifiers.modified())
+                    {
+                        editor.refresh_inlay_hints(
+                            InlayHintRefreshReason::ModifiersChanged(
+                                inlay_modifiers == &event.modifiers,
+                            ),
+                            cx,
+                        );
+                    }
+
                     if editor.hover_state.focused(window, cx) {
                         return;
                     }

crates/editor/src/hover_links.rs 🔗

@@ -1271,6 +1271,7 @@ mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 

crates/editor/src/hover_popover.rs 🔗

@@ -1536,6 +1536,7 @@ mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 

crates/editor/src/inlay_hint_cache.rs 🔗

@@ -36,6 +36,7 @@ pub struct InlayHintCache {
     allowed_hint_kinds: HashSet<Option<InlayHintKind>>,
     version: usize,
     pub(super) enabled: bool,
+    modifiers_override: bool,
     enabled_in_settings: bool,
     update_tasks: HashMap<ExcerptId, TasksForRanges>,
     refresh_task: Task<()>,
@@ -265,6 +266,7 @@ impl InlayHintCache {
         Self {
             allowed_hint_kinds: inlay_hint_settings.enabled_inlay_hint_kinds(),
             enabled: inlay_hint_settings.enabled,
+            modifiers_override: false,
             enabled_in_settings: inlay_hint_settings.enabled,
             hints: HashMap::default(),
             update_tasks: HashMap::default(),
@@ -295,8 +297,9 @@ impl InlayHintCache {
         // visibility would not change when updating the setting if they were ever toggled.
         if new_hint_settings.enabled != self.enabled_in_settings {
             self.enabled = new_hint_settings.enabled;
+            self.enabled_in_settings = new_hint_settings.enabled;
+            self.modifiers_override = false;
         };
-        self.enabled_in_settings = new_hint_settings.enabled;
         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();
@@ -323,6 +326,7 @@ impl InlayHintCache {
                 }
             }
             (true, false) => {
+                self.modifiers_override = false;
                 self.allowed_hint_kinds = new_allowed_hint_kinds;
                 if self.hints.is_empty() {
                     ControlFlow::Break(None)
@@ -335,12 +339,39 @@ impl InlayHintCache {
                 }
             }
             (false, true) => {
+                self.modifiers_override = false;
                 self.allowed_hint_kinds = new_allowed_hint_kinds;
                 ControlFlow::Continue(())
             }
         }
     }
 
+    pub(super) fn modifiers_override(&mut self, new_override: bool) -> Option<bool> {
+        if self.modifiers_override == new_override {
+            return None;
+        }
+        self.modifiers_override = new_override;
+        if (self.enabled && self.modifiers_override) || (!self.enabled && !self.modifiers_override)
+        {
+            self.clear();
+            Some(false)
+        } else {
+            Some(true)
+        }
+    }
+
+    pub(super) fn toggle(&mut self, enabled: bool) -> bool {
+        if self.enabled == enabled {
+            return false;
+        }
+        self.enabled = enabled;
+        self.modifiers_override = false;
+        if !enabled {
+            self.clear();
+        }
+        true
+    }
+
     /// If needed, queries LSP for new inlay hints, using the invalidation strategy given.
     /// To reduce inlay hint jumping, attempts to query a visible range of the editor(s) first,
     /// followed by the delayed queries of the same range above and below the visible one.
@@ -353,7 +384,8 @@ impl InlayHintCache {
         ignore_debounce: bool,
         cx: &mut Context<Editor>,
     ) -> Option<InlaySplice> {
-        if !self.enabled {
+        if (self.enabled && self.modifiers_override) || (!self.enabled && !self.modifiers_override)
+        {
             return None;
         }
         let mut invalidated_hints = Vec::new();
@@ -1288,6 +1320,7 @@ pub mod tests {
                 show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)),
                 show_other_hints: allowed_hint_kinds.contains(&None),
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
         let (_, editor, fake_server) = prepare_test_objects(cx, |fake_server, file_with_hints| {
@@ -1391,6 +1424,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 
@@ -1493,6 +1527,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 
@@ -1712,6 +1747,7 @@ pub mod tests {
                 show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)),
                 show_other_hints: allowed_hint_kinds.contains(&None),
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 
@@ -1871,6 +1907,7 @@ pub mod tests {
                         .contains(&Some(InlayHintKind::Parameter)),
                     show_other_hints: new_allowed_hint_kinds.contains(&None),
                     show_background: false,
+                    toggle_on_modifiers_press: None,
                 })
             });
             cx.executor().run_until_parked();
@@ -1913,6 +1950,7 @@ pub mod tests {
                     .contains(&Some(InlayHintKind::Parameter)),
                 show_other_hints: another_allowed_hint_kinds.contains(&None),
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
         cx.executor().run_until_parked();
@@ -1967,6 +2005,7 @@ pub mod tests {
                     .contains(&Some(InlayHintKind::Parameter)),
                 show_other_hints: final_allowed_hint_kinds.contains(&None),
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
         cx.executor().run_until_parked();
@@ -2038,6 +2077,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 
@@ -2169,6 +2209,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 
@@ -2467,6 +2508,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 
@@ -2811,6 +2853,7 @@ pub mod tests {
                 show_parameter_hints: false,
                 show_other_hints: false,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 
@@ -2992,6 +3035,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
         cx.executor().run_until_parked();
@@ -3023,6 +3067,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 
@@ -3114,6 +3159,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 
@@ -3187,6 +3233,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
         cx.executor().run_until_parked();
@@ -3246,6 +3293,7 @@ pub mod tests {
                 show_parameter_hints: true,
                 show_other_hints: true,
                 show_background: false,
+                toggle_on_modifiers_press: None,
             })
         });
 

crates/gpui/src/platform/keystroke.rs 🔗

@@ -1,4 +1,5 @@
-use serde::Deserialize;
+use schemars::JsonSchema;
+use serde::{Deserialize, Serialize};
 use std::{
     error::Error,
     fmt::{Display, Write},
@@ -306,24 +307,29 @@ impl std::fmt::Display for Keystroke {
 }
 
 /// The state of the modifier keys at some point in time
-#[derive(Copy, Clone, Debug, Eq, PartialEq, Default, Deserialize, Hash)]
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Default, Serialize, Deserialize, Hash, JsonSchema)]
 pub struct Modifiers {
     /// The control key
+    #[serde(default)]
     pub control: bool,
 
     /// The alt key
     /// Sometimes also known as the 'meta' key
+    #[serde(default)]
     pub alt: bool,
 
     /// The shift key
+    #[serde(default)]
     pub shift: bool,
 
     /// The command key, on macos
     /// the windows key, on windows
     /// the super key, on linux
+    #[serde(default)]
     pub platform: bool,
 
     /// The function key
+    #[serde(default)]
     pub function: bool,
 }
 

crates/language/src/language_settings.rs 🔗

@@ -9,7 +9,7 @@ use ec4rs::{
     Properties as EditorconfigProperties,
 };
 use globset::{Glob, GlobMatcher, GlobSet, GlobSetBuilder};
-use gpui::App;
+use gpui::{App, Modifiers};
 use itertools::{Either, Itertools};
 use schemars::{
     schema::{InstanceType, ObjectValidation, Schema, SchemaObject, SingleOrVec},
@@ -932,6 +932,13 @@ pub struct InlayHintSettings {
     /// Default: 50
     #[serde(default = "scroll_debounce_ms")]
     pub scroll_debounce_ms: u64,
+    /// Toggles inlay hints (hides or shows) when the user presses the modifiers specified.
+    /// If only a subset of the modifiers specified is pressed, hints are not toggled.
+    /// If no modifiers are specified, this is equivalent to `None`.
+    ///
+    /// Default: None
+    #[serde(default)]
+    pub toggle_on_modifiers_press: Option<Modifiers>,
 }
 
 fn edit_debounce_ms() -> u64 {

docs/src/configuring-zed.md 🔗

@@ -1517,7 +1517,8 @@ To interpret all `.c` files as C++, files called `MyLockFile` as TOML and files
   "show_other_hints": true,
   "show_background": false,
   "edit_debounce_ms": 700,
-  "scroll_debounce_ms": 50
+  "scroll_debounce_ms": 50,
+  "toggle_on_modifiers_press": null
 }
 ```
 
@@ -1539,6 +1540,22 @@ Use the `lsp` section for the server configuration. Examples are provided in the
 Hints are not instantly queried in Zed, two kinds of debounces are used, either may be set to 0 to be disabled.
 Settings-related hint updates are not debounced.
 
+All possible config values for `toggle_on_modifiers_press` are:
+
+```json
+"inlay_hints": {
+  "toggle_on_modifiers_press": {
+    "control": true,
+    "shift": true,
+    "alt": true,
+    "platform": true,
+    "function": true
+  }
+}
+```
+
+Unspecified values have a `false` value, hints won't be toggled if all the modifiers are `false` or not all the modifiers are pressed.
+
 ## Journal
 
 - Description: Configuration for the journal.