Correctly parse backslash character on replacement (#37014)

hong jihwan created 2 months ago

When a keybind contains a backslash character (\\), it is parsed
incorrectly, which results in an invalid keybind configuration.


This patch fixes the issue by ensuring that backslashes are properly
escaped during the parsing process. This allows them to be used as
intended in keybind definitions.

Release Notes:

- Fixed an issue where keybinds containing a backslash character (\\)
failed to be replaced correctly


## Screenshots
<img width="912" height="530" alt="SCR-20250828-borp"
src="https://github.com/user-attachments/assets/561a040f-575b-4222-ac75-17ab4fa71d07"
/>
<img width="912" height="530" alt="SCR-20250828-bosx"
src="https://github.com/user-attachments/assets/b8e0fb99-549e-4fc9-8609-9b9aa2004656"
/>

Change summary

crates/settings/src/keymap_file.rs   | 122 ++++++++++++++++++++++++++++++
crates/settings/src/settings_json.rs |  10 +
2 files changed, 128 insertions(+), 4 deletions(-)

Detailed changes

crates/settings/src/keymap_file.rs

@@ -1100,6 +1100,24 @@ mod tests {
             .unindent(),
         );
 
+        check_keymap_update(
+            "[]",
+            KeybindUpdateOperation::add(KeybindUpdateTarget {
+                keystrokes: &parse_keystrokes("\\ a"),
+                action_name: "zed::SomeAction",
+                context: None,
+                action_arguments: None,
+            }),
+            r#"[
+                {
+                    "bindings": {
+                        "\\ a": "zed::SomeAction"
+                    }
+                }
+            ]"#
+            .unindent(),
+        );
+
         check_keymap_update(
             "[]",
             KeybindUpdateOperation::add(KeybindUpdateTarget {
@@ -1302,6 +1320,79 @@ mod tests {
             .unindent(),
         );
 
+        check_keymap_update(
+            r#"[
+                {
+                    "bindings": {
+                        "\\ a": "zed::SomeAction"
+                    }
+                }
+            ]"#
+            .unindent(),
+            KeybindUpdateOperation::Replace {
+                target: KeybindUpdateTarget {
+                    keystrokes: &parse_keystrokes("\\ a"),
+                    action_name: "zed::SomeAction",
+                    context: None,
+                    action_arguments: None,
+                },
+                source: KeybindUpdateTarget {
+                    keystrokes: &parse_keystrokes("\\ b"),
+                    action_name: "zed::SomeOtherAction",
+                    context: None,
+                    action_arguments: Some(r#"{"foo": "bar"}"#),
+                },
+                target_keybind_source: KeybindSource::User,
+            },
+            r#"[
+                {
+                    "bindings": {
+                        "\\ b": [
+                            "zed::SomeOtherAction",
+                            {
+                                "foo": "bar"
+                            }
+                        ]
+                    }
+                }
+            ]"#
+            .unindent(),
+        );
+
+        check_keymap_update(
+            r#"[
+                {
+                    "bindings": {
+                        "\\ a": "zed::SomeAction"
+                    }
+                }
+            ]"#
+            .unindent(),
+            KeybindUpdateOperation::Replace {
+                target: KeybindUpdateTarget {
+                    keystrokes: &parse_keystrokes("\\ a"),
+                    action_name: "zed::SomeAction",
+                    context: None,
+                    action_arguments: None,
+                },
+                source: KeybindUpdateTarget {
+                    keystrokes: &parse_keystrokes("\\ a"),
+                    action_name: "zed::SomeAction",
+                    context: None,
+                    action_arguments: None,
+                },
+                target_keybind_source: KeybindSource::User,
+            },
+            r#"[
+                {
+                    "bindings": {
+                        "\\ a": "zed::SomeAction"
+                    }
+                }
+            ]"#
+            .unindent(),
+        );
+
         check_keymap_update(
             r#"[
                 {
@@ -1494,6 +1585,37 @@ mod tests {
             .unindent(),
         );
 
+        check_keymap_update(
+            r#"[
+                {
+                    "context": "SomeContext",
+                    "bindings": {
+                        "\\ a": "foo::bar",
+                        "c": "foo::baz",
+                    }
+                },
+            ]"#
+            .unindent(),
+            KeybindUpdateOperation::Remove {
+                target: KeybindUpdateTarget {
+                    context: Some("SomeContext"),
+                    keystrokes: &parse_keystrokes("\\ a"),
+                    action_name: "foo::bar",
+                    action_arguments: None,
+                },
+                target_keybind_source: KeybindSource::User,
+            },
+            r#"[
+                {
+                    "context": "SomeContext",
+                    "bindings": {
+                        "c": "foo::baz",
+                    }
+                },
+            ]"#
+            .unindent(),
+        );
+
         check_keymap_update(
             r#"[
                 {

crates/settings/src/settings_json.rs

@@ -140,8 +140,10 @@ pub fn replace_value_in_json_text<T: AsRef<str>>(
 
         let found_key = text
             .get(key_range.clone())
-            .map(|key_text| {
-                depth < key_path.len() && key_text == format!("\"{}\"", key_path[depth].as_ref())
+            .and_then(|key_text| {
+                serde_json::to_string(key_path[depth].as_ref())
+                    .ok()
+                    .map(|key_path| depth < key_path.len() && key_text == key_path)
             })
             .unwrap_or(false);
 
@@ -163,8 +165,8 @@ pub fn replace_value_in_json_text<T: AsRef<str>>(
     if depth == key_path.len() {
         if let Some(new_value) = new_value {
             let new_val = to_pretty_json(new_value, tab_size, tab_size * depth);
-            if let Some(replace_key) = replace_key {
-                let new_key = format!("\"{}\": ", replace_key);
+            if let Some(replace_key) = replace_key.and_then(|str| serde_json::to_string(str).ok()) {
+                let new_key = format!("{}: ", replace_key);
                 if let Some(key_start) = text[..existing_value_range.start].rfind('"') {
                     if let Some(prev_key_start) = text[..key_start].rfind('"') {
                         existing_value_range.start = prev_key_start;