keymap ui: Fix remove key mapping bug (cherry-pick #34683) (#34730)

gcp-cherry-pick-bot[bot] , Anthony Eid , and Ben Kunkle created 5 months ago

Cherry-picked keymap ui: Fix remove key mapping bug (#34683)

Release Notes:

- N/A

---------

Co-authored-by: Ben Kunkle <[e-mail]>

Co-authored-by: Anthony Eid <[e-mail]>
Co-authored-by: Ben Kunkle <[e-mail]>

Change summary

crates/settings/src/keymap_file.rs   | 40 ++++++++++++++++++++++++++++++
crates/settings/src/settings_json.rs |  7 +++-
2 files changed, 45 insertions(+), 2 deletions(-)

Detailed changes

crates/settings/src/keymap_file.rs

@@ -1623,4 +1623,44 @@ mod tests {
             .unindent(),
         );
     }
+
+    #[test]
+    fn test_keymap_remove() {
+        zlog::init_test();
+
+        check_keymap_update(
+            r#"
+            [
+              {
+                "context": "Editor",
+                "bindings": {
+                  "cmd-k cmd-u": "editor::ConvertToUpperCase",
+                  "cmd-k cmd-l": "editor::ConvertToLowerCase",
+                  "cmd-[": "pane::GoBack",
+                }
+              },
+            ]
+            "#,
+            KeybindUpdateOperation::Remove {
+                target: KeybindUpdateTarget {
+                    context: Some("Editor"),
+                    keystrokes: &parse_keystrokes("cmd-k cmd-l"),
+                    action_name: "editor::ConvertToLowerCase",
+                    action_arguments: None,
+                },
+                target_keybind_source: KeybindSource::User,
+            },
+            r#"
+            [
+              {
+                "context": "Editor",
+                "bindings": {
+                  "cmd-k cmd-u": "editor::ConvertToUpperCase",
+                  "cmd-[": "pane::GoBack",
+                }
+              },
+            ]
+            "#,
+        );
+    }
 }

crates/settings/src/settings_json.rs

@@ -190,6 +190,7 @@ fn replace_value_in_json_text(
                 }
             }
 
+            let mut removed_comma = false;
             // Look backward for a preceding comma first
             let preceding_text = text.get(0..removal_start).unwrap_or("");
             if let Some(comma_pos) = preceding_text.rfind(',') {
@@ -197,10 +198,12 @@ fn replace_value_in_json_text(
                 let between_comma_and_key = text.get(comma_pos + 1..removal_start).unwrap_or("");
                 if between_comma_and_key.trim().is_empty() {
                     removal_start = comma_pos;
+                    removed_comma = true;
                 }
             }
-
-            if let Some(remaining_text) = text.get(existing_value_range.end..) {
+            if let Some(remaining_text) = text.get(existing_value_range.end..)
+                && !removed_comma
+            {
                 let mut chars = remaining_text.char_indices();
                 while let Some((offset, ch)) = chars.next() {
                     if ch == ',' {