From 304f6dfc3f3f441e4dfe193f851aa5e70d979232 Mon Sep 17 00:00:00 2001
From: Scott Taylor <scott.g.taylor@jpmchase.com>
Date: Sat, 20 Jul 2024 16:36:30 -0500
Subject: [PATCH] workspace: warn if destination doesn't contain path separator

Users may try to run `jj workspace add <name>` without specifying a
path, which results in the workspace being created in the current
directory. This can be confusing, since the workspace contents will also
be snapshotted in the original workspace if it is not sparse. Adding a
warning should reduce confusion in this case.
---
 cli/src/commands/workspace.rs |  9 ++++++
 cli/tests/test_workspaces.rs  | 55 +++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/cli/src/commands/workspace.rs b/cli/src/commands/workspace.rs
index fde6de92eb..56e4661eb8 100644
--- a/cli/src/commands/workspace.rs
+++ b/cli/src/commands/workspace.rs
@@ -171,6 +171,15 @@ fn cmd_workspace_add(
         "Created workspace in \"{}\"",
         file_util::relative_path(command.cwd(), &destination_path).display()
     )?;
+    // Show a warning if the user passed a path without a separator, since they
+    // may have intended the argument to only be the name for the workspace.
+    if !args.destination.contains(std::path::is_separator) {
+        writeln!(
+            ui.warning_default(),
+            r#"Workspace created inside current directory. If this was unintentional, delete the "{}" directory and run `jj workspace forget {name}` to remove it."#,
+            args.destination
+        )?;
+    }
 
     // Copy sparse patterns from workspace where the command was run
     let mut new_workspace_command = command.for_workable_repo(ui, new_workspace, repo)?;
diff --git a/cli/tests/test_workspaces.rs b/cli/tests/test_workspaces.rs
index 4bd653ac16..cd045ad485 100644
--- a/cli/tests/test_workspaces.rs
+++ b/cli/tests/test_workspaces.rs
@@ -374,6 +374,61 @@ fn test_workspaces_add_workspace_from_subdir() {
     "###);
 }
 
+#[test]
+fn test_workspaces_add_workspace_in_current_workspace() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "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, &["commit", "-m", "initial"]);
+
+    // Try to create workspace using name instead of path
+    let (stdout, stderr) = test_env.jj_cmd_ok(&main_path, &["workspace", "add", "secondary"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @"");
+    insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
+    Created workspace in "secondary"
+    Warning: Workspace created inside current directory. If this was unintentional, delete the "secondary" directory and run `jj workspace forget secondary` to remove it.
+    Working copy now at: pmmvwywv 0a77a39d (empty) (no description set)
+    Parent commit      : qpvuntsm 751b12b7 initial
+    Added 1 files, modified 0 files, removed 0 files
+    "###);
+
+    // Workspace created despite warning
+    let stdout = test_env.jj_cmd_success(&main_path, &["workspace", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    default: rlvkpnrz 46d9ba8b (no description set)
+    secondary: pmmvwywv 0a77a39d (empty) (no description set)
+    "###);
+
+    // Use explicit path instead (no warning)
+    let (stdout, stderr) = test_env.jj_cmd_ok(&main_path, &["workspace", "add", "./third"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @"");
+    insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
+    Created workspace in "third"
+    Working copy now at: zxsnswpr 64746d4b (empty) (no description set)
+    Parent commit      : qpvuntsm 751b12b7 initial
+    Added 1 files, modified 0 files, removed 0 files
+    "###);
+
+    // Both workspaces created
+    let stdout = test_env.jj_cmd_success(&main_path, &["workspace", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    default: rlvkpnrz 477c647f (no description set)
+    secondary: pmmvwywv 0a77a39d (empty) (no description set)
+    third: zxsnswpr 64746d4b (empty) (no description set)
+    "###);
+
+    // Can see files from the other workspaces in main workspace, since they are
+    // child directories and will therefore be snapshotted
+    let stdout = test_env.jj_cmd_success(&main_path, &["file", "list"]);
+    insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
+    file
+    secondary/file
+    third/file
+    "###);
+}
+
 /// Test making changes to the working copy in a workspace as it gets rewritten
 /// from another workspace
 #[test]