workspace: workspace forget multiple names at once

Summary: This allows `workspace forget` to forget multiple workspaces in a
single action; it now behaves more consistently with other verbs like `abandon`
which can take multiple revisions at one time.

There's some hoop-jumping involved to ensure the oplog transaction description
looks nice, but as they say: small conveniences cost a lot.

Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Id91da269f87b145010c870b7dc043748

CHANGELOG.md

@@ -28,6 +28,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * `jj workspace add` now takes a `--revision` argument.
 
+* `jj workspace forget` can now forget multiple workspaces at once.
+
 ### Fixed bugs
 
 * Updating the working copy to a commit where a file that's currently ignored

cli/src/commands/mod.rs

@@ -1063,8 +1063,9 @@ struct WorkspaceAddArgs {
 /// before or after running this command.
 #[derive(clap::Args, Clone, Debug)]
 struct WorkspaceForgetArgs {
-    /// Name of the workspace to forget (the current workspace by default)
+    /// Names of the workspaces to forget. By default, forgets only the current
-    workspace: Option<String>,
+    /// workspace.
+    workspaces: Vec<String>,
 }
 
 /// List workspaces
@@ -3812,24 +3813,47 @@ fn cmd_workspace_forget(
     args: &WorkspaceForgetArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
+    let len = args.workspaces.len();
 
-    let workspace_id = if let Some(workspace_str) = &args.workspace {
+    let mut wss = Vec::new();
-        WorkspaceId::new(workspace_str.to_string())
+    let description = match len {
-    } else {
+        // NOTE (aseipp): if there's only 1-or-0 arguments, shortcut. this is
-        workspace_command.workspace_id().to_owned()
+        // mostly so the oplog description can look good: it removes the need,
+        // in the case of more-than-1 argument, to handle pluralization of the
+        // nouns in the description
+        0 | 1 => {
+            let ws = match len == 0 {
+                true => workspace_command.workspace_id().to_owned(),
+                false => WorkspaceId::new(args.workspaces[0].to_string()),
+            };
+            wss.push(ws.clone());
+            format!("forget workspace {}", ws.as_str())
+        }
+        _ => {
+            args.workspaces
+                .iter()
+                .map(|ws| WorkspaceId::new(ws.to_string()))
+                .for_each(|ws| wss.push(ws));
+
+            format!("forget workspaces {}", args.workspaces.join(", "))
+        }
     };
-    if workspace_command
+
-        .repo()
+    for ws in &wss {
-        .view()
+        if workspace_command
-        .get_wc_commit_id(&workspace_id)
+            .repo()
-        .is_none()
+            .view()
-    {
+            .get_wc_commit_id(ws)
-        return Err(user_error("No such workspace"));
+            .is_none()
+        {
+            return Err(user_error(format!("No such workspace: {}", ws.as_str())));
+        }
     }
 
-    let mut tx =
+    // bundle every workspace forget into a single transaction, so that e.g.
-        workspace_command.start_transaction(&format!("forget workspace {}", workspace_id.as_str()));
+    // undo correctly restores all of them at once.
-    tx.mut_repo().remove_wc_commit(&workspace_id);
+    let mut tx = workspace_command.start_transaction(&description);
+    wss.iter().for_each(|ws| tx.mut_repo().remove_wc_commit(ws));
     tx.finish(ui)?;
     Ok(())
 }

cli/tests/test_workspaces.rs

@@ -400,8 +400,11 @@ fn test_workspaces_forget() {
     Error: Workspace already exists
     "###);
 
-    // Forget the secondary workspace
+    // Add a third workspace...
-    let (stdout, stderr) = test_env.jj_cmd_ok(&main_path, &["workspace", "forget", "secondary"]);
+    test_env.jj_cmd_ok(&main_path, &["workspace", "add", "../third"]);
+    // ... and then forget it, and the secondary workspace too
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&main_path, &["workspace", "forget", "secondary", "third"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @"");
     // No workspaces left
@@ -409,6 +412,53 @@ fn test_workspaces_forget() {
     insta::assert_snapshot!(stdout, @"");
 }
 
+#[test]
+fn test_workspaces_forget_multi_transaction() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["init", "--git", "main"]);
+    let main_path = test_env.env_root().join("main");
+
+    std::fs::write(main_path.join("file"), "contents").unwrap();
+    test_env.jj_cmd_ok(&main_path, &["new"]);
+
+    test_env.jj_cmd_ok(&main_path, &["workspace", "add", "../second"]);
+    test_env.jj_cmd_ok(&main_path, &["workspace", "add", "../third"]);
+
+    // there should be three workspaces
+    let stdout = test_env.jj_cmd_success(&main_path, &["workspace", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    default: rlvkpnrz e949be04 (empty) (no description set)
+    second: pmmvwywv feda1c4e (empty) (no description set)
+    third: rzvqmyuk 485853ed (empty) (no description set)
+    "###);
+
+    // delete two at once, in a single tx
+    test_env.jj_cmd_ok(&main_path, &["workspace", "forget", "second", "third"]);
+    let stdout = test_env.jj_cmd_success(&main_path, &["workspace", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    default: rlvkpnrz e949be04 (empty) (no description set)
+    "###);
+
+    // the op log should have multiple workspaces forgotten in a single tx
+    let stdout = test_env.jj_cmd_success(&main_path, &["op", "log", "--limit", "1"]);
+    insta::assert_snapshot!(stdout, @r###"
+    @  e18f223ba3d3 test-username@host.example.com 2001-02-03 04:05:12.000 +07:00 - 2001-02-03 04:05:12.000 +07:00
+    │  forget workspaces second, third
+    │  args: jj workspace forget second third
+    "###);
+
+    // now, undo, and that should restore both workspaces
+    test_env.jj_cmd_ok(&main_path, &["op", "undo"]);
+
+    // finally, there should be three workspaces at the end
+    let stdout = test_env.jj_cmd_success(&main_path, &["workspace", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    default: rlvkpnrz e949be04 (empty) (no description set)
+    second: pmmvwywv feda1c4e (empty) (no description set)
+    third: rzvqmyuk 485853ed (empty) (no description set)
+    "###);
+}
+
 /// Test context of commit summary template
 #[test]
 fn test_list_workspaces_template() {