Add :delm[arks] {marks} command to delete vim marks (#31140)

Release Notes:

- Implements `:delm[arks] {marks}` specified
[here](https://vimhelp.org/motion.txt.html#%3Adelmarks)
- Adds `ArgumentRequired` action for vim commands that require arguments

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>

crates/vim/src/command.rs

@@ -1,5 +1,5 @@
 use anyhow::Result;
-use collections::HashMap;
+use collections::{HashMap, HashSet};
 use command_palette_hooks::CommandInterceptResult;
 use editor::{
     Bias, Editor, ToPoint,
@@ -166,12 +166,21 @@ struct VimSave {
     pub filename: String,
 }
 
+#[derive(Clone, Deserialize, JsonSchema, PartialEq)]
+enum DeleteMarks {
+    Marks(String),
+    AllLocal,
+}
+
+actions!(
+    vim,
+    [VisualCommand, CountCommand, ShellCommand, ArgumentRequired]
+);
 #[derive(Clone, Deserialize, JsonSchema, PartialEq)]
 struct VimEdit {
     pub filename: String,
 }
 
-actions!(vim, [VisualCommand, CountCommand, ShellCommand]);
 impl_internal_actions!(
     vim,
     [
@@ -183,6 +192,7 @@ impl_internal_actions!(
         ShellExec,
         VimSet,
         VimSave,
+        DeleteMarks,
         VimEdit,
     ]
 );
@@ -245,6 +255,25 @@ pub fn register(editor: &mut Editor, cx: &mut Context<Vim>) {
         })
     });
 
+    Vim::action(editor, cx, |_, _: &ArgumentRequired, window, cx| {
+        let _ = window.prompt(
+            gpui::PromptLevel::Critical,
+            "Argument required",
+            None,
+            &["Cancel"],
+            cx,
+        );
+    });
+
+    Vim::action(editor, cx, |vim, _: &ShellCommand, window, cx| {
+        let Some(workspace) = vim.workspace(window) else {
+            return;
+        };
+        workspace.update(cx, |workspace, cx| {
+            command_palette::CommandPalette::toggle(workspace, "'<,'>!", window, cx);
+        })
+    });
+
     Vim::action(editor, cx, |vim, action: &VimSave, window, cx| {
         vim.update_editor(window, cx, |_, editor, window, cx| {
             let Some(project) = editor.project.clone() else {
@@ -286,6 +315,72 @@ pub fn register(editor: &mut Editor, cx: &mut Context<Vim>) {
         });
     });
 
+    Vim::action(editor, cx, |vim, action: &DeleteMarks, window, cx| {
+        fn err(s: String, window: &mut Window, cx: &mut Context<Editor>) {
+            let _ = window.prompt(
+                gpui::PromptLevel::Critical,
+                &format!("Invalid argument: {}", s),
+                None,
+                &["Cancel"],
+                cx,
+            );
+        }
+        vim.update_editor(window, cx, |vim, editor, window, cx| match action {
+            DeleteMarks::Marks(s) => {
+                if s.starts_with('-') || s.ends_with('-') || s.contains(['\'', '`']) {
+                    err(s.clone(), window, cx);
+                    return;
+                }
+
+                let to_delete = if s.len() < 3 {
+                    Some(s.clone())
+                } else {
+                    s.chars()
+                        .tuple_windows::<(_, _, _)>()
+                        .map(|(a, b, c)| {
+                            if b == '-' {
+                                if match a {
+                                    'a'..='z' => a <= c && c <= 'z',
+                                    'A'..='Z' => a <= c && c <= 'Z',
+                                    '0'..='9' => a <= c && c <= '9',
+                                    _ => false,
+                                } {
+                                    Some((a..=c).collect_vec())
+                                } else {
+                                    None
+                                }
+                            } else if a == '-' {
+                                if c == '-' { None } else { Some(vec![c]) }
+                            } else if c == '-' {
+                                if a == '-' { None } else { Some(vec![a]) }
+                            } else {
+                                Some(vec![a, b, c])
+                            }
+                        })
+                        .fold_options(HashSet::<char>::default(), |mut set, chars| {
+                            set.extend(chars.iter().copied());
+                            set
+                        })
+                        .map(|set| set.iter().collect::<String>())
+                };
+
+                let Some(to_delete) = to_delete else {
+                    err(s.clone(), window, cx);
+                    return;
+                };
+
+                for c in to_delete.chars().filter(|c| !c.is_whitespace()) {
+                    vim.delete_mark(c.to_string(), editor, window, cx);
+                }
+            }
+            DeleteMarks::AllLocal => {
+                for s in 'a'..='z' {
+                    vim.delete_mark(s.to_string(), editor, window, cx);
+                }
+            }
+        });
+    });
+
     Vim::action(editor, cx, |vim, action: &VimEdit, window, cx| {
         vim.update_editor(window, cx, |vim, editor, window, cx| {
             let Some(workspace) = vim.workspace(window) else {
@@ -982,6 +1077,9 @@ fn generate_commands(_: &App) -> Vec<VimCommand> {
         }),
         VimCommand::new(("reg", "isters"), ToggleRegistersView).bang(ToggleRegistersView),
         VimCommand::new(("marks", ""), ToggleMarksView).bang(ToggleMarksView),
+        VimCommand::new(("delm", "arks"), ArgumentRequired)
+            .bang(DeleteMarks::AllLocal)
+            .args(|_, args| Some(DeleteMarks::Marks(args).boxed_clone())),
         VimCommand::new(("sor", "t"), SortLinesCaseSensitive).range(select_range),
         VimCommand::new(("sort i", ""), SortLinesCaseInsensitive).range(select_range),
         VimCommand::str(("E", "xplore"), "project_panel::ToggleFocus"),
@@ -1732,6 +1830,7 @@ mod test {
     use std::path::Path;
 
     use crate::{
+        VimAddon,
         state::Mode,
         test::{NeovimBackedTestContext, VimTestContext},
     };
@@ -2084,4 +2183,35 @@ mod test {
             a
             ˇa"});
     }
+
+    #[gpui::test]
+    async fn test_del_marks(cx: &mut TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        cx.set_shared_state(indoc! {"
+            ˇa
+            b
+            a
+            b
+            a
+        "})
+            .await;
+
+        cx.simulate_shared_keystrokes("m a").await;
+
+        let mark = cx.update_editor(|editor, window, cx| {
+            let vim = editor.addon::<VimAddon>().unwrap().entity.clone();
+            vim.update(cx, |vim, cx| vim.get_mark("a", editor, window, cx))
+        });
+        assert!(mark.is_some());
+
+        cx.simulate_shared_keystrokes(": d e l m space a").await;
+        cx.simulate_shared_keystrokes("enter").await;
+
+        let mark = cx.update_editor(|editor, window, cx| {
+            let vim = editor.addon::<VimAddon>().unwrap().entity.clone();
+            vim.update(cx, |vim, cx| vim.get_mark("a", editor, window, cx))
+        });
+        assert!(mark.is_none())
+    }
 }

crates/vim/src/normal/mark.rs

@@ -279,6 +279,10 @@ impl Vim {
         if name == "`" {
             name = "'".to_string();
         }
+        if matches!(&name[..], "-" | " ") {
+            // Not allowed marks
+            return;
+        }
         let entity_id = workspace.entity_id();
         Vim::update_globals(cx, |vim_globals, cx| {
             let Some(marks_state) = vim_globals.marks.get(&entity_id) else {
@@ -326,6 +330,30 @@ impl Vim {
                 .update(cx, |ms, cx| ms.get_mark(name, editor.buffer(), cx))
         })
     }
+
+    pub fn delete_mark(
+        &self,
+        name: String,
+        editor: &mut Editor,
+        window: &mut Window,
+        cx: &mut App,
+    ) {
+        let Some(workspace) = self.workspace(window) else {
+            return;
+        };
+        if name == "`" || name == "'" {
+            return;
+        }
+        let entity_id = workspace.entity_id();
+        Vim::update_globals(cx, |vim_globals, cx| {
+            let Some(marks_state) = vim_globals.marks.get(&entity_id) else {
+                return;
+            };
+            marks_state.update(cx, |ms, cx| {
+                ms.delete_mark(name.clone(), editor.buffer(), cx);
+            });
+        });
+    }
 }
 
 pub fn jump_motion(

crates/vim/src/state.rs

@@ -557,7 +557,9 @@ impl MarksState {
             }
             return;
         };
-        let buffer = buffer.unwrap();
+        let Some(buffer) = buffer else {
+            return;
+        };
 
         let buffer_id = buffer.read(cx).remote_id();
         self.buffer_marks.entry(buffer_id).or_default().insert(
@@ -588,7 +590,7 @@ impl MarksState {
             }
 
             let singleton = multi_buffer.read(cx).as_singleton()?;
-            let excerpt_id = *multi_buffer.read(cx).excerpt_ids().first().unwrap();
+            let excerpt_id = *multi_buffer.read(cx).excerpt_ids().first()?;
             let buffer_id = singleton.read(cx).remote_id();
             if let Some(anchors) = self.buffer_marks.get(&buffer_id) {
                 let text_anchors = anchors.get(name)?;
@@ -611,6 +613,60 @@ impl MarksState {
             }
         }
     }
+    pub fn delete_mark(
+        &mut self,
+        mark_name: String,
+        multi_buffer: &Entity<MultiBuffer>,
+        cx: &mut Context<Self>,
+    ) {
+        let path = if let Some(target) = self.global_marks.get(&mark_name.clone()) {
+            let name = mark_name.clone();
+            if let Some(workspace_id) = self.workspace_id(cx) {
+                cx.background_spawn(async move {
+                    DB.delete_global_marks_path(workspace_id, name).await
+                })
+                .detach_and_log_err(cx);
+            }
+            self.buffer_marks.iter_mut().for_each(|(_, m)| {
+                m.remove(&mark_name.clone());
+            });
+
+            match target {
+                MarkLocation::Buffer(entity_id) => {
+                    self.multibuffer_marks
+                        .get_mut(&entity_id)
+                        .map(|m| m.remove(&mark_name.clone()));
+                    return;
+                }
+                MarkLocation::Path(path) => path.clone(),
+            }
+        } else {
+            self.multibuffer_marks
+                .get_mut(&multi_buffer.entity_id())
+                .map(|m| m.remove(&mark_name.clone()));
+
+            if let Some(singleton) = multi_buffer.read(cx).as_singleton() {
+                let buffer_id = singleton.read(cx).remote_id();
+                self.buffer_marks
+                    .get_mut(&buffer_id)
+                    .map(|m| m.remove(&mark_name.clone()));
+                let Some(path) = self.path_for_buffer(&singleton, cx) else {
+                    return;
+                };
+                path
+            } else {
+                return;
+            }
+        };
+        self.global_marks.remove(&mark_name.clone());
+        self.serialized_marks
+            .get_mut(&path.clone())
+            .map(|m| m.remove(&mark_name.clone()));
+        if let Some(workspace_id) = self.workspace_id(cx) {
+            cx.background_spawn(async move { DB.delete_mark(workspace_id, path, mark_name).await })
+                .detach_and_log_err(cx);
+        }
+    }
 }
 
 impl Global for VimGlobals {}
@@ -1689,6 +1745,21 @@ impl VimDb {
             .collect())
     }
 
+    pub(crate) async fn delete_mark(
+        &self,
+        workspace_id: WorkspaceId,
+        path: Arc<Path>,
+        mark_name: String,
+    ) -> Result<()> {
+        self.write(move |conn| {
+            conn.exec_bound(sql!(
+                DELETE FROM vim_marks
+                WHERE workspace_id = ? AND mark_name = ? AND path = ?
+            ))?((workspace_id, mark_name, path))
+        })
+        .await
+    }
+
     pub(crate) async fn set_global_mark_path(
         &self,
         workspace_id: WorkspaceId,
@@ -1716,4 +1787,18 @@ impl VimDb {
             WHERE workspace_id = ?
         ))?(workspace_id)
     }
+
+    pub(crate) async fn delete_global_marks_path(
+        &self,
+        workspace_id: WorkspaceId,
+        mark_name: String,
+    ) -> Result<()> {
+        self.write(move |conn| {
+            conn.exec_bound(sql!(
+                DELETE FROM vim_global_marks_paths
+                WHERE workspace_id = ? AND mark_name = ?
+            ))?((workspace_id, mark_name))
+        })
+        .await
+    }
 }

crates/vim/test_data/test_del_marks.json

@@ -0,0 +1,11 @@
+{"Put":{"state":"ˇa\nb\na\nb\na\n"}}
+{"Key":"m"}
+{"Key":"a"}
+{"Key":":"}
+{"Key":"d"}
+{"Key":"e"}
+{"Key":"l"}
+{"Key":"m"}
+{"Key":"space"}
+{"Key":"a"}
+{"Key":"enter"}