From 09ceec0a36539b7473fd91860675800e1b80a2f5 Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Tue, 20 Jan 2026 01:27:42 +0100 Subject: [PATCH] settings_ui: Prevent panic when trying to configure edit prediction providers for language (#47162) Closes #46502 The issue here was that we were not looking into the sub page stack when looking headers up, resulting in an out of bounds index. This PR fixes this. Due to me also fixing another small bug in the UI (and adding proper support for the breadcrumbs), I had to move quite some stuff around here to get this to work. Namely, I made the `sub_page_stack` a field on the `SettingsWindow` and now only store the `active_language` in a global to ensure that we store scroll positions properly for all sub pages. For that to work with the edit prediction provider page, I had to remove the struct there and could just move that into a method, which was a nice side effect there I suppose. Release Notes: - Fixed a crash that could occur when trying to edit edit prediction providers in the settings UI. --- Cargo.lock | 1 + crates/settings_ui/Cargo.toml | 1 + crates/settings_ui/src/page_data.rs | 68 ++-- crates/settings_ui/src/pages.rs | 2 +- .../pages/edit_prediction_provider_setup.rs | 176 +++++---- crates/settings_ui/src/settings_ui.rs | 335 ++++++++++-------- 6 files changed, 293 insertions(+), 290 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7d216658353c692613499640b04c591c60b6d033..b64a37844cb3368abb3b845e37829ee48df03554 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15035,6 +15035,7 @@ dependencies = [ "fuzzy", "gpui", "heck 0.5.0", + "itertools 0.14.0", "language", "language_models", "log", diff --git a/crates/settings_ui/Cargo.toml b/crates/settings_ui/Cargo.toml index 94630e860f31f9d8f0e624f32253be4515981760..70f2b044e67f674ec714076d19d7e3f3dc9ba18a 100644 --- a/crates/settings_ui/Cargo.toml +++ b/crates/settings_ui/Cargo.toml @@ -27,6 +27,7 @@ fs.workspace = true fuzzy.workspace = true gpui.workspace = true heck.workspace = true +itertools.workspace = true log.workspace = true menu.workspace = true paths.workspace = true diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index 51f1d81e29b3dc40c9838477ee2b98b0a9ad2960..dc9d1d81aed51da52e15cc030630d9d73991bac3 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -2,11 +2,12 @@ use gpui::{Action as _, App}; use settings::{LanguageSettingsContent, SettingsContent}; use std::sync::Arc; use strum::IntoDiscriminant as _; -use ui::{IntoElement, SharedString}; +use ui::IntoElement; use crate::{ ActionLink, DynamicItem, PROJECT, SettingField, SettingItem, SettingsFieldMetadata, - SettingsPage, SettingsPageItem, SubPageLink, USER, all_language_names, sub_page_stack, + SettingsPage, SettingsPageItem, SubPageLink, USER, active_language, all_language_names, + pages::render_edit_prediction_setup_page, }; const DEFAULT_STRING: String = String::new(); @@ -2913,24 +2914,30 @@ fn languages_and_tools_page(cx: &App) -> SettingsPage { fn languages_list_section(cx: &App) -> Box<[SettingsPageItem]> { // todo(settings_ui): Refresh on extension (un)/installed // Note that `crates/json_schema_store` solves the same problem, there is probably a way to unify the two - std::iter::once(SettingsPageItem::SectionHeader(LANGUAGES_SECTION_HEADER)) + std::iter::once(SettingsPageItem::SectionHeader("Languages")) .chain(all_language_names(cx).into_iter().map(|language_name| { let link = format!("languages.{language_name}"); SettingsPageItem::SubPageLink(SubPageLink { title: language_name, + r#type: crate::SubPageType::Language, description: None, json_path: Some(link.leak()), in_json: true, files: USER | PROJECT, - render: Arc::new(|this, window, cx| { + render: |this, scroll_handle, window, cx| { let items: Box<[SettingsPageItem]> = concat_sections!( language_settings_data(), non_editor_language_settings_data(), edit_prediction_language_settings_section() ); - this.render_sub_page_items(items.iter().enumerate(), None, window, cx) - .into_any_element() - }), + this.render_sub_page_items( + items.iter().enumerate(), + scroll_handle, + window, + cx, + ) + .into_any_element() + }, }) })) .collect() @@ -7168,33 +7175,21 @@ fn network_page() -> SettingsPage { } } -const LANGUAGES_SECTION_HEADER: &'static str = "Languages"; - -fn current_language() -> Option { - sub_page_stack().iter().find_map(|page| { - (page.section_header == LANGUAGES_SECTION_HEADER).then(|| page.link.title.clone()) - }) -} - fn language_settings_field( settings_content: &SettingsContent, - get: fn(&LanguageSettingsContent) -> Option<&T>, + get_language_setting_field: fn(&LanguageSettingsContent) -> Option<&T>, ) -> Option<&T> { let all_languages = &settings_content.project.all_languages; - if let Some(current_language_name) = current_language() { - if let Some(current_language) = all_languages - .languages - .0 - .get(current_language_name.as_ref()) - { - let value = get(current_language); - if value.is_some() { - return value; - } - } - } - let default_value = get(&all_languages.defaults); - return default_value; + + active_language() + .and_then(|current_language_name| { + all_languages + .languages + .0 + .get(current_language_name.as_ref()) + }) + .and_then(get_language_setting_field) + .or_else(|| get_language_setting_field(&all_languages.defaults)) } fn language_settings_field_mut( @@ -7203,7 +7198,7 @@ fn language_settings_field_mut( write: fn(&mut LanguageSettingsContent, Option), ) { let all_languages = &mut settings_content.project.all_languages; - let language_content = if let Some(current_language) = current_language() { + let language_content = if let Some(current_language) = active_language() { all_languages .languages .0 @@ -8382,7 +8377,7 @@ fn language_settings_data() -> Box<[SettingsPageItem]> { ] } - let is_global = current_language().is_none(); + let is_global = active_language().is_none(); let lsp_document_colors_item = [SettingsPageItem::SettingItem(SettingItem { title: "LSP Document Colors", @@ -8724,17 +8719,12 @@ fn edit_prediction_language_settings_section() -> [SettingsPageItem; 4] { SettingsPageItem::SectionHeader("Edit Predictions"), SettingsPageItem::SubPageLink(SubPageLink { title: "Configure Providers".into(), + r#type: Default::default(), json_path: Some("edit_predictions.providers"), description: Some("Set up different edit prediction providers in complement to Zed's built-in Zeta model.".into()), in_json: false, files: USER, - render: Arc::new(|_, window, cx| { - let settings_window = cx.entity(); - let page = window.use_state(cx, |_, _| { - crate::pages::EditPredictionSetupPage::new(settings_window) - }); - page.into_any_element() - }), + render: render_edit_prediction_setup_page }), SettingsPageItem::SettingItem(SettingItem { title: "Show Edit Predictions", diff --git a/crates/settings_ui/src/pages.rs b/crates/settings_ui/src/pages.rs index 2b2c4818c1322216707f38bf93cefffeb14add03..cda63c6f0db5297333d2c371ffe75905d89a59fb 100644 --- a/crates/settings_ui/src/pages.rs +++ b/crates/settings_ui/src/pages.rs @@ -1,2 +1,2 @@ mod edit_prediction_provider_setup; -pub use edit_prediction_provider_setup::EditPredictionSetupPage; +pub(crate) use edit_prediction_provider_setup::render_edit_prediction_setup_page; diff --git a/crates/settings_ui/src/pages/edit_prediction_provider_setup.rs b/crates/settings_ui/src/pages/edit_prediction_provider_setup.rs index 58094df30650b87d18ed771a9819f90ce8095630..6d14c245f06df3a03197a2714ceebaf0bb0999f3 100644 --- a/crates/settings_ui/src/pages/edit_prediction_provider_setup.rs +++ b/crates/settings_ui/src/pages/edit_prediction_provider_setup.rs @@ -7,109 +7,91 @@ use feature_flags::FeatureFlagAppExt as _; use gpui::{Entity, ScrollHandle, prelude::*}; use language_models::provider::mistral::{CODESTRAL_API_URL, codestral_api_key}; use project::Project; -use ui::{ButtonLink, ConfiguredApiCard, WithScrollbar, prelude::*}; +use ui::{ButtonLink, ConfiguredApiCard, prelude::*}; use crate::{ SettingField, SettingItem, SettingsFieldMetadata, SettingsPageItem, SettingsWindow, USER, components::{SettingsInputField, SettingsSectionHeader}, }; -pub struct EditPredictionSetupPage { - settings_window: Entity, - scroll_handle: ScrollHandle, -} - -impl EditPredictionSetupPage { - pub fn new(settings_window: Entity) -> Self { - Self { - settings_window, - scroll_handle: ScrollHandle::new(), - } - } -} - -impl Render for EditPredictionSetupPage { - fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { - let settings_window = self.settings_window.clone(); - let project = settings_window - .read(cx) - .original_window - .as_ref() - .and_then(|window| { - window - .read_with(cx, |workspace, _| workspace.project().clone()) - .ok() - }); - let providers = [ - project.and_then(|project| { - Some(render_github_copilot_provider(project, window, cx)?.into_any_element()) - }), - cx.has_flag::().then(|| { - render_api_key_provider( - IconName::Inception, - "Mercury", - "https://platform.inceptionlabs.ai/dashboard/api-keys".into(), - mercury_api_token(cx), - |_cx| MERCURY_CREDENTIALS_URL, - None, - window, - cx, - ) - .into_any_element() - }), - cx.has_flag::().then(|| { - render_api_key_provider( - IconName::SweepAi, - "Sweep", - "https://app.sweep.dev/".into(), - sweep_api_token(cx), - |_cx| SWEEP_CREDENTIALS_URL, - None, - window, - cx, - ) - .into_any_element() - }), - Some( - render_api_key_provider( - IconName::AiMistral, - "Codestral", - "https://console.mistral.ai/codestral".into(), - codestral_api_key(cx), - |cx| language_models::MistralLanguageModelProvider::api_url(cx), - Some(settings_window.update(cx, |settings_window, cx| { - let codestral_settings = codestral_settings(); - settings_window - .render_sub_page_items_section( - codestral_settings.iter().enumerate(), - None, - window, - cx, - ) - .into_any_element() - })), - window, - cx, - ) - .into_any_element(), - ), - ]; - - div() - .size_full() - .vertical_scrollbar_for(&self.scroll_handle, window, cx) - .child( - v_flex() - .id("ep-setup-page") - .min_w_0() - .size_full() - .px_8() - .pb_16() - .overflow_y_scroll() - .track_scroll(&self.scroll_handle) - .children(providers.into_iter().flatten()), +pub(crate) fn render_edit_prediction_setup_page( + settings_window: &SettingsWindow, + scroll_handle: &ScrollHandle, + window: &mut Window, + cx: &mut Context, +) -> AnyElement { + let project = settings_window.original_window.as_ref().and_then(|window| { + window + .read_with(cx, |workspace, _| workspace.project().clone()) + .ok() + }); + let providers = [ + project.and_then(|project| { + render_github_copilot_provider(project, window, cx).map(IntoElement::into_any_element) + }), + cx.has_flag::().then(|| { + render_api_key_provider( + IconName::Inception, + "Mercury", + "https://platform.inceptionlabs.ai/dashboard/api-keys".into(), + mercury_api_token(cx), + |_cx| MERCURY_CREDENTIALS_URL, + None, + window, + cx, ) - } + .into_any_element() + }), + cx.has_flag::().then(|| { + render_api_key_provider( + IconName::SweepAi, + "Sweep", + "https://app.sweep.dev/".into(), + sweep_api_token(cx), + |_cx| SWEEP_CREDENTIALS_URL, + None, + window, + cx, + ) + .into_any_element() + }), + Some( + render_api_key_provider( + IconName::AiMistral, + "Codestral", + "https://console.mistral.ai/codestral".into(), + codestral_api_key(cx), + |cx| language_models::MistralLanguageModelProvider::api_url(cx), + Some( + settings_window + .render_sub_page_items_section( + codestral_settings().iter().enumerate(), + window, + cx, + ) + .into_any_element(), + ), + window, + cx, + ) + .into_any_element(), + ), + ]; + + div() + .size_full() + .child( + v_flex() + .id("ep-setup-page") + .min_w_0() + .size_full() + .px_8() + .pb_16() + .overflow_y_scroll() + .track_scroll(&scroll_handle) + .children(providers.into_iter().flatten()), + ) + .into_any_element() } fn render_api_key_provider( @@ -120,7 +102,7 @@ fn render_api_key_provider( current_url: fn(&mut App) -> SharedString, additional_fields: Option, window: &mut Window, - cx: &mut Context, + cx: &mut Context, ) -> impl IntoElement { let weak_page = cx.weak_entity(); _ = window.use_keyed_state(title, cx, |_, cx| { diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index ccbce7130cccba602b6f0a93f0cf410a7b6c2b6e..9dc15ff9671b5e3bc777734c17ebfeeb1ffc88e9 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -32,7 +32,8 @@ use theme::ThemeSettings; use title_bar::platform_title_bar::PlatformTitleBar; use ui::{ Banner, ContextMenu, Divider, DropdownMenu, DropdownStyle, IconButtonShape, KeyBinding, - KeybindingHint, PopoverMenu, Switch, Tooltip, TreeViewItem, WithScrollbar, prelude::*, + KeybindingHint, PopoverMenu, Scrollbars, Switch, Tooltip, TreeViewItem, WithScrollbar, + prelude::*, }; use ui_input::{NumberField, NumberFieldMode, NumberFieldType}; use util::{ResultExt as _, paths::PathStyle, rel_path::RelPath}; @@ -658,18 +659,18 @@ pub fn open_settings_editor( /// /// Global so that `pick` and `write` callbacks can access it /// and use it to dynamically render sub pages (e.g. for language settings) -static SUB_PAGE_STACK: LazyLock>> = LazyLock::new(|| RwLock::new(Vec::new())); +static ACTIVE_LANGUAGE: LazyLock>> = + LazyLock::new(|| RwLock::new(Option::None)); -fn sub_page_stack() -> std::sync::RwLockReadGuard<'static, Vec> { - SUB_PAGE_STACK +fn active_language() -> Option { + ACTIVE_LANGUAGE .read() - .expect("SUB_PAGE_STACK is never poisoned") + .ok() + .and_then(|language| language.clone()) } -fn sub_page_stack_mut() -> std::sync::RwLockWriteGuard<'static, Vec> { - SUB_PAGE_STACK - .write() - .expect("SUB_PAGE_STACK is never poisoned") +fn active_language_mut() -> Option>> { + ACTIVE_LANGUAGE.write().ok() } pub struct SettingsWindow { @@ -679,6 +680,7 @@ pub struct SettingsWindow { worktree_root_dirs: HashMap, current_file: SettingsUiFile, pages: Vec, + sub_page_stack: Vec, search_bar: Entity, search_task: Option>, /// Index into navbar_entries @@ -692,7 +694,6 @@ pub struct SettingsWindow { filter_table: Vec>, has_query: bool, content_handles: Vec>>, - sub_page_scroll_handle: ScrollHandle, focus_handle: FocusHandle, navbar_focus_handle: Entity, content_focus_handle: Entity, @@ -717,7 +718,37 @@ struct SearchKeyLUTEntry { struct SubPage { link: SubPageLink, - section_header: &'static str, + section_header: SharedString, + scroll_handle: ScrollHandle, +} + +impl SubPage { + fn new(link: SubPageLink, section_header: SharedString) -> Self { + if link.r#type == SubPageType::Language + && let Some(mut active_language_global) = active_language_mut() + { + active_language_global.replace(link.title.clone()); + } + + SubPage { + link, + section_header, + scroll_handle: ScrollHandle::new(), + } + } +} + +impl Drop for SubPage { + fn drop(&mut self) { + if self.link.r#type == SubPageType::Language + && let Some(mut active_language_global) = active_language_mut() + && active_language_global + .as_ref() + .is_some_and(|language_name| language_name == &self.link.title) + { + active_language_global.take(); + } + } } #[derive(Debug)] @@ -765,6 +796,13 @@ impl std::fmt::Debug for SettingsPageItem { } impl SettingsPageItem { + fn header_text(&self) -> Option<&'static str> { + match self { + SettingsPageItem::SectionHeader(header) => Some(header), + _ => None, + } + } + fn render( &self, settings_window: &SettingsWindow, @@ -901,19 +939,22 @@ impl SettingsPageItem { .on_click({ let sub_page_link = sub_page_link.clone(); cx.listener(move |this, _, window, cx| { - let mut section_index = item_index; - let current_page = this.current_page(); - - while !matches!( - current_page.items[section_index], - SettingsPageItem::SectionHeader(_) - ) { - section_index -= 1; - } - - let SettingsPageItem::SectionHeader(header) = - current_page.items[section_index] - else { + let header_text = this + .sub_page_stack + .last() + .map(|sub_page| sub_page.link.title.clone()) + .or_else(|| { + this.current_page() + .items + .iter() + .take(item_index) + .rev() + .find_map(|item| { + item.header_text().map(SharedString::new_static) + }) + }); + + let Some(header) = header_text else { unreachable!( "All items always have a section header above them" ) @@ -946,7 +987,7 @@ impl SettingsPageItem { render_setting_item_inner(discriminant_setting_item, true, false, cx); let has_sub_fields = - rendered_ok && discriminant.map(|d| !fields[d].is_empty()).unwrap_or(false); + rendered_ok && discriminant.is_some_and(|d| !fields[d].is_empty()); let mut content = v_flex() .id("dynamic-item") @@ -1109,7 +1150,7 @@ fn render_settings_item( ), ) .child(control) - .when(sub_page_stack().is_empty(), |this| { + .when(settings_window.sub_page_stack.is_empty(), |this| { this.child(render_settings_item_link( setting_item.description, setting_item.field.json_path(), @@ -1246,9 +1287,17 @@ impl PartialEq for SettingItem { } } +#[derive(Clone, PartialEq, Default)] +enum SubPageType { + Language, + #[default] + Other, +} + #[derive(Clone)] struct SubPageLink { title: SharedString, + r#type: SubPageType, description: Option, /// See [`SettingField.json_path`] json_path: Option<&'static str>, @@ -1256,12 +1305,8 @@ struct SubPageLink { /// Removes the "Edit in settings.json" button from the page. in_json: bool, files: FileMask, - render: Arc< - dyn Fn(&mut SettingsWindow, &mut Window, &mut Context) -> AnyElement - + 'static - + Send - + Sync, - >, + render: + fn(&SettingsWindow, &ScrollHandle, &mut Window, &mut Context) -> AnyElement, } impl PartialEq for SubPageLink { @@ -1523,6 +1568,7 @@ impl SettingsWindow { current_file: current_file, pages: vec![], + sub_page_stack: vec![], navbar_entries: vec![], navbar_entry: 0, navbar_scroll_handle: UniformListScrollHandle::default(), @@ -1531,7 +1577,6 @@ impl SettingsWindow { filter_table: vec![], has_query: false, content_handles: vec![], - sub_page_scroll_handle: ScrollHandle::new(), focus_handle: cx.focus_handle(), navbar_focus_handle: NonFocusableHandle::new( NAVBAR_CONTAINER_TAB_INDEX, @@ -2002,7 +2047,7 @@ impl SettingsWindow { self.setup_navbar_focus_subscriptions(window, cx); self.build_content_handles(window, cx); } - sub_page_stack_mut().clear(); + self.sub_page_stack.clear(); // PERF: doesn't have to be rebuilt, can just be filled with true. pages is constant once it is built self.build_filter_table(); self.reset_list_state(); @@ -2118,7 +2163,7 @@ impl SettingsWindow { self.reset_list_state(); } - sub_page_stack_mut().clear(); + self.sub_page_stack.clear(); } fn open_first_nav_page(&mut self) { @@ -2613,8 +2658,10 @@ impl SettingsWindow { if self.navbar_entries[navbar_entry_index].is_root || !self.is_nav_entry_visible(navbar_entry_index) { - self.sub_page_scroll_handle - .set_offset(point(px(0.), px(0.))); + if let Some(scroll_handle) = self.current_sub_page_scroll_handle() { + scroll_handle.set_offset(point(px(0.), px(0.))); + } + if focus_content { let Some(first_item_index) = self.visible_page_items().next().map(|(index, _)| index) @@ -2685,8 +2732,10 @@ impl SettingsWindow { .position(|(index, _)| index == content_item_index) .unwrap_or(0); if index == 0 { - self.sub_page_scroll_handle - .set_offset(point(px(0.), px(0.))); + if let Some(scroll_handle) = self.current_sub_page_scroll_handle() { + scroll_handle.set_offset(point(px(0.), px(0.))); + } + self.list_state.scroll_to(gpui::ListOffset { item_ix: 0, offset_in_item: px(0.), @@ -2734,6 +2783,10 @@ impl SettingsWindow { cx.notify(); } + fn current_sub_page_scroll_handle(&self) -> Option<&ScrollHandle> { + self.sub_page_stack.last().map(|page| &page.scroll_handle) + } + fn visible_page_items(&self) -> impl Iterator { let page_idx = self.current_page_index(); @@ -2741,33 +2794,32 @@ impl SettingsWindow { .items .iter() .enumerate() - .filter_map(move |(item_index, item)| { - self.filter_table[page_idx][item_index].then_some((item_index, item)) - }) + .filter(move |&(item_index, _)| self.filter_table[page_idx][item_index]) } fn render_sub_page_breadcrumbs(&self) -> impl IntoElement { - let mut items = vec![]; - items.push(self.current_page().title.into()); - items.extend( - sub_page_stack() - .iter() - .flat_map(|page| [page.section_header.into(), page.link.title.clone()]), - ); - - let last = items.pop().unwrap(); - h_flex() - .gap_1() - .children( - items - .into_iter() - .flat_map(|item| [item, "/".into()]) - .map(|item| Label::new(item).color(Color::Muted)), + h_flex().gap_1().children( + itertools::intersperse( + std::iter::once(self.current_page().title.into()).chain( + self.sub_page_stack + .iter() + .enumerate() + .flat_map(|(index, page)| { + (index == 0) + .then(|| page.section_header.clone()) + .into_iter() + .chain(std::iter::once(page.link.title.clone())) + }), + ), + "/".into(), ) - .child(Label::new(last)) + .map(|item| Label::new(item).color(Color::Muted)), + ) } - fn render_empty_state(&self, search_query: SharedString) -> impl IntoElement { + fn render_no_results(&self, cx: &App) -> impl IntoElement { + let search_query = self.search_bar.read(cx).text(cx); + v_flex() .size_full() .items_center() @@ -2775,28 +2827,25 @@ impl SettingsWindow { .gap_1() .child(Label::new("No Results")) .child( - Label::new(search_query) + Label::new(format!("No settings match \"{}\"", search_query)) .size(LabelSize::Small) .color(Color::Muted), ) } - fn render_page_items( + fn render_current_page_items( &mut self, - page_index: usize, _window: &mut Window, cx: &mut Context, ) -> impl IntoElement { + let current_page_index = self.current_page_index(); let mut page_content = v_flex().id("settings-ui-page").size_full(); let has_active_search = !self.search_bar.read(cx).is_empty(cx); let has_no_results = self.visible_page_items().next().is_none() && has_active_search; if has_no_results { - let search_query = self.search_bar.read(cx).text(cx); - page_content = page_content.child( - self.render_empty_state(format!("No settings match \"{}\"", search_query).into()), - ) + page_content = page_content.child(self.render_no_results(cx)) } else { let last_non_header_index = self .visible_page_items() @@ -2817,7 +2866,7 @@ impl SettingsWindow { if index == 0 { return div() .px_8() - .when(sub_page_stack().is_empty(), |this| { + .when(this.sub_page_stack.is_empty(), |this| { this.when_some(root_nav_label, |this, title| { this.child( Label::new(title).size(LabelSize::Large).mt_2().mb_3(), @@ -2839,8 +2888,9 @@ impl SettingsWindow { let is_last = Some(actual_item_index) == last_non_header_index; - let item_focus_handle = - this.content_handles[page_index][actual_item_index].focus_handle(cx); + let item_focus_handle = this.content_handles[current_page_index] + [actual_item_index] + .focus_handle(cx); v_flex() .id(("settings-page-item", actual_item_index)) @@ -2866,7 +2916,7 @@ impl SettingsWindow { fn render_sub_page_items<'a, Items>( &self, items: Items, - page_index: Option, + scroll_handle: &ScrollHandle, window: &mut Window, cx: &mut Context, ) -> impl IntoElement @@ -2877,14 +2927,13 @@ impl SettingsWindow { .id("settings-ui-page") .size_full() .overflow_y_scroll() - .track_scroll(&self.sub_page_scroll_handle); - self.render_sub_page_items_in(page_content, items, page_index, window, cx) + .track_scroll(scroll_handle); + self.render_sub_page_items_in(page_content, items, window, cx) } fn render_sub_page_items_section<'a, Items>( &self, items: Items, - page_index: Option, window: &mut Window, cx: &mut Context, ) -> impl IntoElement @@ -2892,14 +2941,13 @@ impl SettingsWindow { Items: Iterator, { let page_content = v_flex().id("settings-ui-sub-page-section").size_full(); - self.render_sub_page_items_in(page_content, items, page_index, window, cx) + self.render_sub_page_items_in(page_content, items, window, cx) } fn render_sub_page_items_in<'a, Items>( &self, - mut page_content: Stateful
, + page_content: Stateful
, items: Items, - page_index: Option, window: &mut Window, cx: &mut Context, ) -> impl IntoElement @@ -2908,16 +2956,12 @@ impl SettingsWindow { { let items: Vec<_> = items.collect(); let items_len = items.len(); - let mut section_header = None; let has_active_search = !self.search_bar.read(cx).is_empty(cx); let has_no_results = items_len == 0 && has_active_search; if has_no_results { - let search_query = self.search_bar.read(cx).text(cx); - page_content = page_content.child( - self.render_empty_state(format!("No settings match \"{}\"", search_query).into()), - ) + page_content.child(self.render_no_results(cx)) } else { let last_non_header_index = items .iter() @@ -2932,35 +2976,25 @@ impl SettingsWindow { .find(|entry| entry.is_root && entry.page_index == self.current_page_index()) .map(|entry| entry.title); - page_content = page_content - .when(sub_page_stack().is_empty(), |this| { + page_content + .when(self.sub_page_stack.is_empty(), |this| { this.when_some(root_nav_label, |this, title| { this.child(Label::new(title).size(LabelSize::Large).mt_2().mb_3()) }) }) .children(items.clone().into_iter().enumerate().map( |(index, (actual_item_index, item))| { - let no_bottom_border = items - .get(index + 1) - .map(|(_, next_item)| { + let no_bottom_border = + items.get(index + 1).is_some_and(|(_, next_item)| { matches!(next_item, SettingsPageItem::SectionHeader(_)) - }) - .unwrap_or(false); + }); + let is_last = Some(index) == last_non_header_index; - if let SettingsPageItem::SectionHeader(header) = item { - section_header = Some(*header); - } v_flex() .w_full() .min_w_0() .id(("settings-page-item", actual_item_index)) - .when_some(page_index, |element, page_index| { - element.track_focus( - &self.content_handles[page_index][actual_item_index] - .focus_handle(cx), - ) - }) .child(item.render( self, actual_item_index, @@ -2971,7 +3005,6 @@ impl SettingsWindow { }, )) } - page_content } fn render_page( @@ -2982,13 +3015,7 @@ impl SettingsWindow { let page_header; let page_content; - if sub_page_stack().is_empty() { - page_header = self.render_files_header(window, cx).into_any_element(); - - page_content = self - .render_page_items(self.current_page_index(), window, cx) - .into_any_element(); - } else { + if let Some(current_sub_page) = self.sub_page_stack.last() { page_header = h_flex() .w_full() .justify_between() @@ -3006,32 +3033,36 @@ impl SettingsWindow { ) .child(self.render_sub_page_breadcrumbs()), ) - .when( - sub_page_stack() - .last() - .is_none_or(|sub_page| sub_page.link.in_json), - |this| { - this.child( - Button::new("open-in-settings-file", "Edit in settings.json") - .tab_index(0_isize) - .style(ButtonStyle::OutlinedGhost) - .tooltip(Tooltip::for_action_title_in( - "Edit in settings.json", - &OpenCurrentFile, - &self.focus_handle, - )) - .on_click(cx.listener(|this, _, window, cx| { - this.open_current_settings_file(window, cx); - })), - ) - }, - ) + .when(current_sub_page.link.in_json, |this| { + this.child( + Button::new("open-in-settings-file", "Edit in settings.json") + .tab_index(0_isize) + .style(ButtonStyle::OutlinedGhost) + .tooltip(Tooltip::for_action_title_in( + "Edit in settings.json", + &OpenCurrentFile, + &self.focus_handle, + )) + .on_click(cx.listener(|this, _, window, cx| { + this.open_current_settings_file(window, cx); + })), + ) + }) .into_any_element(); - let active_page_render_fn = sub_page_stack().last().unwrap().link.render.clone(); - page_content = (active_page_render_fn)(self, window, cx); + let active_page_render_fn = ¤t_sub_page.link.render; + page_content = + (active_page_render_fn)(self, ¤t_sub_page.scroll_handle, window, cx); + } else { + page_header = self.render_files_header(window, cx).into_any_element(); + + page_content = self + .render_current_page_items(window, cx) + .into_any_element(); } + let current_sub_page = self.sub_page_stack.last(); + let mut warning_banner = gpui::Empty.into_any_element(); if let Some(error) = SettingsStore::global(cx).error_for_file(self.current_file.to_settings()) @@ -3101,10 +3132,10 @@ impl SettingsWindow { .into_any_element() } - return v_flex() + v_flex() .id("settings-ui-page") .on_action(cx.listener(|this, _: &menu::SelectNext, window, cx| { - if !sub_page_stack().is_empty() { + if !this.sub_page_stack.is_empty() { window.focus_next(cx); return; } @@ -3136,7 +3167,7 @@ impl SettingsWindow { window.focus_next(cx); })) .on_action(cx.listener(|this, _: &menu::SelectPrevious, window, cx| { - if !sub_page_stack().is_empty() { + if !this.sub_page_stack.is_empty() { window.focus_prev(cx); return; } @@ -3168,11 +3199,17 @@ impl SettingsWindow { } window.focus_prev(cx); })) - .when(sub_page_stack().is_empty(), |this| { + .when(current_sub_page.is_none(), |this| { this.vertical_scrollbar_for(&self.list_state, window, cx) }) - .when(!sub_page_stack().is_empty(), |this| { - this.vertical_scrollbar_for(&self.sub_page_scroll_handle, window, cx) + .when_some(current_sub_page, |this, current_sub_page| { + this.custom_scrollbars( + Scrollbars::new(ui::ScrollAxes::Vertical) + .tracked_scroll_handle(¤t_sub_page.scroll_handle) + .id((current_sub_page.link.title.clone(), 42)), + window, + cx, + ) }) .track_focus(&self.content_focus_handle.focus_handle(cx)) .pt_6() @@ -3193,7 +3230,7 @@ impl SettingsWindow { .tab_group() .tab_index(CONTENT_GROUP_TAB_INDEX) .child(page_content), - ); + ) } /// This function will create a new settings file if one doesn't exist @@ -3354,19 +3391,15 @@ impl SettingsWindow { } fn current_page_index(&self) -> usize { - self.page_index_from_navbar_index(self.navbar_entry) - } - - fn current_page(&self) -> &SettingsPage { - &self.pages[self.current_page_index()] - } - - fn page_index_from_navbar_index(&self, index: usize) -> usize { if self.navbar_entries.is_empty() { return 0; } - self.navbar_entries[index].page_index + self.navbar_entries[self.navbar_entry].page_index + } + + fn current_page(&self) -> &SettingsPage { + &self.pages[self.current_page_index()] } fn is_navbar_entry_selected(&self, ix: usize) -> bool { @@ -3376,22 +3409,18 @@ impl SettingsWindow { fn push_sub_page( &mut self, sub_page_link: SubPageLink, - section_header: &'static str, + section_header: SharedString, window: &mut Window, cx: &mut Context, ) { - sub_page_stack_mut().push(SubPage { - link: sub_page_link, - section_header, - }); - self.sub_page_scroll_handle - .set_offset(point(px(0.), px(0.))); + self.sub_page_stack + .push(SubPage::new(sub_page_link, section_header)); self.content_focus_handle.focus_handle(cx).focus(window, cx); cx.notify(); } fn pop_sub_page(&mut self, window: &mut Window, cx: &mut Context) { - sub_page_stack_mut().pop(); + self.sub_page_stack.pop(); self.content_focus_handle.focus_handle(cx).focus(window, cx); cx.notify(); } @@ -4047,10 +4076,10 @@ pub mod test { navbar_scroll_handle: UniformListScrollHandle::default(), navbar_focus_subscriptions: vec![], filter_table: vec![], + sub_page_stack: vec![], has_query: false, content_handles: vec![], search_task: None, - sub_page_scroll_handle: ScrollHandle::new(), focus_handle: cx.focus_handle(), navbar_focus_handle: NonFocusableHandle::new( NAVBAR_CONTAINER_TAB_INDEX,