diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 9d6ec1fa4aa373bb808a00bf563ab15150fcf4ce..3c384bb4b708444528c0b641939e13306471a2f4 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/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, sub_page_stack: Vec, + opening_link: bool, search_bar: Entity, search_task: Option>, /// 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 { + 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) { + 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) { 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, - cx: &mut Context, - ) { - 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, diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index e839ecf68607f70276147ab1f64229550489e13c..d76796e0286cd1e865e1e915c2e6e2eb18b2eccc 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/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 { 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 { 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, + cx: &mut VisualTestAppContext, + update_baseline: bool, +) -> Result { + // 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 = 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::()) + }) + .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::()) + }) + .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: