settings_ui: Make it so settings links open sub pages (#48212) (cherry-pick to preview) (#48219)

zed-zippy[bot] and Ben Kunkle created

Cherry-pick of #48212 to preview

----
Closes #ISSUE

Release Notes:

- Fixed an issue where opening a link to a settings item that involved a
sub page would not open the sub page

Co-authored-by: Ben Kunkle <ben@zed.dev>

Change summary

crates/settings_ui/src/settings_ui.rs | 163 +++++++++++++++---------
crates/zed/src/visual_test_runner.rs  | 191 ++++++++++++++++++++++++++++
2 files changed, 290 insertions(+), 64 deletions(-)

Detailed changes

crates/settings_ui/src/settings_ui.rs πŸ”—

@@ -562,10 +562,36 @@ pub fn open_settings_editor(
             return;
         }
 
+        let query = format!("#{path}");
+        let indices = settings_window.filter_by_json_path(&query);
+
+        settings_window.opening_link = true;
         settings_window.search_bar.update(cx, |editor, cx| {
-            editor.set_text(format!("#{path}"), window, cx);
+            editor.set_text(query, window, cx);
         });
-        settings_window.update_matches(cx);
+        settings_window.apply_match_indices(indices.iter().copied());
+
+        if indices.len() == 1
+            && let Some(search_index) = settings_window.search_index.as_ref()
+        {
+            let SearchKeyLUTEntry {
+                page_index,
+                item_index,
+                header_index,
+                ..
+            } = search_index.key_lut[indices[0]];
+            let page = &settings_window.pages[page_index];
+            let item = &page.items[item_index];
+
+            if settings_window.filter_table[page_index][item_index]
+                && let SettingsPageItem::SubPageLink(link) = item
+                && let SettingsPageItem::SectionHeader(header) = page.items[header_index]
+            {
+                settings_window.push_sub_page(link.clone(), SharedString::from(header), window, cx);
+            }
+        }
+
+        cx.notify();
     }
 
     let existing_window = cx
@@ -691,6 +717,7 @@ pub struct SettingsWindow {
     current_file: SettingsUiFile,
     pages: Vec<SettingsPage>,
     sub_page_stack: Vec<SubPage>,
+    opening_link: bool,
     search_bar: Entity<Editor>,
     search_task: Option<Task<()>>,
     /// Cached settings file buffers to avoid repeated disk I/O on each settings change
@@ -1433,12 +1460,15 @@ impl SettingsWindow {
             editor.set_placeholder_text("Search settings…", window, cx);
             editor
         });
-
         cx.subscribe(&search_bar, |this, _, event: &EditorEvent, cx| {
             let EditorEvent::Edited { transaction_id: _ } = event else {
                 return;
             };
 
+            if this.opening_link {
+                this.opening_link = false;
+                return;
+            }
             this.update_matches(cx);
         })
         .detach();
@@ -1582,6 +1612,7 @@ impl SettingsWindow {
             project_setting_file_buffers: HashMap::default(),
             pages: vec![],
             sub_page_stack: vec![],
+            opening_link: false,
             navbar_entries: vec![],
             navbar_entry: 0,
             navbar_scroll_handle: UniformListScrollHandle::default(),
@@ -1787,9 +1818,58 @@ impl SettingsWindow {
         }
     }
 
