From ebaefa8cbcaa0439d8bb74646d6140a05dd45db7 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Mon, 20 Oct 2025 15:25:20 -0500 Subject: [PATCH] settings_ui: Add maybe settings (#40724) Closes #ISSUE Adds a `Maybe` type to `settings_content`, that makes the distinction between `null` and omitted settings values explicit. This unlocks a few more settings in the settings UI Release Notes: - N/A *or* Added/Fixed/Improved ... --- assets/settings/default.json | 4 +- crates/settings/src/settings_content.rs | 215 ++++++++++++++++++ .../settings/src/settings_content/project.rs | 10 +- .../settings/src/settings_content/terminal.rs | 14 +- crates/settings/src/vscode_import.rs | 2 +- crates/settings_ui/src/page_data.rs | 215 ++++++++++++++++-- crates/settings_ui/src/settings_ui.rs | 3 + crates/worktree/src/worktree_settings.rs | 2 +- 8 files changed, 434 insertions(+), 31 deletions(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index 3afa21be2bb4e1e542b2610c70a7672158d274de..327b35c4b197818c09a065e2b6203430a6150665 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -1,8 +1,8 @@ { "$schema": "zed://schemas/settings", - /// The displayed name of this project. If not set or empty, the root directory name + /// The displayed name of this project. If not set or null, the root directory name /// will be displayed. - "project_name": "", + "project_name": null, // The name of the Zed theme to use for the UI. // // `mode` is one of: diff --git a/crates/settings/src/settings_content.rs b/crates/settings/src/settings_content.rs index 526bb1b5c7ab8134bc53b79d1c37092cd49144b3..94bde9e4e403a090a32e145e532114e7a3b65681 100644 --- a/crates/settings/src/settings_content.rs +++ b/crates/settings/src/settings_content.rs @@ -1034,3 +1034,218 @@ impl std::fmt::Display for DelayMs { write!(f, "{}ms", self.0) } } + +/// A wrapper type that distinguishes between an explicitly set value (including null) and an unset value. +/// +/// This is useful for configuration where you need to differentiate between: +/// - A field that is not present in the configuration file (`Maybe::Unset`) +/// - A field that is explicitly set to `null` (`Maybe::Set(None)`) +/// - A field that is explicitly set to a value (`Maybe::Set(Some(value))`) +/// +/// # Examples +/// +/// In JSON: +/// - `{}` (field missing) deserializes to `Maybe::Unset` +/// - `{"field": null}` deserializes to `Maybe::Set(None)` +/// - `{"field": "value"}` deserializes to `Maybe::Set(Some("value"))` +/// +/// WARN: This type should not be wrapped in an option inside of settings, otherwise the default `serde_json` behavior +/// of treating `null` and missing as the `Option::None` will be used +#[derive(Debug, Clone, PartialEq, Eq, strum::EnumDiscriminants, Default)] +#[strum_discriminants(derive(strum::VariantArray, strum::VariantNames, strum::FromRepr))] +pub enum Maybe { + /// An explicitly set value, which may be `None` (representing JSON `null`) or `Some(value)`. + Set(Option), + /// A value that was not present in the configuration. + #[default] + Unset, +} + +impl merge_from::MergeFrom for Maybe { + fn merge_from(&mut self, other: &Self) { + if self.is_unset() { + *self = other.clone(); + } + } +} + +impl From>> for Maybe { + fn from(value: Option>) -> Self { + match value { + Some(value) => Maybe::Set(value), + None => Maybe::Unset, + } + } +} + +impl Maybe { + pub fn is_set(&self) -> bool { + matches!(self, Maybe::Set(_)) + } + + pub fn is_unset(&self) -> bool { + matches!(self, Maybe::Unset) + } + + pub fn into_inner(self) -> Option { + match self { + Maybe::Set(value) => value, + Maybe::Unset => None, + } + } + + pub fn as_ref(&self) -> Option<&Option> { + match self { + Maybe::Set(value) => Some(value), + Maybe::Unset => None, + } + } +} + +impl serde::Serialize for Maybe { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + match self { + Maybe::Set(value) => value.serialize(serializer), + Maybe::Unset => serializer.serialize_none(), + } + } +} + +impl<'de, T: serde::Deserialize<'de>> serde::Deserialize<'de> for Maybe { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Option::::deserialize(deserializer).map(Maybe::Set) + } +} + +impl JsonSchema for Maybe { + fn schema_name() -> std::borrow::Cow<'static, str> { + format!("Nullable<{}>", T::schema_name()).into() + } + + fn json_schema(generator: &mut schemars::generate::SchemaGenerator) -> schemars::Schema { + let mut schema = generator.subschema_for::>(); + // Add description explaining that null is an explicit value + let description = if let Some(existing_desc) = + schema.get("description").and_then(|desc| desc.as_str()) + { + format!( + "{}. Note: `null` is treated as an explicit value, different from omitting the field entirely.", + existing_desc + ) + } else { + "This field supports explicit `null` values. Omitting the field is different from setting it to `null`.".to_string() + }; + + schema.insert("description".to_string(), description.into()); + + schema + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json; + + #[test] + fn test_maybe() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct TestStruct { + #[serde(default)] + #[serde(skip_serializing_if = "Maybe::is_unset")] + field: Maybe, + } + + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct NumericTest { + #[serde(default)] + value: Maybe, + } + + let json = "{}"; + let result: TestStruct = serde_json::from_str(json).unwrap(); + assert!(result.field.is_unset()); + assert_eq!(result.field, Maybe::Unset); + + let json = r#"{"field": null}"#; + let result: TestStruct = serde_json::from_str(json).unwrap(); + assert!(result.field.is_set()); + assert_eq!(result.field, Maybe::Set(None)); + + let json = r#"{"field": "hello"}"#; + let result: TestStruct = serde_json::from_str(json).unwrap(); + assert!(result.field.is_set()); + assert_eq!(result.field, Maybe::Set(Some("hello".to_string()))); + + let test = TestStruct { + field: Maybe::Unset, + }; + let json = serde_json::to_string(&test).unwrap(); + assert_eq!(json, "{}"); + + let test = TestStruct { + field: Maybe::Set(None), + }; + let json = serde_json::to_string(&test).unwrap(); + assert_eq!(json, r#"{"field":null}"#); + + let test = TestStruct { + field: Maybe::Set(Some("world".to_string())), + }; + let json = serde_json::to_string(&test).unwrap(); + assert_eq!(json, r#"{"field":"world"}"#); + + let default_maybe: Maybe = Maybe::default(); + assert!(default_maybe.is_unset()); + + let unset: Maybe = Maybe::Unset; + assert!(unset.is_unset()); + assert!(!unset.is_set()); + + let set_none: Maybe = Maybe::Set(None); + assert!(set_none.is_set()); + assert!(!set_none.is_unset()); + + let set_some: Maybe = Maybe::Set(Some("value".to_string())); + assert!(set_some.is_set()); + assert!(!set_some.is_unset()); + + let original = TestStruct { + field: Maybe::Set(Some("test".to_string())), + }; + let json = serde_json::to_string(&original).unwrap(); + let deserialized: TestStruct = serde_json::from_str(&json).unwrap(); + assert_eq!(original, deserialized); + + let json = r#"{"value": 42}"#; + let result: NumericTest = serde_json::from_str(json).unwrap(); + assert_eq!(result.value, Maybe::Set(Some(42))); + + let json = r#"{"value": null}"#; + let result: NumericTest = serde_json::from_str(json).unwrap(); + assert_eq!(result.value, Maybe::Set(None)); + + let json = "{}"; + let result: NumericTest = serde_json::from_str(json).unwrap(); + assert_eq!(result.value, Maybe::Unset); + + // Test JsonSchema implementation + use schemars::schema_for; + let schema = schema_for!(Maybe); + let schema_json = serde_json::to_value(&schema).unwrap(); + + // Verify the description mentions that null is an explicit value + let description = schema_json["description"].as_str().unwrap(); + assert!( + description.contains("null") && description.contains("explicit"), + "Schema description should mention that null is an explicit value. Got: {}", + description + ); + } +} diff --git a/crates/settings/src/settings_content/project.rs b/crates/settings/src/settings_content/project.rs index a26a72e7aa32d513dd5afe3c7aa5078f3e3201a5..6a77b815fa547d41e6f38541fe1d681c82b3347b 100644 --- a/crates/settings/src/settings_content/project.rs +++ b/crates/settings/src/settings_content/project.rs @@ -8,7 +8,7 @@ use settings_macros::MergeFrom; use util::serde::default_true; use crate::{ - AllLanguageSettingsContent, DelayMs, ExtendingVec, ProjectTerminalSettingsContent, + AllLanguageSettingsContent, DelayMs, ExtendingVec, Maybe, ProjectTerminalSettingsContent, SlashCommandSettings, }; @@ -56,11 +56,13 @@ pub struct ProjectSettingsContent { #[skip_serializing_none] #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, JsonSchema, MergeFrom)] pub struct WorktreeSettingsContent { - /// The displayed name of this project. If not set or empty, the root directory name + /// The displayed name of this project. If not set or null, the root directory name /// will be displayed. /// - /// Default: "" - pub project_name: Option, + /// Default: null + #[serde(default)] + #[serde(skip_serializing_if = "Maybe::is_unset")] + pub project_name: Maybe, /// Completely ignore files matching globs from `file_scan_exclusions`. Overrides /// `file_scan_inclusions`. diff --git a/crates/settings/src/settings_content/terminal.rs b/crates/settings/src/settings_content/terminal.rs index aba2d5209a071b1d8aa50a7feea2965d9cafb948..e1b101e5ae72a260ec90c01a63315b2b1c3f2c11 100644 --- a/crates/settings/src/settings_content/terminal.rs +++ b/crates/settings/src/settings_content/terminal.rs @@ -134,7 +134,19 @@ pub struct TerminalSettingsContent { } /// Shell configuration to open the terminal with. -#[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq, Eq, JsonSchema, MergeFrom)] +#[derive( + Clone, + Debug, + Default, + Serialize, + Deserialize, + PartialEq, + Eq, + JsonSchema, + MergeFrom, + strum::EnumDiscriminants, +)] +#[strum_discriminants(derive(strum::VariantArray, strum::VariantNames, strum::FromRepr))] #[serde(rename_all = "snake_case")] pub enum Shell { /// Use the system's default terminal configuration in /etc/passwd diff --git a/crates/settings/src/vscode_import.rs b/crates/settings/src/vscode_import.rs index c07f75a9e3b96bea689f5d6dda22d0742800d321..586f5685a8962aef1f6a13c29c19f0bcf3e557b4 100644 --- a/crates/settings/src/vscode_import.rs +++ b/crates/settings/src/vscode_import.rs @@ -853,7 +853,7 @@ impl VsCodeSettings { fn worktree_settings_content(&self) -> WorktreeSettingsContent { WorktreeSettingsContent { - project_name: None, + project_name: crate::Maybe::Unset, file_scan_exclusions: self .read_value("files.watcherExclude") .and_then(|v| v.as_array()) diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index 19a495cf770cddd4dd3252e23ef5be8a6c1308eb..7ced4000349612b7a9629e33a6934ce801645b86 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -9,6 +9,16 @@ use crate::{ SettingsPageItem, SubPageLink, USER, all_language_names, sub_page_stack, }; +const DEFAULT_STRING: String = String::new(); +/// A default empty string reference. Useful in `pick` functions for cases either in dynamic item fields, or when dealing with `settings::Maybe` +/// to avoid the "NO DEFAULT" case. +const DEFAULT_EMPTY_STRING: Option<&String> = Some(&DEFAULT_STRING); + +const DEFAULT_SHARED_STRING: SharedString = SharedString::new_static(""); +/// A default empty string reference. Useful in `pick` functions for cases either in dynamic item fields, or when dealing with `settings::Maybe` +/// to avoid the "NO DEFAULT" case. +const DEFAULT_EMPTY_SHARED_STRING: Option<&SharedString> = Some(&DEFAULT_SHARED_STRING); + pub(crate) fn settings_data(cx: &App) -> Vec { vec![ SettingsPage { @@ -16,16 +26,20 @@ pub(crate) fn settings_data(cx: &App) -> Vec { items: vec![ SettingsPageItem::SectionHeader("General Settings"), SettingsPageItem::SettingItem(SettingItem { - title: "Confirm Quit", - description: "Confirm before quitting Zed", - field: Box::new(SettingField { - pick: |settings_content| settings_content.workspace.confirm_quit.as_ref(), - write: |settings_content, value| { - settings_content.workspace.confirm_quit = value; - }, - }), - metadata: None, - files: USER, + files: LOCAL, + title: "Project Name", + description: "The Displayed Name Of This Project. If Left Empty, The Root Directory Name Will Be Displayed", + field: Box::new( + SettingField { + pick: |settings_content| { + settings_content.project.worktree.project_name.as_ref()?.as_ref().or(DEFAULT_EMPTY_STRING) + }, + write: |settings_content, value| { + settings_content.project.worktree.project_name = settings::Maybe::Set(value.filter(|name| !name.is_empty())); + }, + } + ), + metadata: Some(Box::new(SettingsFieldMetadata { placeholder: Some("Project Name"), ..Default::default() })), }), SettingsPageItem::SettingItem(SettingItem { title: "When Closing With No Tabs", @@ -4205,26 +4219,183 @@ pub(crate) fn settings_data(cx: &App) -> Vec { title: "Terminal", items: vec![ SettingsPageItem::SectionHeader("Environment"), - SettingsPageItem::SettingItem(SettingItem { - title: "Shell", - description: "What shell to use when opening a terminal", - field: Box::new( - SettingField { + SettingsPageItem::DynamicItem(DynamicItem { + discriminant: SettingItem { + files: USER | LOCAL, + title: "Shell", + description: "What shell to use when opening a terminal", + field: Box::new(SettingField { pick: |settings_content| { - settings_content.terminal.as_ref()?.project.shell.as_ref() + Some(&dynamic_variants::()[ + settings_content + .terminal + .as_ref()? + .project + .shell + .as_ref()? + .discriminant() as usize]) }, write: |settings_content, value| { - settings_content + let Some(value) = value else { + return; + }; + let settings_value = settings_content .terminal .get_or_insert_default() .project - .shell = value; + .shell + .get_or_insert_with(|| settings::Shell::default()); + *settings_value = match value { + settings::ShellDiscriminants::System => { + settings::Shell::System + }, + settings::ShellDiscriminants::Program => { + let program = match settings_value { + settings::Shell::Program(p) => p.clone(), + settings::Shell::WithArguments { program, .. } => program.clone(), + _ => String::from("sh"), + }; + settings::Shell::Program(program) + }, + settings::ShellDiscriminants::WithArguments => { + let (program, args, title_override) = match settings_value { + settings::Shell::Program(p) => (p.clone(), vec![], None), + settings::Shell::WithArguments { program, args, title_override } => { + (program.clone(), args.clone(), title_override.clone()) + }, + _ => (String::from("sh"), vec![], None), + }; + settings::Shell::WithArguments { + program, + args, + title_override, + } + }, + }; }, + }), + metadata: None, + }, + pick_discriminant: |settings_content| { + Some(settings_content.terminal.as_ref()?.project.shell.as_ref()?.discriminant() as usize) + }, + fields: dynamic_variants::().into_iter().map(|variant| { + match variant { + settings::ShellDiscriminants::System => vec![], + settings::ShellDiscriminants::Program => vec![ + SettingItem { + files: USER | LOCAL, + title: "Program", + description: "The shell program to use", + field: Box::new(SettingField { + pick: |settings_content| { + match settings_content.terminal.as_ref()?.project.shell.as_ref() { + Some(settings::Shell::Program(program)) => Some(program), + _ => None + } + }, + write: |settings_content, value| { + let Some(value) = value else { + return; + }; + match settings_content + .terminal + .get_or_insert_default() + .project + .shell.as_mut() { + Some(settings::Shell::Program(program)) => *program = value, + _ => return + } + }, + }), + metadata: None, + } + ], + settings::ShellDiscriminants::WithArguments => vec![ + SettingItem { + files: USER | LOCAL, + title: "Program", + description: "The shell program to run", + field: Box::new(SettingField { + pick: |settings_content| { + match settings_content.terminal.as_ref()?.project.shell.as_ref() { + Some(settings::Shell::WithArguments { program, .. }) => Some(program), + _ => None + } + }, + write: |settings_content, value| { + let Some(value) = value else { + return; + }; + match settings_content + .terminal + .get_or_insert_default() + .project + .shell.as_mut() { + Some(settings::Shell::WithArguments { program, .. }) => *program = value, + _ => return + } + }, + }), + metadata: None, + }, + SettingItem { + files: USER | LOCAL, + title: "Arguments", + description: "The arguments to pass to the shell program", + field: Box::new( + SettingField { + pick: |settings_content| { + match settings_content.terminal.as_ref()?.project.shell.as_ref() { + Some(settings::Shell::WithArguments { args, .. }) => Some(args), + _ => None + } + }, + write: |settings_content, value| { + let Some(value) = value else { + return; + }; + match settings_content + .terminal + .get_or_insert_default() + .project + .shell.as_mut() { + Some(settings::Shell::WithArguments { args, .. }) => *args = value, + _ => return + } + }, + } + .unimplemented(), + ), + metadata: None, + }, + SettingItem { + files: USER | LOCAL, + title: "Title Override", + description: "An optional string to override the title of the terminal tab", + field: Box::new(SettingField { + pick: |settings_content| { + match settings_content.terminal.as_ref()?.project.shell.as_ref() { + Some(settings::Shell::WithArguments { title_override, .. }) => title_override.as_ref().or(DEFAULT_EMPTY_SHARED_STRING), + _ => None + } + }, + write: |settings_content, value| { + match settings_content + .terminal + .get_or_insert_default() + .project + .shell.as_mut() { + Some(settings::Shell::WithArguments { title_override, .. }) => *title_override = value.filter(|s| !s.is_empty()), + _ => return + } + }, + }), + metadata: None, + } + ], } - .unimplemented(), - ), - metadata: None, - files: USER | LOCAL, + }).collect(), }), SettingsPageItem::DynamicItem(DynamicItem { discriminant: SettingItem { diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index d430f719c9476e4216e5fedd42269b13b916f4fe..c9b1b7ac9e7632bb9741cab396ee5f2d6252603c 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -370,6 +370,7 @@ fn init_renderers(cx: &mut App) { }) .add_basic_renderer::(render_toggle_button) .add_basic_renderer::(render_text_field) + .add_basic_renderer::(render_text_field) .add_basic_renderer::(render_toggle_button) .add_basic_renderer::(render_dropdown) .add_basic_renderer::(render_dropdown) @@ -444,8 +445,10 @@ fn init_renderers(cx: &mut App) { .add_basic_renderer::(render_dropdown) .add_basic_renderer::(render_dropdown) .add_basic_renderer::(render_dropdown) + .add_basic_renderer::(render_dropdown) .add_basic_renderer::(render_dropdown) .add_basic_renderer::(render_dropdown) + .add_basic_renderer::(render_dropdown) // please semicolon stay on next line ; } diff --git a/crates/worktree/src/worktree_settings.rs b/crates/worktree/src/worktree_settings.rs index 8e432f8affbbfa9e7eb53ec474970c43ec0e8a94..9eddef8eaf43cecca949ea6f595c75795698ab38 100644 --- a/crates/worktree/src/worktree_settings.rs +++ b/crates/worktree/src/worktree_settings.rs @@ -50,7 +50,7 @@ impl Settings for WorktreeSettings { .collect(); Self { - project_name: worktree.project_name.filter(|p| !p.is_empty()), + project_name: worktree.project_name.into_inner(), file_scan_exclusions: path_matchers(file_scan_exclusions, "file_scan_exclusions") .log_err() .unwrap_or_default(),