From 54cec5b484c3e71b5fe47331e1e21ad0cf801425 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Mon, 1 Sep 2025 19:26:42 -0500 Subject: [PATCH] settings_ui: Get editor settings working (#37330) Closes #ISSUE This PR includes the necessary work to get `EditorSettings` showing up in the settings UI. Including making the `path` field on `SettingsUiItem`'s optional so that top level items such as `EditorSettings` which have `Settings::KEY = None` (i.e. are treated like `serde(flatten)`) have their paths computed correctly for JSON reading/updating. It includes the first examples of a pattern I expect to continue with the `SettingsUi` work with respect to settings reorganization, that being adding missing defaults, and adding explicit values (or aliases) to settings which previously relied on `null` being a value for optional fields. Release Notes: - N/A *or* Added/Fixed/Improved ... --- assets/settings/default.json | 8 +- crates/editor/src/editor_settings.rs | 59 +++++--- crates/language/src/buffer.rs | 6 +- crates/project/src/project_settings.rs | 13 +- crates/settings/src/settings.rs | 4 +- crates/settings/src/settings_store.rs | 2 +- .../{settings_ui.rs => settings_ui_core.rs} | 109 +++++++++----- crates/settings_ui/src/settings_ui.rs | 137 ++++++++++++------ .../src/settings_ui_macros.rs | 60 +++----- 9 files changed, 246 insertions(+), 152 deletions(-) rename crates/settings/src/{settings_ui.rs => settings_ui_core.rs} (51%) diff --git a/assets/settings/default.json b/assets/settings/default.json index 623a4612d06975ca4681d75a775d061e594608b2..0b5481bd4e4e2177302e38199bb66e87471d2904 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -188,8 +188,8 @@ // 4. A box drawn around the following character // "hollow" // - // Default: not set, defaults to "bar" - "cursor_shape": null, + // Default: "bar" + "cursor_shape": "bar", // Determines when the mouse cursor should be hidden in an editor or input box. // // 1. Never hide the mouse cursor: @@ -282,8 +282,8 @@ // - "warning" // - "info" // - "hint" - // - null — allow all diagnostics (default) - "diagnostics_max_severity": null, + // - "all" — allow all diagnostics (default) + "diagnostics_max_severity": "all", // Whether to show wrap guides (vertical rulers) in the editor. // Setting this to true will show a guide at the 'preferred_line_length' value // if 'soft_wrap' is set to 'preferred_line_length', and will show any diff --git a/crates/editor/src/editor_settings.rs b/crates/editor/src/editor_settings.rs index 3e4e86f023530b89fa3e325474ecea801ff06ecb..44cb0749760e9e3af91bc837df0ef0589e251703 100644 --- a/crates/editor/src/editor_settings.rs +++ b/crates/editor/src/editor_settings.rs @@ -61,7 +61,9 @@ pub struct EditorSettings { } /// How to render LSP `textDocument/documentColor` colors in the editor. -#[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive( + Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, SettingsUi, +)] #[serde(rename_all = "snake_case")] pub enum DocumentColorsRenderMode { /// Do not query and render document colors. @@ -75,7 +77,7 @@ pub enum DocumentColorsRenderMode { Background, } -#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema, SettingsUi)] #[serde(rename_all = "snake_case")] pub enum CurrentLineHighlight { // Don't highlight the current line. @@ -89,7 +91,7 @@ pub enum CurrentLineHighlight { } /// When to populate a new search's query based on the text under the cursor. -#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema, SettingsUi)] #[serde(rename_all = "snake_case")] pub enum SeedQuerySetting { /// Always populate the search query with the word under the cursor. @@ -101,7 +103,9 @@ pub enum SeedQuerySetting { } /// What to do when multibuffer is double clicked in some of its excerpts (parts of singleton buffers). -#[derive(Default, Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive( + Default, Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema, SettingsUi, +)] #[serde(rename_all = "snake_case")] pub enum DoubleClickInMultibuffer { /// Behave as a regular buffer and select the whole word. @@ -120,7 +124,9 @@ pub struct Jupyter { pub enabled: bool, } -#[derive(Default, Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive( + Default, Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema, SettingsUi, +)] #[serde(rename_all = "snake_case")] pub struct JupyterContent { /// Whether the Jupyter feature is enabled. @@ -292,7 +298,9 @@ pub struct ScrollbarAxes { } /// Whether to allow drag and drop text selection in buffer. -#[derive(Copy, Clone, Default, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +#[derive( + Copy, Clone, Default, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq, SettingsUi, +)] pub struct DragAndDropSelection { /// When true, enables drag and drop text selection in buffer. /// @@ -332,7 +340,7 @@ pub enum ScrollbarDiagnostics { /// The key to use for adding multiple cursors /// /// Default: alt -#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq, SettingsUi)] #[serde(rename_all = "snake_case")] pub enum MultiCursorModifier { Alt, @@ -343,7 +351,7 @@ pub enum MultiCursorModifier { /// Whether the editor will scroll beyond the last line. /// /// Default: one_page -#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq, SettingsUi)] #[serde(rename_all = "snake_case")] pub enum ScrollBeyondLastLine { /// The editor will not scroll beyond the last line. @@ -357,7 +365,9 @@ pub enum ScrollBeyondLastLine { } /// Default options for buffer and project search items. -#[derive(Copy, Clone, Default, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +#[derive( + Copy, Clone, Default, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq, SettingsUi, +)] pub struct SearchSettings { /// Whether to show the project search button in the status bar. #[serde(default = "default_true")] @@ -373,7 +383,9 @@ pub struct SearchSettings { } /// What to do when go to definition yields no results. -#[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive( + Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, SettingsUi, +)] #[serde(rename_all = "snake_case")] pub enum GoToDefinitionFallback { /// Disables the fallback. @@ -386,7 +398,9 @@ pub enum GoToDefinitionFallback { /// Determines when the mouse cursor should be hidden in an editor or input box. /// /// Default: on_typing_and_movement -#[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive( + Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, SettingsUi, +)] #[serde(rename_all = "snake_case")] pub enum HideMouseMode { /// Never hide the mouse cursor @@ -401,7 +415,9 @@ pub enum HideMouseMode { /// Determines how snippets are sorted relative to other completion items. /// /// Default: inline -#[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive( + Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, SettingsUi, +)] #[serde(rename_all = "snake_case")] pub enum SnippetSortOrder { /// Place snippets at the top of the completion list @@ -416,6 +432,7 @@ pub enum SnippetSortOrder { } #[derive(Clone, Default, Serialize, Deserialize, JsonSchema, SettingsUi)] +#[settings_ui(group = "Editor")] pub struct EditorSettingsContent { /// Whether the cursor blinks in the editor. /// @@ -424,7 +441,7 @@ pub struct EditorSettingsContent { /// Cursor shape for the default editor. /// Can be "bar", "block", "underline", or "hollow". /// - /// Default: None + /// Default: bar pub cursor_shape: Option, /// Determines when the mouse cursor should be hidden in an editor or input box. /// @@ -601,7 +618,7 @@ pub struct EditorSettingsContent { } // Status bar related settings -#[derive(Clone, Default, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +#[derive(Clone, Default, Serialize, Deserialize, JsonSchema, PartialEq, Eq, SettingsUi)] pub struct StatusBarContent { /// Whether to display the active language button in the status bar. /// @@ -614,7 +631,7 @@ pub struct StatusBarContent { } // Toolbar related settings -#[derive(Clone, Default, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +#[derive(Clone, Default, Serialize, Deserialize, JsonSchema, PartialEq, Eq, SettingsUi)] pub struct ToolbarContent { /// Whether to display breadcrumbs in the editor toolbar. /// @@ -640,7 +657,9 @@ pub struct ToolbarContent { } /// Scrollbar related settings -#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Default)] +#[derive( + Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Default, SettingsUi, +)] pub struct ScrollbarContent { /// When to show the scrollbar in the editor. /// @@ -675,7 +694,9 @@ pub struct ScrollbarContent { } /// Minimap related settings -#[derive(Copy, Clone, Default, Debug, Serialize, Deserialize, JsonSchema, PartialEq)] +#[derive( + Copy, Clone, Default, Debug, Serialize, Deserialize, JsonSchema, PartialEq, SettingsUi, +)] pub struct MinimapContent { /// When to show the minimap in the editor. /// @@ -723,7 +744,9 @@ pub struct ScrollbarAxesContent { } /// Gutter related settings -#[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, JsonSchema, PartialEq, Eq)] +#[derive( + Copy, Clone, Debug, Default, Serialize, Deserialize, JsonSchema, PartialEq, Eq, SettingsUi, +)] pub struct GutterContent { /// Whether to show line numbers in the gutter. /// diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 1a1d9fb4a7dc3a3d2a847cee3661361343a6871e..c978f6c4ef9f60e092d67a655adc6d95693788a8 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -32,7 +32,7 @@ use parking_lot::Mutex; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::Value; -use settings::WorktreeId; +use settings::{SettingsUi, WorktreeId}; use smallvec::SmallVec; use smol::future::yield_now; use std::{ @@ -173,7 +173,9 @@ pub enum IndentKind { } /// The shape of a selection cursor. -#[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[derive( + Copy, Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, SettingsUi, +)] #[serde(rename_all = "snake_case")] pub enum CursorShape { /// A vertical bar diff --git a/crates/project/src/project_settings.rs b/crates/project/src/project_settings.rs index 30a71c4caeb676509239151a4766beb590fdb47e..4a97130f15d582df392c25d6d64482bc4ca17834 100644 --- a/crates/project/src/project_settings.rs +++ b/crates/project/src/project_settings.rs @@ -281,7 +281,17 @@ impl Default for GlobalLspSettings { } #[derive( - Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize, JsonSchema, + Clone, + Copy, + Debug, + Eq, + PartialEq, + Ord, + PartialOrd, + Serialize, + Deserialize, + JsonSchema, + SettingsUi, )] #[serde(rename_all = "snake_case")] pub enum DiagnosticSeverity { @@ -290,6 +300,7 @@ pub enum DiagnosticSeverity { Error, Warning, Info, + #[serde(alias = "all")] Hint, } diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 983cd31dd31d6b9c2cd017568fffe0812f9ae4e5..7e567cc085101713b0f6b100d0b47f6bf4c3531f 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -4,7 +4,7 @@ mod keymap_file; mod settings_file; mod settings_json; mod settings_store; -mod settings_ui; +mod settings_ui_core; mod vscode_import; use gpui::{App, Global}; @@ -24,7 +24,7 @@ pub use settings_store::{ InvalidSettingsError, LocalSettingsKind, Settings, SettingsLocation, SettingsSources, SettingsStore, }; -pub use settings_ui::*; +pub use settings_ui_core::*; // Re-export the derive macro pub use settings_ui_macros::SettingsUi; pub use vscode_import::{VsCodeSettings, VsCodeSettingsSource}; diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index c1a7fd9e3c4812b78627ae6899ce068d499cbdf7..60eb132ad8b4f6419f463f32b1874ea97be07ec1 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -33,7 +33,7 @@ pub type EditorconfigProperties = ec4rs::Properties; use crate::{ ActiveSettingsProfileName, ParameterizedJsonSchema, SettingsJsonSchemaParams, SettingsUiEntry, VsCodeSettings, WorktreeId, parse_json_with_comments, replace_value_in_json_text, - settings_ui::SettingsUi, update_value_in_json_text, + settings_ui_core::SettingsUi, update_value_in_json_text, }; /// A value that can be defined as a user setting. diff --git a/crates/settings/src/settings_ui.rs b/crates/settings/src/settings_ui_core.rs similarity index 51% rename from crates/settings/src/settings_ui.rs rename to crates/settings/src/settings_ui_core.rs index 3a77627d5976f20c5732b47a1dfc164bc0a74b58..3a5fa3016b78ff90bdf9c6166255809f4f89b1d2 100644 --- a/crates/settings/src/settings_ui.rs +++ b/crates/settings/src/settings_ui_core.rs @@ -1,3 +1,5 @@ +use std::any::TypeId; + use anyhow::Context as _; use fs::Fs; use gpui::{AnyElement, App, AppContext as _, ReadGlobal as _, Window}; @@ -14,40 +16,26 @@ pub trait SettingsUi { fn settings_ui_entry() -> SettingsUiEntry { SettingsUiEntry { - item: SettingsUiEntryVariant::None, + path: None, + title: "None entry", + item: SettingsUiItem::None, } } } pub struct SettingsUiEntry { // todo(settings_ui): move this back here once there isn't a None variant - // pub path: &'static str, - // pub title: &'static str, - pub item: SettingsUiEntryVariant, -} - -pub enum SettingsUiEntryVariant { - Group { - path: &'static str, - title: &'static str, - items: Vec, - }, - Item { - path: &'static str, - item: SettingsUiItemSingle, - }, - Dynamic { - path: &'static str, - options: Vec, - determine_option: fn(&serde_json::Value, &mut App) -> usize, - }, - // todo(settings_ui): remove - None, + /// The path in the settings JSON file for this setting. Relative to parent + /// None implies `#[serde(flatten)]` or `Settings::KEY.is_none()` for top level settings + pub path: Option<&'static str>, + pub title: &'static str, + pub item: SettingsUiItem, } pub enum SettingsUiItemSingle { SwitchField, - NumericStepper, + /// A numeric stepper for a specific type of number + NumericStepper(NumType), ToggleGroup(&'static [&'static str]), /// This should be used when toggle group size > 6 DropDown(&'static [&'static str]), @@ -96,16 +84,19 @@ impl SettingsValue { } } +pub struct SettingsUiItemDynamic { + pub options: Vec, + pub determine_option: fn(&serde_json::Value, &mut App) -> usize, +} + +pub struct SettingsUiItemGroup { + pub items: Vec, +} + pub enum SettingsUiItem { - Group { - title: &'static str, - items: Vec, - }, + Group(SettingsUiItemGroup), Single(SettingsUiItemSingle), - Dynamic { - options: Vec, - determine_option: fn(&serde_json::Value, &mut App) -> usize, - }, + Dynamic(SettingsUiItemDynamic), None, } @@ -121,8 +112,56 @@ impl SettingsUi for Option { } } -impl SettingsUi for u64 { - fn settings_ui_item() -> SettingsUiItem { - SettingsUiItem::Single(SettingsUiItemSingle::NumericStepper) +#[repr(u8)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum NumType { + U64 = 0, + U32 = 1, + F32 = 2, +} +pub static NUM_TYPE_NAMES: std::sync::LazyLock<[&'static str; NumType::COUNT]> = + std::sync::LazyLock::new(|| NumType::ALL.map(NumType::type_name)); +pub static NUM_TYPE_IDS: std::sync::LazyLock<[TypeId; NumType::COUNT]> = + std::sync::LazyLock::new(|| NumType::ALL.map(NumType::type_id)); + +impl NumType { + const COUNT: usize = 3; + const ALL: [NumType; Self::COUNT] = [NumType::U64, NumType::U32, NumType::F32]; + + pub fn type_id(self) -> TypeId { + match self { + NumType::U64 => TypeId::of::(), + NumType::U32 => TypeId::of::(), + NumType::F32 => TypeId::of::(), + } + } + + pub fn type_name(self) -> &'static str { + match self { + NumType::U64 => std::any::type_name::(), + NumType::U32 => std::any::type_name::(), + NumType::F32 => std::any::type_name::(), + } } } + +macro_rules! numeric_stepper_for_num_type { + ($type:ty, $num_type:ident) => { + impl SettingsUi for $type { + fn settings_ui_item() -> SettingsUiItem { + SettingsUiItem::Single(SettingsUiItemSingle::NumericStepper(NumType::$num_type)) + } + } + + impl SettingsUi for Option<$type> { + fn settings_ui_item() -> SettingsUiItem { + SettingsUiItem::Single(SettingsUiItemSingle::NumericStepper(NumType::$num_type)) + } + } + }; +} + +numeric_stepper_for_num_type!(u64, U64); +numeric_stepper_for_num_type!(u32, U32); +// todo(settings_ui) is there a better ui for f32? +numeric_stepper_for_num_type!(f32, F32); diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 37edfd5679d259b1d81159f02472fc682bf17243..01f539d2422b01d4eeae7b855e1bfebf2296d383 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -9,7 +9,10 @@ use command_palette_hooks::CommandPaletteFilter; use editor::EditorSettingsControls; use feature_flags::{FeatureFlag, FeatureFlagViewExt}; use gpui::{App, Entity, EventEmitter, FocusHandle, Focusable, ReadGlobal, actions}; -use settings::{SettingsStore, SettingsUiEntryVariant, SettingsUiItemSingle, SettingsValue}; +use settings::{ + NumType, SettingsStore, SettingsUiEntry, SettingsUiItem, SettingsUiItemDynamic, + SettingsUiItemGroup, SettingsUiItemSingle, SettingsValue, +}; use smallvec::SmallVec; use ui::{NumericStepper, SwitchField, ToggleButtonGroup, ToggleButtonSimple, prelude::*}; use workspace::{ @@ -134,7 +137,7 @@ impl Item for SettingsPage { struct UiEntry { title: &'static str, - path: &'static str, + path: Option<&'static str>, _depth: usize, // a // b < a descendant range < a total descendant range @@ -182,14 +185,14 @@ struct SettingsUiTree { fn build_tree_item( tree: &mut Vec, - entry: SettingsUiEntryVariant, + entry: SettingsUiEntry, depth: usize, prev_index: Option, ) { let index = tree.len(); tree.push(UiEntry { - title: "", - path: "", + title: entry.title, + path: entry.path, _depth: depth, descendant_range: index + 1..index + 1, total_descendant_range: index + 1..index + 1, @@ -200,14 +203,8 @@ fn build_tree_item( if let Some(prev_index) = prev_index { tree[prev_index].next_sibling = Some(index); } - match entry { - SettingsUiEntryVariant::Group { - path, - title, - items: group_items, - } => { - tree[index].path = path; - tree[index].title = title; + match entry.item { + SettingsUiItem::Group(SettingsUiItemGroup { items: group_items }) => { for group_item in group_items { let prev_index = tree[index] .descendant_range @@ -215,22 +212,17 @@ fn build_tree_item( .not() .then_some(tree[index].descendant_range.end - 1); tree[index].descendant_range.end = tree.len() + 1; - build_tree_item(tree, group_item.item, depth + 1, prev_index); + build_tree_item(tree, group_item, depth + 1, prev_index); tree[index].total_descendant_range.end = tree.len(); } } - SettingsUiEntryVariant::Item { path, item } => { - tree[index].path = path; - // todo(settings_ui) create title from path in macro, and use here - tree[index].title = path; + SettingsUiItem::Single(item) => { tree[index].render = Some(item); } - SettingsUiEntryVariant::Dynamic { - path, + SettingsUiItem::Dynamic(SettingsUiItemDynamic { options, determine_option, - } => { - tree[index].path = path; + }) => { tree[index].select_descendant = Some(determine_option); for option in options { let prev_index = tree[index] @@ -239,11 +231,11 @@ fn build_tree_item( .not() .then_some(tree[index].descendant_range.end - 1); tree[index].descendant_range.end = tree.len() + 1; - build_tree_item(tree, option.item, depth + 1, prev_index); + build_tree_item(tree, option, depth + 1, prev_index); tree[index].total_descendant_range.end = tree.len(); } } - SettingsUiEntryVariant::None => { + SettingsUiItem::None => { return; } } @@ -255,21 +247,17 @@ impl SettingsUiTree { let mut tree = vec![]; let mut root_entry_indices = vec![]; for item in settings_store.settings_ui_items() { - if matches!(item.item, SettingsUiEntryVariant::None) { + if matches!(item.item, SettingsUiItem::None) + // todo(settings_ui): How to handle top level single items? BaseKeymap is in this category. Probably need a way to + // link them to other groups + || matches!(item.item, SettingsUiItem::Single(_)) + { continue; } - assert!( - matches!(item.item, SettingsUiEntryVariant::Group { .. }), - "top level items must be groups: {:?}", - match item.item { - SettingsUiEntryVariant::Item { path, .. } => path, - _ => unreachable!(), - } - ); let prev_root_entry_index = root_entry_indices.last().copied(); root_entry_indices.push(tree.len()); - build_tree_item(&mut tree, item.item, 0, prev_root_entry_index); + build_tree_item(&mut tree, item, 0, prev_root_entry_index); } root_entry_indices.sort_by_key(|i| tree[*i].title); @@ -314,9 +302,12 @@ fn render_content( .size_full() .child(Label::new(SharedString::new_static("No settings found")).color(Color::Error)); }; - let mut content = v_flex().size_full().gap_4(); + let mut content = v_flex().size_full().gap_4().overflow_hidden(); - let mut path = smallvec::smallvec![active_entry.path]; + let mut path = smallvec::smallvec![]; + if let Some(active_entry_path) = active_entry.path { + path.push(active_entry_path); + } let mut entry_index_queue = VecDeque::new(); if let Some(child_index) = active_entry.first_descendant_index() { @@ -331,7 +322,11 @@ fn render_content( while let Some(index) = entry_index_queue.pop_front() { // todo(settings_ui): subgroups? let child = &tree.entries[index]; - path.push(child.path); + let mut pushed_path = false; + if let Some(child_path) = child.path { + path.push(child_path); + pushed_path = true; + } let settings_value = settings_value_from_settings_and_path( path.clone(), // PERF: how to structure this better? There feels like there's a way to avoid the clone @@ -347,7 +342,9 @@ fn render_content( entry_index_queue.push_front(descendant_index); } } - path.pop(); + if pushed_path { + path.pop(); + } let Some(child_render) = child.render.as_ref() else { continue; }; @@ -433,8 +430,8 @@ fn render_item_single( SettingsUiItemSingle::SwitchField => { render_any_item(settings_value, render_switch_field, window, cx) } - SettingsUiItemSingle::NumericStepper => { - render_any_item(settings_value, render_numeric_stepper, window, cx) + SettingsUiItemSingle::NumericStepper(num_type) => { + render_any_numeric_stepper(settings_value, *num_type, window, cx) } SettingsUiItemSingle::ToggleGroup(variants) => { render_toggle_button_group(settings_value, variants, window, cx) @@ -468,6 +465,7 @@ fn downcast_any_item( .map(|value| serde_json::from_value::(value).expect("value is not a T")); // todo(settings_ui) Create test that constructs UI tree, and asserts that all elements have default values let default_value = serde_json::from_value::(settings_value.default_value) + .with_context(|| format!("path: {:?}", settings_value.path.join("."))) .expect("default value is not an Option"); let deserialized_setting_value = SettingsValue { title: settings_value.title, @@ -488,14 +486,62 @@ fn render_any_item( render_fn(deserialized_setting_value, window, cx) } -fn render_numeric_stepper( - value: SettingsValue, +fn render_any_numeric_stepper( + settings_value: SettingsValue, + num_type: NumType, + window: &mut Window, + cx: &mut App, +) -> AnyElement { + match num_type { + NumType::U64 => render_numeric_stepper::( + downcast_any_item(settings_value), + u64::saturating_sub, + u64::saturating_add, + |n| { + serde_json::Number::try_from(n) + .context("Failed to convert u64 to serde_json::Number") + }, + window, + cx, + ), + NumType::U32 => render_numeric_stepper::( + downcast_any_item(settings_value), + u32::saturating_sub, + u32::saturating_add, + |n| { + serde_json::Number::try_from(n) + .context("Failed to convert u32 to serde_json::Number") + }, + window, + cx, + ), + NumType::F32 => render_numeric_stepper::( + downcast_any_item(settings_value), + |a, b| a - b, + |a, b| a + b, + |n| { + serde_json::Number::from_f64(n as f64) + .context("Failed to convert f32 to serde_json::Number") + }, + window, + cx, + ), + } +} + +fn render_numeric_stepper< + T: serde::de::DeserializeOwned + std::fmt::Display + Copy + From + 'static, +>( + value: SettingsValue, + saturating_sub: fn(T, T) -> T, + saturating_add: fn(T, T) -> T, + to_serde_number: fn(T) -> anyhow::Result, _window: &mut Window, _cx: &mut App, ) -> AnyElement { let id = element_id_from_path(&value.path); let path = value.path.clone(); - let num = value.value.unwrap_or_else(|| value.default_value); + let num = *value.read(); NumericStepper::new( id, @@ -503,8 +549,7 @@ fn render_numeric_stepper( { let path = value.path.clone(); move |_, _, cx| { - let Some(number) = serde_json::Number::from_u128(num.saturating_sub(1) as u128) - else { + let Some(number) = to_serde_number(saturating_sub(num, 1.into())).ok() else { return; }; let new_value = serde_json::Value::Number(number); @@ -512,7 +557,7 @@ fn render_numeric_stepper( } }, move |_, _, cx| { - let Some(number) = serde_json::Number::from_u128(num.saturating_add(1) as u128) else { + let Some(number) = to_serde_number(saturating_add(num, 1.into())).ok() else { return; }; diff --git a/crates/settings_ui_macros/src/settings_ui_macros.rs b/crates/settings_ui_macros/src/settings_ui_macros.rs index 3840bc38da88a37fdbf17b8b06795f81c58ad132..947840a5d990ef32a277380f83840b10c811ff65 100644 --- a/crates/settings_ui_macros/src/settings_ui_macros.rs +++ b/crates/settings_ui_macros/src/settings_ui_macros.rs @@ -57,30 +57,22 @@ pub fn derive_settings_ui(input: proc_macro::TokenStream) -> proc_macro::TokenSt } } - if path_name.is_none() && group_name.is_some() { - // todo(settings_ui) derive path from settings - panic!("path is required when group is specified"); - } + let ui_item_fn_body = generate_ui_item_body(group_name.as_ref(), path_name.as_ref(), &input); - let ui_render_fn_body = generate_ui_item_body(group_name.as_ref(), path_name.as_ref(), &input); + // todo(settings_ui): Reformat title to be title case with spaces if group name not present, + // and make group name optional, repurpose group as tag indicating item is group + let title = group_name.unwrap_or(input.ident.to_string()); - let settings_ui_item_fn_body = path_name - .as_ref() - .map(|path_name| map_ui_item_to_render(path_name, quote! { Self })) - .unwrap_or(quote! { - settings::SettingsUiEntry { - item: settings::SettingsUiEntryVariant::None - } - }); + let ui_entry_fn_body = map_ui_item_to_entry(path_name.as_deref(), &title, quote! { Self }); let expanded = quote! { impl #impl_generics settings::SettingsUi for #name #ty_generics #where_clause { fn settings_ui_item() -> settings::SettingsUiItem { - #ui_render_fn_body + #ui_item_fn_body } fn settings_ui_entry() -> settings::SettingsUiEntry { - #settings_ui_item_fn_body + #ui_entry_fn_body } } }; @@ -114,27 +106,14 @@ fn option_inner_type(ty: TokenStream) -> Option { return Some(ty.to_token_stream()); } -fn map_ui_item_to_render(path: &str, ty: TokenStream) -> TokenStream { +fn map_ui_item_to_entry(path: Option<&str>, title: &str, ty: TokenStream) -> TokenStream { let ty = extract_type_from_option(ty); + let path = path.map_or_else(|| quote! {None}, |path| quote! {Some(#path)}); quote! { settings::SettingsUiEntry { - item: match #ty::settings_ui_item() { - settings::SettingsUiItem::Group{title, items} => settings::SettingsUiEntryVariant::Group { - title, - path: #path, - items, - }, - settings::SettingsUiItem::Single(item) => settings::SettingsUiEntryVariant::Item { - path: #path, - item, - }, - settings::SettingsUiItem::Dynamic{ options, determine_option } => settings::SettingsUiEntryVariant::Dynamic { - path: #path, - options, - determine_option, - }, - settings::SettingsUiItem::None => settings::SettingsUiEntryVariant::None, - } + title: #title, + path: #path, + item: #ty::settings_ui_item(), } } } @@ -146,16 +125,10 @@ fn generate_ui_item_body( ) -> TokenStream { match (group_name, path_name, &input.data) { (_, _, Data::Union(_)) => unimplemented!("Derive SettingsUi for Unions"), - (None, None, Data::Struct(_)) => quote! { - settings::SettingsUiItem::None - }, - (Some(_), None, Data::Struct(_)) => quote! { - settings::SettingsUiItem::None - }, - (None, Some(_), Data::Struct(_)) => quote! { + (None, _, Data::Struct(_)) => quote! { settings::SettingsUiItem::None }, - (Some(group_name), _, Data::Struct(data_struct)) => { + (Some(_), _, Data::Struct(data_struct)) => { let fields = data_struct .fields .iter() @@ -180,10 +153,11 @@ fn generate_ui_item_body( field.ty.to_token_stream(), ) }) - .map(|(name, ty)| map_ui_item_to_render(&name, ty)); + // todo(settings_ui): Re-format field name as nice title, and support setting different title with attr + .map(|(name, ty)| map_ui_item_to_entry(Some(&name), &name, ty)); quote! { - settings::SettingsUiItem::Group{ title: #group_name, items: vec![#(#fields),*] } + settings::SettingsUiItem::Group(settings::SettingsUiItemGroup{ items: vec![#(#fields),*] }) } } (None, _, Data::Enum(data_enum)) => {