worktree: remove --keep option from remove subcmd

Summary:
I previously added this as I found it useful, but it's annoying to code around (see D99632807) and it looks like I am the only user besides AI agents using it incorrectly https://fburl.com/scuba/dev_command_timers/n0ltcql9

We can add something back if folks ask for it, perhaps even just as its own subcommand so its not used incorrectly by agents

Reviewed By: muirdm

Differential Revision: D100056485

fbshipit-source-id: f438ce47eeb52519db915221428d519da44309f7

Author: genevievehelsel

eden/scm/lib/commands/commands/cmdworktree/src/lib.rs

@@ -30,9 +30,6 @@ define_flags! {
         /// create a snapshot of the current working copy, then restore it in the new worktree (for 'add')
         snapshot: bool,
 
-        /// only unlink from group, keep the checkout on disk (for 'remove')
-        keep: bool,
-
         /// remove all linked worktrees (for 'remove')
         all: bool,
 
@@ -102,7 +99,7 @@ pub fn doc() -> &'static str {
 
       list [-Tjson]                            List all worktrees in the group
       add [PATH] [--label TEXT] [--snapshot]   Create a new linked worktree
-      remove PATH [--all] [--keep] [-y]        Remove linked worktree(s)
+      remove PATH [--all] [-y]                 Remove linked worktree(s)
       label [PATH] TEXT [--remove]             Set or remove a worktree label
 
     If PATH is omitted from `add`, `worktree.path-generator` is used to

eden/scm/lib/commands/commands/cmdworktree/src/remove.rs

@@ -72,22 +72,13 @@ pub(crate) fn run(ctx: &ReqCtx<WorktreeOpts>, repo: &Repo) -> Result<u8> {
         pre_hooks.run_hooks(
             Some(repo),
             true,
-            Some(&HashMap::from([
-                ("path".to_string(), target.display().to_string()),
-                (
-                    "keep".to_string(),
-                    if ctx.opts.keep {
-                        "true".to_string()
-                    } else {
-                        String::new()
-                    },
-                ),
-            ])),
+            Some(&HashMap::from([(
+                "path".to_string(),
+                target.display().to_string(),
+            )])),
         )?;
 
-        if !ctx.opts.keep {
-            edenfs_client::run_eden_remove(repo.config().as_ref(), &target)?;
-        }
+        edenfs_client::run_eden_remove(repo.config().as_ref(), &target)?;
 
         Ok(())
     })?;
@@ -108,8 +99,7 @@ pub(crate) fn run(ctx: &ReqCtx<WorktreeOpts>, repo: &Repo) -> Result<u8> {
         Ok(())
     })?;
 
-    let action = if ctx.opts.keep { "unlinked" } else { "removed" };
-    logger.info(format!("{} {}", action, target.display()));
+    logger.info(format!("removed {}", target.display()));
 
     // NOTE: Add post-worktree-remove hook if the need arises. Note that the
     // hook's cwd (repo.path()) may not exist if the user removed the worktree
@@ -149,24 +139,15 @@ fn run_remove_all(
             pre_hooks.run_hooks(
                 Some(repo),
                 true,
-                Some(&HashMap::from([
-                    ("path".to_string(), path.display().to_string()),
-                    (
-                        "keep".to_string(),
-                        if ctx.opts.keep {
-                            "true".to_string()
-                        } else {
-                            String::new()
-                        },
-                    ),
-                ])),
+                Some(&HashMap::from([(
+                    "path".to_string(),
+                    path.display().to_string(),
+                )])),
             )?;
         }
 
         for path in &linked_paths {
-            if !ctx.opts.keep {
-                edenfs_client::run_eden_remove(repo.config().as_ref(), path)?;
-            }
+            edenfs_client::run_eden_remove(repo.config().as_ref(), path)?;
             grp.worktrees.remove(path);
         }
 
@@ -177,9 +158,8 @@ fn run_remove_all(
         logger.info("no linked worktrees to remove");
         return Ok(0);
     }
-    let action = if ctx.opts.keep { "unlinked" } else { "removed" };
     for path in &removed_paths {
-        logger.info(format!("{} {}", action, path.display()));
+        logger.info(format!("removed {}", path.display()));
     }
 
     // NOTE: Add post-worktree-remove hook if the need arises. See single-remove
@@ -198,13 +178,12 @@ fn confirm_remove(ctx: &ReqCtx<WorktreeOpts>, paths: &[&Path]) -> Result<()> {
         abort!("running non-interactively, use -y instead");
     }
 
-    let action = if ctx.opts.keep { "unlink" } else { "remove" };
     if paths.len() == 1 {
         ctx.io()
-            .write(format!("will {} {}\n", action, paths[0].display()))?;
+            .write(format!("will remove {}\n", paths[0].display()))?;
     } else {
         ctx.io()
-            .write(format!("will {} {} worktrees:\n", action, paths.len()))?;
+            .write(format!("will remove {} worktrees:\n", paths.len()))?;
         for p in paths {
             ctx.io().write(format!("  {}\n", p.display()))?;
         }

eden/scm/tests/test-eden-worktree-remove.t

@@ -51,21 +51,6 @@ test worktree remove - list after remove shows fewer entries
     linked  $TESTTMP/linked2   feature-x
   * main    $TESTTMP/myrepo
 
-test worktree remove - with --keep
-
-  $ sl worktree add $TESTTMP/keep_me
-  created linked worktree at $TESTTMP/keep_me
-  $ sl worktree remove $TESTTMP/keep_me --keep -y
-  unlinked $TESTTMP/keep_me
-  $ test -d $TESTTMP/keep_me
-  $ sl worktree list
-    linked  $TESTTMP/linked1
-    linked  $TESTTMP/linked2   feature-x
-  * main    $TESTTMP/myrepo
-
-clean up kept checkout
-  $ EDENFSCTL_ONLY_RUST=true eden rm -y $TESTTMP/keep_me > /dev/null 2>&1
-
 test worktree remove --all
 
   $ sl worktree remove --all -y
@@ -87,14 +72,14 @@ test worktree remove - pre-worktree-remove hook fires with correct env vars
   $ sl worktree add $TESTTMP/pre_hook_rm1
   created linked worktree at $TESTTMP/pre_hook_rm1
 #if windows
-  $ setconfig hooks.pre-worktree-remove="echo PATH:%HG_PATH% KEEP:%HG_KEEP%"
+  $ setconfig hooks.pre-worktree-remove="echo PATH:%HG_PATH%"
   $ sl worktree remove $TESTTMP/pre_hook_rm1 -y
-  PATH:$TESTTMP?pre_hook_rm1 KEEP:\r (esc) (glob)
+  PATH:$TESTTMP?pre_hook_rm1\r (esc) (glob)
   removed $TESTTMP/pre_hook_rm1
 #else
-  $ setconfig hooks.pre-worktree-remove="echo PATH:\$HG_PATH KEEP:\$HG_KEEP"
+  $ setconfig hooks.pre-worktree-remove="echo PATH:\$HG_PATH"
   $ sl worktree remove $TESTTMP/pre_hook_rm1 -y
-  PATH:$TESTTMP/pre_hook_rm1 KEEP:
+  PATH:$TESTTMP/pre_hook_rm1
   removed $TESTTMP/pre_hook_rm1
 #endif