vim: Fix global mark overwriting inconsistency (#44765)

AidanV created 1 week ago

Closes #43963

This issue was caused by the global marks not being deleted. Previously
marking the first file `m A`

<img width="1736" height="888" alt="Screenshot From 2025-12-13 01-37-55"
src="https://github.com/user-attachments/assets/9e46747f-7bb3-4297-82d4-44a20ef9e91a"
/>

followed by marking the second file `m A`

<img width="1736" height="888" alt="Screenshot From 2025-12-13 01-37-42"
src="https://github.com/user-attachments/assets/0d126b47-2c42-475f-826a-173c0d5a1156"
/>

and navigating back to the first file

<img width="1736" height="888" alt="Screenshot From 2025-12-13 01-37-30"
src="https://github.com/user-attachments/assets/032fd0bd-ff71-4a12-987a-7f1743016f6d"
/>

shows that the mark still exists and was not properly deleted. After
these changes the global mark in the original file is correctly
overwritten.

Added regression test for this.

Release Notes:

- Fixed bug where overwriting global Vim marks was inconsistent

Change summary

crates/vim/src/normal/mark.rs | 72 +++++++++++++++++++++++++++++++++++-
crates/vim/src/state.rs       | 10 ++++
2 files changed, 79 insertions(+), 3 deletions(-)

Detailed changes

crates/vim/src/normal/mark.rs

@@ -372,9 +372,12 @@ pub fn jump_motion(
 
 #[cfg(test)]
 mod test {
+    use crate::test::{NeovimBackedTestContext, VimTestContext};
+    use editor::Editor;
     use gpui::TestAppContext;
-
-    use crate::test::NeovimBackedTestContext;
+    use std::path::Path;
+    use util::path;
+    use workspace::{CloseActiveItem, OpenOptions};
 
     #[gpui::test]
     async fn test_quote_mark(cx: &mut TestAppContext) {
@@ -394,4 +397,69 @@ mod test {
         cx.simulate_shared_keystrokes("^ ` `").await;
         cx.shared_state().await.assert_eq("Hello, worldˇ!");
     }
+
+    #[gpui::test]
+    async fn test_global_mark_overwrite(cx: &mut TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+
+        let path = Path::new(path!("/first.rs"));
+        let fs = cx.workspace(|workspace, _, cx| workspace.project().read(cx).fs().clone());
+        fs.as_fake().insert_file(path, "one".into()).await;
+        let path = Path::new(path!("/second.rs"));
+        fs.as_fake().insert_file(path, "two".into()).await;
+
+        let _ = cx
+            .workspace(|workspace, window, cx| {
+                workspace.open_abs_path(
+                    path!("/first.rs").into(),
+                    OpenOptions::default(),
+                    window,
+                    cx,
+                )
+            })
+            .await;
+
+        cx.simulate_keystrokes("m A");
+
+        let _ = cx
+            .workspace(|workspace, window, cx| {
+                workspace.open_abs_path(
+                    path!("/second.rs").into(),
+                    OpenOptions::default(),
+                    window,
+                    cx,
+                )
+            })
+            .await;
+
+        cx.simulate_keystrokes("m A");
+
+        let _ = cx
+            .workspace(|workspace, window, cx| {
+                workspace.active_pane().update(cx, |pane, cx| {
+                    pane.close_active_item(&CloseActiveItem::default(), window, cx)
+                })
+            })
+            .await;
+
+        cx.simulate_keystrokes("m B");
+
+        cx.simulate_keystrokes("' A");
+
+        cx.workspace(|workspace, _, cx| {
+            let active_editor = workspace.active_item_as::<Editor>(cx).unwrap();
+
+            let buffer = active_editor
+                .read(cx)
+                .buffer()
+                .read(cx)
+                .as_singleton()
+                .unwrap();
+
+            let file = buffer.read(cx).file().unwrap();
+            let file_path = file.as_local().unwrap().abs_path(cx);
+
+            assert_eq!(file_path.to_str().unwrap(), path!("/second.rs"));
+        })
+    }
 }

crates/vim/src/state.rs

@@ -550,6 +550,10 @@ impl MarksState {
         let buffer = multibuffer.read(cx).as_singleton();
         let abs_path = buffer.as_ref().and_then(|b| self.path_for_buffer(b, cx));
 
+        if self.is_global_mark(&name) && self.global_marks.contains_key(&name) {
+            self.delete_mark(name.clone(), multibuffer, cx);
+        }
+
         let Some(abs_path) = abs_path else {
             self.multibuffer_marks
                 .entry(multibuffer.entity_id())
@@ -573,7 +577,7 @@ impl MarksState {
 
         let buffer_id = buffer.read(cx).remote_id();
         self.buffer_marks.entry(buffer_id).or_default().insert(
-            name,
+            name.clone(),
             anchors
                 .into_iter()
                 .map(|anchor| anchor.text_anchor)
@@ -582,6 +586,10 @@ impl MarksState {
         if !self.watched_buffers.contains_key(&buffer_id) {
             self.watch_buffer(MarkLocation::Path(abs_path.clone()), &buffer, cx)
         }
+        if self.is_global_mark(&name) {
+            self.global_marks
+                .insert(name, MarkLocation::Path(abs_path.clone()));
+        }
         self.serialize_buffer_marks(abs_path, &buffer, cx)
     }