wip - debugging why there is empty input, and why saving does not result in new create row

crates/language/src/language_settings.rs

@@ -552,7 +552,6 @@ pub struct LanguageSettingsContent {
     ///
     /// Default: ["..."]
     #[serde(default)]
-    #[settings_ui(skip)]
     pub language_servers: Option<Vec<String>>,
     /// Controls where the `editor::Rewrap` action is allowed for this language.
     ///

crates/settings/src/settings_ui_core.rs

@@ -121,6 +121,7 @@ pub struct SettingsUiItemUnion {
 #[derive(Clone)]
 pub struct SettingsEnumVariants {}
 
+#[derive(Debug)]
 pub struct SettingsUiEntryMetaData {
     pub title: SharedString,
     pub path: SharedString,
@@ -136,7 +137,14 @@ pub struct SettingsUiItemDynamicMap {
 
 #[derive(Clone)]
 pub struct SettingsUiItemGroup {
-    pub items: Vec<SettingsUiEntry>,
+    pub items: Box<[SettingsUiEntry]>,
+}
+
+#[derive(Clone)]
+pub struct SettingsUiItemArray {
+    pub item: fn() -> SettingsUiItem,
+    pub determine_items: fn(&serde_json::Value, &App) -> Vec<SettingsUiEntryMetaData>,
+    pub default_item: serde_json::Value,
 }
 
 #[derive(Clone)]
@@ -145,7 +153,7 @@ pub enum SettingsUiItem {
     Single(SettingsUiItemSingle),
     Union(SettingsUiItemUnion),
     DynamicMap(SettingsUiItemDynamicMap),
-    // Array(SettingsUiItemArray), // code-actions: array of objects, array of string
+    Array(SettingsUiItemArray), // code-actions: array of objects, array of string
     None,
 }
 
@@ -167,9 +175,25 @@ impl SettingsUi for String {
     }
 }
 
-impl SettingsUi for SettingsUiItem {
+impl<T: SettingsUi + Default + serde::Serialize> SettingsUi for Vec<T> {
     fn settings_ui_item() -> SettingsUiItem {
-        SettingsUiItem::Single(SettingsUiItemSingle::TextField)
+        SettingsUiItem::Array(SettingsUiItemArray {
+            item: T::settings_ui_item,
+            default_item: serde_json::to_value(T::default()).unwrap(),
+            determine_items: |value, _cx| {
+                let items: &[serde_json::Value] =
+                    value.as_array().map(Vec::as_slice).unwrap_or(&[]);
+                let mut metadata = Vec::with_capacity(items.len());
+                for index in 0..items.len() {
+                    metadata.push(SettingsUiEntryMetaData {
+                        title: serde_json::to_string(&items[index]).unwrap().into(),
+                        path: format!("#{}", index).into(),
+                        documentation: None,
+                    });
+                }
+                return metadata;
+            },
+        })
     }
 }
 

crates/settings_ui/src/settings_ui.rs

@@ -12,13 +12,13 @@ use feature_flags::{FeatureFlag, FeatureFlagAppExt};
 use gpui::{App, Entity, EventEmitter, FocusHandle, Focusable, ReadGlobal, ScrollHandle, actions};
 use settings::{
     NumType, SettingsStore, SettingsUiEntry, SettingsUiEntryMetaData, SettingsUiItem,
-    SettingsUiItemDynamicMap, SettingsUiItemGroup, SettingsUiItemSingle, SettingsUiItemUnion,
-    SettingsValue,
+    SettingsUiItemArray, SettingsUiItemDynamicMap, SettingsUiItemGroup, SettingsUiItemSingle,
+    SettingsUiItemUnion, SettingsValue,
 };
 use smallvec::SmallVec;
 use ui::{
-    ContextMenu, DropdownMenu, NumericStepper, SwitchField, ToggleButtonGroup, ToggleButtonSimple,
-    prelude::*,
+    ContextMenu, DropdownMenu, NumericStepper, SwitchField, TableInteractionState,
+    ToggleButtonGroup, ToggleButtonSimple, prelude::*,
 };
 use workspace::{
     Workspace,
@@ -157,6 +157,7 @@ struct UiEntry {
         fn(&serde_json::Value, &App) -> Vec<SettingsUiEntryMetaData>,
         SmallVec<[SharedString; 1]>,
     )>,
+    array: Option<SettingsUiItemArray>,
 }
 
 impl UiEntry {
@@ -207,6 +208,7 @@ fn build_tree_item(
         next_sibling: None,
         dynamic_render: None,
         generate_items: None,
+        array: None,
     });
     if let Some(prev_index) = prev_index {
         tree[prev_index].next_sibling = Some(index);
@@ -260,6 +262,9 @@ fn build_tree_item(
                     .collect(),
             ));
         }
+        SettingsUiItem::Array(array) => {
+            tree[index].array = Some(array);
+        }
         SettingsUiItem::None => {
             return;
         }
@@ -388,7 +393,7 @@ fn render_content(
         tree.active_entry_index,
         &mut path,
         content,
-        &mut None,
+        None,
         true,
         window,
         cx,
@@ -397,43 +402,45 @@ fn render_content(
 
 fn render_recursive(
     tree: &[UiEntry],
-    index: usize,
+    entry_index: usize,
     path: &mut SmallVec<[SharedString; 1]>,
     mut element: Div,
-    fallback_path: &mut Option<SmallVec<[SharedString; 1]>>,
+    parent_fallback_default_value: Option<&serde_json::Value>,
     render_next_title: bool,
     window: &mut Window,
     cx: &mut App,
 ) -> Div {
-    let Some(child) = tree.get(index) else {
+    let Some(entry) = tree.get(entry_index) else {
         return element
             .child(Label::new(SharedString::new_static("No settings found")).color(Color::Error));
     };
 
     if render_next_title {
-        element = element.child(Label::new(child.title.clone()).size(LabelSize::Large));
+        element = element.child(Label::new(entry.title.clone()).size(LabelSize::Large));
     }
 
     // todo(settings_ui): subgroups?
     let mut pushed_path = false;
-    if let Some(child_path) = child.path.as_ref() {
+    let mut fallback_default_value = parent_fallback_default_value;
+    if let Some(child_path) = entry.path.as_ref() {
         path.push(child_path.clone());
-        if let Some(fallback_path) = fallback_path.as_mut() {
-            fallback_path.push(child_path.clone());
+        if let Some(fallback_value) = parent_fallback_default_value.as_ref() {
+            fallback_default_value =
+                read_settings_value_from_path(fallback_value, std::slice::from_ref(&child_path));
         }
         pushed_path = true;
     }
     let settings_value = settings_value_from_settings_and_path(
         path.clone(),
-        fallback_path.as_ref().map(|path| path.as_slice()),
-        child.title.clone(),
-        child.documentation.clone(),
+        fallback_default_value,
+        entry.title.clone(),
+        entry.documentation.clone(),
         // 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(),
         SettingsStore::global(cx).raw_default_settings(),
     );
-    if let Some(dynamic_render) = child.dynamic_render.as_ref() {
+    if let Some(dynamic_render) = entry.dynamic_render.as_ref() {
         let value = settings_value.read();
         let selected_index = (dynamic_render.determine_option)(value, cx);
         element = element.child(div().child(render_toggle_button_group_inner(
@@ -461,24 +468,96 @@ fn render_recursive(
                 .count();
         if dynamic_render.options[selected_index].is_some()
             && let Some(descendant_index) =
-                child.nth_descendant_index(tree, selected_descendant_index)
+                entry.nth_descendant_index(tree, selected_descendant_index)
         {
             element = render_recursive(
                 tree,
                 descendant_index,
                 path,
                 element,
-                fallback_path,
+                fallback_default_value,
                 false,
                 window,
                 cx,
             );
         }
+    } else if let Some(array) = entry.array.as_ref() {
+        let generated_items = (array.determine_items)(settings_value.read(), cx);
+        let mut ui_items = Vec::with_capacity(generated_items.len());
+        let settings_ui_item = (array.item)();
+
+        let table_interaction_state =
+            window.use_keyed_state(("settings_ui_table", entry_index), cx, |_, cx| {
+                TableInteractionState::new(cx)
+            });
+
+        let mut table = ui::Table::<2>::new()
+            .column_widths([relative(0.1), relative(0.9)])
+            .header(["#", "Value"])
+            .interactable(&table_interaction_state)
+            .striped();
+        let mut row_count = 0;
+
+        // todo(settings_ui): Try to make the static item on these items built into the tree
+        // because we already propagate the path down so they don't all have to be recreated
+        for (index, item) in generated_items.iter().enumerate() {
+            let settings_ui_entry = SettingsUiEntry {
+                path: None,
+                title: "",
+                documentation: None,
+                item: settings_ui_item.clone(),
+            };
+            let prev_index = if ui_items.is_empty() {
+                None
+            } else {
+                Some(ui_items.len() - 1)
+            };
+            let item_index = ui_items.len();
+            build_tree_item(
+                &mut ui_items,
+                settings_ui_entry,
+                entry._depth + 1,
+                prev_index,
+            );
+            if item_index < ui_items.len() {
+                ui_items[item_index].path = None;
+                ui_items[item_index].title = item.title.clone();
+                ui_items[item_index].documentation = item.documentation.clone();
+
+                // push path instead of setting path on ui item so that the path isn't
+                // pushed to default_default_value as well when we recurse
+                path.push(item.path.clone());
+                dbg!(path.join("."));
+                let row_element = render_recursive(
+                    &ui_items,
+                    item_index,
+                    path,
+                    div(),
+                    Some(&array.default_item),
+                    false,
+                    window,
+                    cx,
+                );
+                path.pop();
+                table = table.row([index.to_string().into_any_element(), row_element.into_any()]);
+                row_count += 1;
+            }
+        }
+        table = table.row([
+            row_count.to_string().into_any_element(),
+            "create".into_any(),
+        ]);
+        element = element.child(div().child(table).debug());
     } else if let Some((settings_ui_item, generate_items, defaults_path)) =
-        child.generate_items.as_ref()
+        entry.generate_items.as_ref()
     {
         let generated_items = generate_items(settings_value.read(), cx);
         let mut ui_items = Vec::with_capacity(generated_items.len());
+        let default_value = read_settings_value_from_path(
+            SettingsStore::global(cx).raw_default_settings(),
+            &defaults_path,
+        )
+        .cloned();
         for item in generated_items {
             let settings_ui_entry = SettingsUiEntry {
                 path: None,
@@ -495,7 +574,7 @@ fn render_recursive(
             build_tree_item(
                 &mut ui_items,
                 settings_ui_entry,
-                child._depth + 1,
+                entry._depth + 1,
                 prev_index,
             );
             if item_index < ui_items.len() {
@@ -511,7 +590,7 @@ fn render_recursive(
                     item_index,
                     path,
                     element,
-                    &mut Some(defaults_path.clone()),
+                    default_value.as_ref(),
                     true,
                     window,
                     cx,
@@ -519,14 +598,14 @@ fn render_recursive(
                 path.pop();
             }
         }
-    } else if let Some(child_render) = child.render.as_ref() {
+    } else if let Some(child_render) = entry.render.as_ref() {
         element = element.child(div().child(render_item_single(
             settings_value,
             child_render,
             window,
             cx,
         )));
-    } else if let Some(child_index) = child.first_descendant_index() {
+    } else if let Some(child_index) = entry.first_descendant_index() {
         let mut index = Some(child_index);
         while let Some(sub_child_index) = index {
             element = render_recursive(
@@ -534,7 +613,7 @@ fn render_recursive(
                 sub_child_index,
                 path,
                 element,
-                fallback_path,
+                fallback_default_value,
                 true,
                 window,
                 cx,
@@ -547,9 +626,6 @@ fn render_recursive(
 
     if pushed_path {
         path.pop();
-        if let Some(fallback_path) = fallback_path.as_mut() {
-            fallback_path.pop();
-        }
     }
     return element;
 }
@@ -835,26 +911,51 @@ fn render_text_field(
 ) -> AnyElement {
     let value = downcast_any_item::<String>(value);
     let path = value.path.clone();
-    let editor = window.use_state(cx, {
+    let current_text = value.read().clone();
+
+    let dirty = window.use_keyed_state((element_id_from_path(&path), "dirty"), cx, |_, _| false);
+
+    let editor = window.use_keyed_state((element_id_from_path(&path), "editor"), cx, {
         let path = path.clone();
+        let dirty = dirty.clone();
         move |window, cx| {
             let mut editor = Editor::single_line(window, cx);
+            // editor.set_text(current_text, window, cx);
 
-            cx.observe_global_in::<SettingsStore>(window, move |editor, window, cx| {
-                let user_settings = SettingsStore::global(cx).raw_user_settings();
-                if let Some(value) = read_settings_value_from_path(&user_settings, &path).cloned()
-                    && let Some(value) = value.as_str()
-                {
-                    editor.set_text(value, window, cx);
+            let dirty = dirty.downgrade();
+            cx.subscribe_self(move |_, event: &editor::EditorEvent, cx| match event {
+                editor::EditorEvent::BufferEdited => {
+                    let Some(dirty) = dirty.upgrade() else { return };
+                    dirty.write(cx, true);
                 }
+                _ => {}
             })
             .detach();
+            // cx.observe_global_in::<SettingsStore>(window, move |editor, window, cx| {
+            //     let user_settings = SettingsStore::global(cx).raw_user_settings();
+            //     if let Some(value) = read_settings_value_from_path(&user_settings, &path)
+            //         .and_then(serde_json::Value::as_str)
+            //         .map(str::to_string)
+            //     {
+            //         editor.set_text(value, window, cx);
+            //     }
+            //     // else {
+            //     //     editor.clear(window, cx);
+            //     // }
+            // })
+            // .detach();
 
-            editor.set_text(value.read().clone(), window, cx);
             editor
         }
     });
 
+    // todo! WAAY to slow
+    editor.update(cx, |editor, cx| {
+        if &editor.text(cx) != &current_text && !*dirty.read(cx) {
+            editor.set_text(current_text, window, cx);
+        }
+    });
+
     let weak_editor = editor.downgrade();
     let theme_colors = cx.theme().colors();
 
@@ -988,18 +1089,14 @@ fn render_toggle_button_group_inner(
 
 fn settings_value_from_settings_and_path(
     path: SmallVec<[SharedString; 1]>,
-    fallback_path: Option<&[SharedString]>,
+    fallback_value: Option<&serde_json::Value>,
     title: SharedString,
     documentation: Option<SharedString>,
     user_settings: &serde_json::Value,
     default_settings: &serde_json::Value,
 ) -> SettingsValue<serde_json::Value> {
     let default_value = read_settings_value_from_path(default_settings, &path)
-        .or_else(|| {
-            fallback_path.and_then(|fallback_path| {
-                read_settings_value_from_path(default_settings, fallback_path)
-            })
-        })
+        .or_else(|| fallback_value)
         .with_context(|| format!("No default value for item at path {:?}", path.join(".")))
         .expect("Default value set for item")
         .clone();

crates/settings_ui_macros/src/settings_ui_macros.rs

@@ -129,8 +129,8 @@ fn map_ui_item_to_entry(
     ty: TokenStream,
 ) -> TokenStream {
     // todo(settings_ui): does quote! just work with options?
-    let path = path.map_or_else(|| quote! {None}, |path| quote! {Some(#path)});
-    let doc_str = doc_str.map_or_else(|| quote! {None}, |doc_str| quote! {Some(#doc_str)});
+    let path = token_stream_from_option(path);
+    let doc_str = token_stream_from_option(doc_str);
     let item = ui_item_from_type(ty);
     quote! {
         settings::SettingsUiEntry {
@@ -154,14 +154,24 @@ fn trait_method_call(
 ) -> TokenStream {
     // doing the <ty as settings::SettingsUi> makes the error message better:
     //  -> "#ty Doesn't implement settings::SettingsUi" instead of "no item "settings_ui_item" for #ty"
-    // and ensures safety against name conflicts
-    //
-    // todo(settings_ui): Turn `Vec<T>` into `Vec::<T>` here as well
+    // it also ensures safety against name conflicts with the method name, and works for parameterized types such as `Vec<T>`,
+    // as the syntax for calling methods directly on parameterized types is `Vec::<T>::method_name()`,
+    // but `<Vec<T> as MyTrait>::method_name()` is valid so we don't have to worry about inserting the extra
+    // colons as appropriate
     quote! {
         <#ty as #trait_name>::#method_name()
     }
 }
 
+// the default impl for `Option<T: ToTokens>` in quote!{} is to include T if it is Some, and include nothing if it is None
+// This function actually results in a `Some(T)` if the option is Some, and a `None` if the option is None
+fn token_stream_from_option<T: ToTokens>(option: Option<T>) -> TokenStream {
+    match option {
+        Some(value) => quote! { Some(#value) },
+        None => quote! { None },
+    }
+}
+
 fn generate_ui_item_body(group_name: Option<&String>, input: &syn::DeriveInput) -> TokenStream {
     match (group_name, &input.data) {
         (_, Data::Union(_)) => unimplemented!("Derive SettingsUi for Unions"),
@@ -217,12 +227,13 @@ fn generate_ui_item_body(group_name: Option<&String>, input: &syn::DeriveInput)
                 }
                 let name = &variant.ident;
                 let item = item_group_from_fields(&variant.fields, &serde_attrs);
-                // todo(settings_ui): documentation
+                let documentation =
+                    token_stream_from_option(parse_documentation_from_attrs(&variant.attrs));
                 return quote! {
                     Some(settings::SettingsUiEntry {
                         path: None,
                         title: stringify!(#name),
-                        documentation: None,
+                        documentation: #documentation,
                         item: #item,
                     })
                 };
@@ -236,7 +247,8 @@ fn generate_ui_item_body(group_name: Option<&String>, input: &syn::DeriveInput)
                 } else {
                     let fields = variant.fields.iter().enumerate().map(|(index, field)| {
                         let field_name = field.ident.as_ref().map_or_else(|| syn::Index::from(index).into_token_stream(), |ident| ident.to_token_stream());
-                        let field_type_is_option = option_inner_type(field.ty.to_token_stream()).is_some();
+                        let field_type = &field.ty;
+                        let field_type_is_option = option_inner_type(field_type.to_token_stream()).is_some();
                         let field_default = if field_type_is_option {
                             quote! {
                                 None
@@ -290,9 +302,7 @@ fn generate_ui_item_body(group_name: Option<&String>, input: &syn::DeriveInput)
                     determine_option: #determine_option_fn,
                 })
             };
-            // panic!("Unhandled");
         }
-        // todo(settings_ui) discriminated unions
         (_, Data::Enum(_)) => quote! {
             settings::SettingsUiItem::None
         },
@@ -303,8 +313,8 @@ fn item_group_from_fields(fields: &syn::Fields, parent_serde_attrs: &SerdeOption
     let group_items = fields
         .iter()
         .filter(|field| {
-            !field.attrs.iter().any(|attr| {
-                let mut has_skip = false;
+            let mut has_skip = false;
+            for attr in &field.attrs {
                 if attr.path().is_ident("settings_ui") {
                     let _ = attr.parse_nested_meta(|meta| {
                         if meta.path.is_ident("skip") {
@@ -313,9 +323,9 @@ fn item_group_from_fields(fields: &syn::Fields, parent_serde_attrs: &SerdeOption
                         Ok(())
                     });
                 }
+            }
 
-                has_skip
-            })
+            !has_skip
         })
         .map(|field| {
             let field_serde_attrs = parse_serde_attributes(&field.attrs);
@@ -329,6 +339,7 @@ fn item_group_from_fields(fields: &syn::Fields, parent_serde_attrs: &SerdeOption
             (
                 title,
                 doc_str,
+                // todo(settings_ui): Have apply_rename_to_field take flatten into account
                 name.filter(|_| !field_serde_attrs.flatten).map(|name| {
                     parent_serde_attrs.apply_rename_to_field(&field_serde_attrs, &name)
                 }),
@@ -341,7 +352,7 @@ fn item_group_from_fields(fields: &syn::Fields, parent_serde_attrs: &SerdeOption
         });
 
     quote! {
-        settings::SettingsUiItem::Group(settings::SettingsUiItemGroup{ items: vec![#(#group_items),*] })
+        settings::SettingsUiItem::Group(settings::SettingsUiItemGroup{ items: Box::new([#(#group_items),*]) })
     }
 }
 
@@ -567,11 +578,8 @@ pub fn derive_settings_key(input: proc_macro::TokenStream) -> proc_macro::TokenS
         panic!("Missing #[settings_key] attribute");
     };
 
-    let key = key.map_or_else(|| quote! {None}, |key| quote! {Some(#key)});
-    let fallback_key = fallback_key.map_or_else(
-        || quote! {None},
-        |fallback_key| quote! {Some(#fallback_key)},
-    );
+    let key = token_stream_from_option(key);
+    let fallback_key = token_stream_from_option(fallback_key);
 
     let expanded = quote! {
         impl #impl_generics settings::SettingsKey for #name #ty_generics #where_clause {