diff --git a/crates/settings/src/settings_content.rs b/crates/settings/src/settings_content.rs index 42b88bd3654159ca3ad55dfecffbe3d4e2b547d0..9cd8ff46e8bea5be69bd5415b5668f21dc71f13a 100644 --- a/crates/settings/src/settings_content.rs +++ b/crates/settings/src/settings_content.rs @@ -1039,218 +1039,3 @@ 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 6c450bc8384d61acf9d6f894f2ae3de500611618..83e0537940870bd944cb75f20e35cc522059570c 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, Maybe, ProjectTerminalSettingsContent, + AllLanguageSettingsContent, DelayMs, ExtendingVec, ProjectTerminalSettingsContent, SlashCommandSettings, }; @@ -61,8 +61,8 @@ pub struct WorktreeSettingsContent { /// /// Default: null #[serde(default)] - #[serde(skip_serializing_if = "Maybe::is_unset")] - pub project_name: Maybe, + #[serde(skip_serializing_if = "Option::is_none")] + pub project_name: Option, /// Whether to prevent this project from being shared in public channels. /// diff --git a/crates/settings/src/vscode_import.rs b/crates/settings/src/vscode_import.rs index 4b87d6f5f30c075f90a6da698396bc4576a56b92..5644cd7a1a8463a1f072d838a0a1b16bd7ad991b 100644 --- a/crates/settings/src/vscode_import.rs +++ b/crates/settings/src/vscode_import.rs @@ -870,7 +870,7 @@ impl VsCodeSettings { fn worktree_settings_content(&self) -> WorktreeSettingsContent { WorktreeSettingsContent { - project_name: crate::Maybe::Unset, + project_name: None, prevent_sharing_in_public_channels: false, file_scan_exclusions: self .read_value("files.watcherExclude") diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index d776b9787eb804a77f2d4a6b6c605846eb6604ea..a6baaf94842955a323f348dcbae8130dcfd060c6 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -33,10 +33,10 @@ pub(crate) fn settings_data(cx: &App) -> Vec { SettingField { json_path: Some("project_name"), pick: |settings_content| { - settings_content.project.worktree.project_name.as_ref()?.as_ref().or(DEFAULT_EMPTY_STRING) + settings_content.project.worktree.project_name.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())); + settings_content.project.worktree.project_name = value.filter(|name| !name.is_empty()); }, } ), diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 1f32716f0639197cf6391e377b2619cc3843605f..7b90464633c47caf7a2b11421fdbc6ac5aefe129 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -507,7 +507,6 @@ 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) diff --git a/crates/worktree/src/worktree_settings.rs b/crates/worktree/src/worktree_settings.rs index 94e83a16decd6b5d68498944e26ddcabecd27eed..97723829dd78a3dab517634971f8d0753500aa4b 100644 --- a/crates/worktree/src/worktree_settings.rs +++ b/crates/worktree/src/worktree_settings.rs @@ -66,7 +66,7 @@ impl Settings for WorktreeSettings { .collect(); Self { - project_name: worktree.project_name.into_inner(), + project_name: worktree.project_name, prevent_sharing_in_public_channels: worktree.prevent_sharing_in_public_channels, file_scan_exclusions: path_matchers(file_scan_exclusions, "file_scan_exclusions") .log_err()