settings ui: Fix panic from reading `BufferLineHeight` custom variant (#39631)

Anthony Eid created 2 months ago

The panic happened when a user had a settings file with a buffer line
height custom variant, because the drop-down renderer only took into
account the two named variants.

The fix for this will be creating a custom element that allows a user to
manually input a line height greater than one or select either
Comfortable or Standard.

Release Notes:

- N/A

Change summary

crates/settings/src/settings_content/theme.rs | 17 +----------------
crates/settings_ui/src/page_data.rs           | 19 ++++++++++---------
crates/settings_ui/src/settings_ui.rs         | 11 ++++++-----
3 files changed, 17 insertions(+), 30 deletions(-)

Detailed changes

crates/settings/src/settings_content/theme.rs

@@ -262,18 +262,7 @@ impl From<FontFamilyName> for String {
 }
 
 /// The buffer's line height.
-#[derive(
-    Clone,
-    Copy,
-    Debug,
-    Serialize,
-    Deserialize,
-    PartialEq,
-    JsonSchema,
-    MergeFrom,
-    Default,
-    strum::VariantNames,
-)]
+#[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq, JsonSchema, MergeFrom, Default)]
 #[serde(rename_all = "snake_case")]
 pub enum BufferLineHeight {
     /// A less dense line height.
@@ -285,10 +274,6 @@ pub enum BufferLineHeight {
     Custom(#[serde(deserialize_with = "deserialize_line_height")] f32),
 }
 
-impl strum::VariantArray for BufferLineHeight {
-    const VARIANTS: &'static [Self] = &[Self::Comfortable, Self::Standard];
-}
-
 fn deserialize_line_height<'de, D>(deserializer: D) -> Result<f32, D::Error>
 where
     D: serde::Deserializer<'de>,

crates/settings_ui/src/page_data.rs

@@ -250,15 +250,16 @@ pub(crate) fn user_settings_data() -> Vec<SettingsPage> {
                     }),
                     metadata: None,
                 }),
-                SettingsPageItem::SettingItem(SettingItem {
-                    title: "Buffer Line Height",
-                    description: "Line height for editor text",
-                    field: Box::new(SettingField {
-                        pick: |settings_content| &settings_content.theme.buffer_line_height,
-                        pick_mut: |settings_content| &mut settings_content.theme.buffer_line_height,
-                    }),
-                    metadata: None,
-                }),
+                // todo(settings_ui): This needs custom ui
+                // SettingsPageItem::SettingItem(SettingItem {
+                //     title: "Buffer Line Height",
+                //     description: "Line height for editor text",
+                //     field: Box::new(SettingField {
+                //         pick: |settings_content| &settings_content.theme.buffer_line_height,
+                //         pick_mut: |settings_content| &mut settings_content.theme.buffer_line_height,
+                //     }),
+                //     metadata: None,
+                // }),
                 SettingsPageItem::SettingItem(SettingItem {
                     title: "UI Font Family",
                     description: "Font family for UI elements",

crates/settings_ui/src/settings_ui.rs

@@ -239,11 +239,12 @@ fn init_renderers(cx: &mut App) {
             // todo(settings_ui): We need to pass in a validator for this to ensure that users that type in invalid font names
             render_font_picker(settings_field.clone(), file, window, cx)
         })
-        .add_renderer::<settings::BufferLineHeight>(|settings_field, file, _, window, cx| {
-            // todo(settings_ui): Do we want to expose the custom variant of buffer line height?
-            // right now there's a manual impl of strum::VariantArray
-            render_dropdown(*settings_field, file, window, cx)
-        })
+        // todo(settings_ui): This needs custom ui
+        // .add_renderer::<settings::BufferLineHeight>(|settings_field, file, _, window, cx| {
+        //     // todo(settings_ui): Do we want to expose the custom variant of buffer line height?
+        //     // right now there's a manual impl of strum::VariantArray
+        //     render_dropdown(*settings_field, file, window, cx)
+        // })
         .add_renderer::<settings::BaseKeymapContent>(|settings_field, file, _, window, cx| {
             render_dropdown(*settings_field, file, window, cx)
         })