+    fn filter_by_json_path(&self, query: &str) -> Vec<usize> {
+        let Some(path) = query.strip_prefix('#') else {
+            return vec![];
+        };
+        let Some(search_index) = self.search_index.as_ref() else {
+            return vec![];
+        };
+        let mut indices = vec![];
+        for (index, SearchKeyLUTEntry { json_path, .. }) in search_index.key_lut.iter().enumerate()
+        {
+            let Some(json_path) = json_path else {
+                continue;
+            };
+
+            if let Some(post) = json_path.strip_prefix(path)
+                && (post.is_empty() || post.starts_with('.'))
+            {
+                indices.push(index);
+            }
+        }
+        indices
+    }
+
+    fn apply_match_indices(&mut self, match_indices: impl Iterator<Item = usize>) {
+        let Some(search_index) = self.search_index.as_ref() else {
+            return;
+        };
+
+        for page in &mut self.filter_table {
+            page.fill(false);
+        }
+
+        for match_index in match_indices {
+            let SearchKeyLUTEntry {
+                page_index,
+                header_index,
+                item_index,
+                ..
+            } = search_index.key_lut[match_index];
+            let page = &mut self.filter_table[page_index];
+            page[header_index] = true;
+            page[item_index] = true;
+        }
+        self.has_query = true;
+        self.filter_matches_to_file();
+        self.open_first_nav_page();
+        self.reset_list_state();
+    }
+
     fn update_matches(&mut self, cx: &mut Context<SettingsWindow>) {
         self.search_task.take();
-        let mut query = self.search_bar.read(cx).text(cx);
+        let query = self.search_bar.read(cx).text(cx);
         if query.is_empty() || self.search_index.is_none() {
             for page in &mut self.filter_table {
                 page.fill(true);
@@ -1801,68 +1881,19 @@ impl SettingsWindow {
             return;
         }
 
-        let is_json_link_query;
-        if query.starts_with("#") {
-            query.remove(0);
-            is_json_link_query = true;
-        } else {
-            is_json_link_query = false;
+        let is_json_link_query = query.starts_with("#");
+        if is_json_link_query {
+            let indices = self.filter_by_json_path(&query);
+            if !indices.is_empty() {
+                self.apply_match_indices(indices.into_iter());
+                cx.notify();
+                return;
+            }
         }
 
         let search_index = self.search_index.as_ref().unwrap().clone();
 
-        fn update_matches_inner(
-            this: &mut SettingsWindow,
-            search_index: &SearchIndex,
-            match_indices: impl Iterator<Item = usize>,
-            cx: &mut Context<SettingsWindow>,
-        ) {
-            for page in &mut this.filter_table {
-                page.fill(false);
-            }
-
-            for match_index in match_indices {
-                let SearchKeyLUTEntry {
-                    page_index,
-                    header_index,
-                    item_index,
-                    ..
-                } = search_index.key_lut[match_index];
-                let page = &mut this.filter_table[page_index];
-                page[header_index] = true;
-                page[item_index] = true;
-            }
-            this.has_query = true;
-            this.filter_matches_to_file();
-            this.open_first_nav_page();
-            this.reset_list_state();
-            cx.notify();
-        }
-
         self.search_task = Some(cx.spawn(async move |this, cx| {
-            if is_json_link_query {
-                let mut indices = vec![];
-                for (index, SearchKeyLUTEntry { json_path, .. }) in
-                    search_index.key_lut.iter().enumerate()
-                {
-                    let Some(json_path) = json_path else {
-                        continue;
-                    };
-
-                    if let Some(post) = query.strip_prefix(json_path)
-                        && (post.is_empty() || post.starts_with('.'))
-                    {
-                        indices.push(index);
-                    }
-                }
-                if !indices.is_empty() {
-                    this.update(cx, |this, cx| {
-                        update_matches_inner(this, search_index.as_ref(), indices.into_iter(), cx);
-                    })
-                    .ok();
-                    return;
-                }
-            }
             let bm25_task = cx.background_spawn({
                 let search_index = search_index.clone();
                 let max_results = search_index.key_lut.len();
@@ -1881,6 +1912,11 @@ impl SettingsWindow {
             );
 
             let fuzzy_matches = fuzzy_search_task.await;
+            // PERF:
+            // If results are slow to appear, we should:
+            // - return to the structure we had previously where we wait on fuzzy matches first (they resolve quickly) with a min match score of 0.3
+            // - wait on bm25 and replace fuzzy matches with bm25 matches
+            // - to deal with lack of fuzzyness with bm25 searches however, we should keep the fuzzy matches around, and merge fuzzy matches with high score (>0.75?) into bm25 results
             let bm25_matches = bm25_task.await;
 
             _ = this
@@ -1917,7 +1953,8 @@ impl SettingsWindow {
                         .map(|bm25_match| bm25_match.document.id);
                     let merged_indices = bm25_indices.chain(fuzzy_indices);
 
-                    update_matches_inner(this, search_index.as_ref(), merged_indices, cx);
+                    this.apply_match_indices(merged_indices);
+                    cx.notify();
                 })
                 .ok();
 
@@ -4163,6 +4200,7 @@ pub mod test {
                 content_handles: Vec::default(),
                 search_task: None,
                 sub_page_stack: Vec::default(),
+                opening_link: false,
                 focus_handle: cx.focus_handle(),
                 navbar_focus_handle: NonFocusableHandle::new(
                     NAVBAR_CONTAINER_TAB_INDEX,
@@ -4283,6 +4321,7 @@ pub mod test {
             navbar_focus_subscriptions: vec![],
             filter_table: vec![],
             sub_page_stack: vec![],
+            opening_link: false,
             has_query: false,
             content_handles: vec![],
             search_task: None,

crates/zed/src/visual_test_runner.rs πŸ”—

@@ -60,6 +60,7 @@ use {
     image::RgbaImage,
     project_panel::ProjectPanel,
     settings::{NotifyWhenAgentWaiting, Settings as _},
+    settings_ui::SettingsWindow,
     std::{
         any::Any,
         path::{Path, PathBuf},
@@ -69,6 +70,7 @@ use {
     },
     watch,
     workspace::{AppState, Workspace},
+    zed_actions::OpenSettingsAt,
 };
 
 // All macOS-specific constants grouped together
@@ -183,6 +185,7 @@ fn run_visual_tests(project_path: PathBuf, update_baseline: bool) -> Result<()>
         language_model::init(app_state.client.clone(), cx);
         language_models::init(app_state.user_store.clone(), app_state.client.clone(), cx);
         git_ui::init(cx);
+        settings_ui::init(cx);
 
         // Load default keymaps so tooltips can show keybindings like "f9" for ToggleBreakpoint
         // We load a minimal set of editor keybindings needed for visual tests
@@ -480,6 +483,23 @@ fn run_visual_tests(project_path: PathBuf, update_baseline: bool) -> Result<()>
         }
     }
 
+    // Run Test 7: Settings UI sub-page auto-open visual tests
+    println!("\n--- Test 7: settings_ui_subpage_auto_open (2 variants) ---");
+    match run_settings_ui_subpage_visual_tests(app_state.clone(), &mut cx, update_baseline) {
+        Ok(TestResult::Passed) => {
+            println!("βœ“ settings_ui_subpage_auto_open: PASSED");
+            passed += 1;
+        }
+        Ok(TestResult::BaselineUpdated(_)) => {
+            println!("βœ“ settings_ui_subpage_auto_open: Baselines updated");
+            updated += 1;
+        }
+        Err(e) => {
+            eprintln!("βœ— settings_ui_subpage_auto_open: FAILED - {}", e);
+            failed += 1;
+        }
+    }
+
     // Clean up the main workspace's worktree to stop background scanning tasks
     // This prevents "root path could not be canonicalized" errors when main() drops temp_dir
     workspace_window
@@ -845,7 +865,7 @@ fn init_app_state(cx: &mut App) -> Arc<AppState> {
     theme::init(theme::LoadThemes::JustBase, cx);
     client::init(&client, cx);
 
-    Arc::new(AppState {
+    let app_state = Arc::new(AppState {
         client,
         fs,
         languages,
@@ -854,7 +874,9 @@ fn init_app_state(cx: &mut App) -> Arc<AppState> {
         node_runtime: NodeRuntime::unavailable(),
         build_window_options: |_, _| Default::default(),
         session,
-    })
+    });
+    AppState::set_global(Arc::downgrade(&app_state), cx);
+    app_state
 }
 
 /// Runs visual tests for breakpoint hover states in the editor gutter.
@@ -1145,6 +1167,171 @@ fn run_breakpoint_hover_visual_tests(
     }
 }
 
+/// Runs visual tests for the settings UI sub-page auto-open feature.
+///
+/// This test verifies that when opening settings via OpenSettingsAt with a path
+/// that maps to a single SubPageLink, the sub-page is automatically opened.
+///
+/// This test captures two states:
+/// 1. Settings opened with a path that maps to multiple items (no auto-open)
+/// 2. Settings opened with a path that maps to a single SubPageLink (auto-opens sub-page)
+#[cfg(target_os = "macos")]
+fn run_settings_ui_subpage_visual_tests(
+    app_state: Arc<AppState>,
+    cx: &mut VisualTestAppContext,
+    update_baseline: bool,
+) -> Result<TestResult> {
+    // Create a workspace window for dispatching actions
+    let window_size = size(px(1280.0), px(800.0));
+    let bounds = Bounds {
+        origin: point(px(0.0), px(0.0)),
+        size: window_size,
+    };
+
+    let project = cx.update(|cx| {
+        project::Project::local(
+            app_state.client.clone(),
+            app_state.node_runtime.clone(),
+            app_state.user_store.clone(),
+            app_state.languages.clone(),
+            app_state.fs.clone(),
+            None,
+            project::LocalProjectFlags {
+                init_worktree_trust: false,
+                ..Default::default()
+            },
+            cx,
+        )
+    });
+
+    let workspace_window: WindowHandle<Workspace> = cx
+        .update(|cx| {
+            cx.open_window(
+                WindowOptions {
+                    window_bounds: Some(WindowBounds::Windowed(bounds)),
+                    focus: false,
+                    show: false,
+                    ..Default::default()
+                },
+                |window, cx| {
+                    cx.new(|cx| {
+                        Workspace::new(None, project.clone(), app_state.clone(), window, cx)
+                    })
+                },
+            )
+        })
+        .context("Failed to open workspace window")?;
+
+    cx.run_until_parked();
+
+    // Test 1: Open settings with a path that maps to multiple items (e.g., "agent")
+    // This should NOT auto-open a sub-page since multiple items match
+    workspace_window
+        .update(cx, |_workspace, window, cx| {
+            window.dispatch_action(
+                Box::new(OpenSettingsAt {
+                    path: "agent".to_string(),
+                }),
+                cx,
+            );
+        })
+        .context("Failed to dispatch OpenSettingsAt for multiple items")?;
+
+    cx.run_until_parked();
+
+    // Find the settings window
+    let settings_window_1 = cx
+        .update(|cx| {
+            cx.windows()
+                .into_iter()
+                .find_map(|window| window.downcast::<SettingsWindow>())
+        })
+        .context("Settings window not found")?;
+
+    // Refresh and capture screenshot
+    cx.update_window(settings_window_1.into(), |_, window, _cx| {
+        window.refresh();
+    })?;
+    cx.run_until_parked();
+
+    let test1_result = run_visual_test(
+        "settings_ui_no_auto_open",
+        settings_window_1.into(),
+        cx,
+        update_baseline,
+    )?;
+
+    // Close the settings window
+    let _ = cx.update_window(settings_window_1.into(), |_, window, _cx| {
+        window.remove_window();
+    });
+    cx.run_until_parked();
+
+    // Test 2: Open settings with a path that maps to a single SubPageLink
+    // "edit_predictions.providers" maps to the "Configure Providers" SubPageLink
+    // This should auto-open the sub-page
+    workspace_window
+        .update(cx, |_workspace, window, cx| {
+            window.dispatch_action(
+                Box::new(OpenSettingsAt {
+                    path: "edit_predictions.providers".to_string(),
+                }),
+                cx,
+            );
+        })
+        .context("Failed to dispatch OpenSettingsAt for single SubPageLink")?;
+
+    cx.run_until_parked();
+
+    // Find the new settings window
+    let settings_window_2 = cx
+        .update(|cx| {
+            cx.windows()
+                .into_iter()
+                .find_map(|window| window.downcast::<SettingsWindow>())
+        })
+        .context("Settings window not found for sub-page test")?;
+
+    // Refresh and capture screenshot
+    cx.update_window(settings_window_2.into(), |_, window, _cx| {
+        window.refresh();
+    })?;
+    cx.run_until_parked();
+
+    let test2_result = run_visual_test(
+        "settings_ui_subpage_auto_open",
+        settings_window_2.into(),
+        cx,
+        update_baseline,
+    )?;
+
+    // Clean up: close the settings window
+    let _ = cx.update_window(settings_window_2.into(), |_, window, _cx| {
+        window.remove_window();
+    });
+    cx.run_until_parked();
+
+    // Clean up: close the workspace window
+    let _ = cx.update_window(workspace_window.into(), |_, window, _cx| {
+        window.remove_window();
+    });
+    cx.run_until_parked();
+
+    // Give background tasks time to finish
+    for _ in 0..5 {
+        cx.advance_clock(Duration::from_millis(100));
+        cx.run_until_parked();
+    }
+
+    // Return combined result
+    match (&test1_result, &test2_result) {
+        (TestResult::Passed, TestResult::Passed) => Ok(TestResult::Passed),
+        (TestResult::BaselineUpdated(p), _) | (_, TestResult::BaselineUpdated(p)) => {
+            Ok(TestResult::BaselineUpdated(p.clone()))
+        }
+    }
+}
+
 /// Runs visual tests for the diff review button in git diff views.
 ///
 /// This test captures three states: