settings_ui: Improve case handling (#37342)

Ben Kunkle created

Closes #ISSUE

Improves the derive macro for `SettingsUi` so that titles generated from
struct and field names are shown in title case, and toggle button groups
use title case for rendering, while using lower case/snake case in JSON

Release Notes:

- N/A *or* Added/Fixed/Improved ...

Change summary

Cargo.lock                                          |  1 
crates/settings/src/settings_ui_core.rs             | 15 +++++-
crates/settings_ui/src/settings_ui.rs               | 33 +++++++++-----
crates/settings_ui_macros/Cargo.toml                |  1 
crates/settings_ui_macros/src/settings_ui_macros.rs | 28 ++++++++----
5 files changed, 53 insertions(+), 25 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -14921,6 +14921,7 @@ dependencies = [
 name = "settings_ui_macros"
 version = "0.1.0"
 dependencies = [
+ "heck 0.5.0",
  "proc-macro2",
  "quote",
  "syn 2.0.101",

crates/settings/src/settings_ui_core.rs 🔗

@@ -24,7 +24,6 @@ pub trait SettingsUi {
 }
 
 pub struct SettingsUiEntry {
-    // todo(settings_ui): move this back here once there isn't a None variant
     /// 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>,
@@ -36,9 +35,19 @@ pub enum SettingsUiItemSingle {
     SwitchField,
     /// A numeric stepper for a specific type of number
     NumericStepper(NumType),
-    ToggleGroup(&'static [&'static str]),
+    ToggleGroup {
+        /// Must be the same length as `labels`
+        variants: &'static [&'static str],
+        /// Must be the same length as `variants`
+        labels: &'static [&'static str],
+    },
     /// This should be used when toggle group size > 6
-    DropDown(&'static [&'static str]),
+    DropDown {
+        /// Must be the same length as `labels`
+        variants: &'static [&'static str],
+        /// Must be the same length as `variants`
+        labels: &'static [&'static str],
+    },
     Custom(Box<dyn Fn(SettingsValue<serde_json::Value>, &mut Window, &mut App) -> AnyElement>),
 }
 

crates/settings_ui/src/settings_ui.rs 🔗

@@ -329,6 +329,7 @@ fn render_content(
         }
         let settings_value = settings_value_from_settings_and_path(
             path.clone(),
+            child.title,
             // PERF: how to structure this better? There feels like there's a way to avoid the clone
             // and every value lookup
             SettingsStore::global(cx).raw_user_settings(),
@@ -433,10 +434,11 @@ fn render_item_single(
         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)
-        }
-        SettingsUiItemSingle::DropDown(_) => {
+        SettingsUiItemSingle::ToggleGroup {
+            variants: values,
+            labels: titles,
+        } => render_toggle_button_group(settings_value, values, titles, window, cx),
+        SettingsUiItemSingle::DropDown { .. } => {
             unimplemented!("This")
         }
     }
@@ -603,6 +605,7 @@ fn render_switch_field(
 fn render_toggle_button_group(
     value: SettingsValue<serde_json::Value>,
     variants: &'static [&'static str],
+    labels: &'static [&'static str],
     _: &mut Window,
     _: &mut App,
 ) -> AnyElement {
@@ -612,15 +615,18 @@ fn render_toggle_button_group(
         group_name: &'static str,
         value: SettingsValue<String>,
         variants: &'static [&'static str],
+        labels: &'static [&'static str],
     ) -> AnyElement {
-        let mut variants_array: [&'static str; LEN] = ["default"; LEN];
-        variants_array.copy_from_slice(variants);
+        let mut variants_array: [(&'static str, &'static str); LEN] = [("unused", "unused"); LEN];
+        for i in 0..LEN {
+            variants_array[i] = (variants[i], labels[i]);
+        }
         let active_value = value.read();
 
         let selected_idx = variants_array
             .iter()
             .enumerate()
-            .find_map(|(idx, variant)| {
+            .find_map(|(idx, (variant, _))| {
                 if variant == &active_value {
                     Some(idx)
                 } else {
@@ -628,11 +634,13 @@ fn render_toggle_button_group(
                 }
             });
 
+        let mut idx = 0;
         ToggleButtonGroup::single_row(
             group_name,
-            variants_array.map(|variant| {
+            variants_array.map(|(variant, label)| {
                 let path = value.path.clone();
-                ToggleButtonSimple::new(variant, move |_, _, cx| {
+                idx += 1;
+                ToggleButtonSimple::new(label, move |_, _, cx| {
                     SettingsValue::write_value(
                         &path,
                         serde_json::Value::String(variant.to_string()),
@@ -649,7 +657,7 @@ fn render_toggle_button_group(
     macro_rules! templ_toggl_with_const_param {
         ($len:expr) => {
             if variants.len() == $len {
-                return make_toggle_group::<$len>(value.title, value, variants);
+                return make_toggle_group::<$len>(value.title, value, variants, labels);
             }
         };
     }
@@ -664,6 +672,7 @@ fn render_toggle_button_group(
 
 fn settings_value_from_settings_and_path(
     path: SmallVec<[&'static str; 1]>,
+    title: &'static str,
     user_settings: &serde_json::Value,
     default_settings: &serde_json::Value,
 ) -> SettingsValue<serde_json::Value> {
@@ -677,8 +686,8 @@ fn settings_value_from_settings_and_path(
         default_value,
         value,
         path: path.clone(),
-        // todo(settings_ui) title for items
-        title: path.last().expect("path non empty"),
+        // todo(settings_ui) is title required inside SettingsValue?
+        title,
     };
     return settings_value;
 }

crates/settings_ui_macros/Cargo.toml 🔗

@@ -16,6 +16,7 @@ workspace = true
 default = []
 
 [dependencies]
+heck.workspace = true
 proc-macro2.workspace = true
 quote.workspace = true
 syn.workspace = true

crates/settings_ui_macros/src/settings_ui_macros.rs 🔗

@@ -1,3 +1,4 @@
+use heck::{ToSnakeCase as _, ToTitleCase as _};
 use proc_macro2::TokenStream;
 use quote::{ToTokens, quote};
 use syn::{Data, DeriveInput, LitStr, Token, parse_macro_input};
@@ -59,9 +60,8 @@ pub fn derive_settings_ui(input: proc_macro::TokenStream) -> proc_macro::TokenSt
 
     let ui_item_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());
+    // todo(settings_ui): make group name optional, repurpose group as tag indicating item is group, and have "title" tag for custom title
+    let title = group_name.unwrap_or(input.ident.to_string().to_title_case());
 
     let ui_entry_fn_body = map_ui_item_to_entry(path_name.as_deref(), &title, quote! { Self });
 
@@ -154,7 +154,7 @@ fn generate_ui_item_body(
                     )
                 })
                 // 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));
+                .map(|(name, ty)| map_ui_item_to_entry(Some(&name), &name.to_title_case(), ty));
 
             quote! {
                 settings::SettingsUiItem::Group(settings::SettingsUiItemGroup{ items: vec![#(#fields),*] })
@@ -162,14 +162,15 @@ fn generate_ui_item_body(
         }
         (None, _, Data::Enum(data_enum)) => {
             let mut lowercase = false;
+            let mut snake_case = false;
             for attr in &input.attrs {
                 if attr.path().is_ident("serde") {
                     attr.parse_nested_meta(|meta| {
                         if meta.path.is_ident("rename_all") {
                             meta.input.parse::<Token![=]>()?;
                             let lit = meta.input.parse::<LitStr>()?.value();
-                            // todo(settings_ui) snake case
-                            lowercase = lit == "lowercase" || lit == "snake_case";
+                            lowercase = lit == "lowercase";
+                            snake_case = lit == "snake_case";
                         }
                         Ok(())
                     })
@@ -181,20 +182,27 @@ fn generate_ui_item_body(
             let variants = data_enum.variants.iter().map(|variant| {
                 let string = variant.ident.clone().to_string();
 
-                if lowercase {
+                let title = string.to_title_case();
+                let string = if lowercase {
                     string.to_lowercase()
+                } else if snake_case {
+                    string.to_snake_case()
                 } else {
                     string
-                }
+                };
+
+                (string, title)
             });
 
+            let (variants, labels): (Vec<_>, Vec<_>) = variants.unzip();
+
             if length > 6 {
                 quote! {
-                    settings::SettingsUiItem::Single(settings::SettingsUiItemSingle::DropDown(&[#(#variants),*]))
+                    settings::SettingsUiItem::Single(settings::SettingsUiItemSingle::DropDown{ variants: &[#(#variants),*], labels: &[#(#labels),*] })
                 }
             } else {
                 quote! {
-                    settings::SettingsUiItem::Single(settings::SettingsUiItemSingle::ToggleGroup(&[#(#variants),*]))
+                    settings::SettingsUiItem::Single(settings::SettingsUiItemSingle::ToggleGroup{ variants: &[#(#variants),*], labels: &[#(#labels),*] })
                 }
             }
         }