workspace: require path separator when adding workspace

Running `jj workspace add <name>` is an easy mistake to make, and
currently it creates a new workspace inside of the current workspace,
which probably isn't what the user intended. This commit adds an error
message with a hint to use "./<name>" explicitly if it was intentional.

CHANGELOG.md

@@ -23,6 +23,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * Updated defaults for graph node symbol templates `templates.log_node` and
   `templates.op_log_node`.
 
+* `jj workspace add <path>` will now give an error if `<path>` doesn't contain
+  any path separators, since creating a workspace inside of the current
+  directory is likely a mistake. If creating a workspace inside of the current
+  directory is intended, explicitly use `jj workspace add ./<path>` instead.
+
 ### Deprecations
 
 ### New features

cli/src/commands/workspace.rs

@@ -15,6 +15,7 @@
 use std::fmt::Debug;
 use std::fs;
 use std::io::Write;
+use std::path::Path;
 use std::sync::Arc;
 
 use clap::Subcommand;
@@ -34,7 +35,9 @@ use crate::cli_util::{
     check_stale_working_copy, print_checkout_stats, short_commit_hash, CommandHelper, RevisionArg,
     WorkingCopyFreshness, WorkspaceCommandHelper,
 };
-use crate::command_error::{internal_error_with_message, user_error, CommandError};
+use crate::command_error::{
+    internal_error_with_message, user_error, user_error_with_hint, CommandError,
+};
 use crate::ui::Ui;
 
 /// Commands for working with workspaces
@@ -137,9 +140,26 @@ fn cmd_workspace_add(
     let destination_path = command.cwd().join(&args.destination);
     if destination_path.exists() {
         return Err(user_error("Workspace already exists"));
-    } else {
-        fs::create_dir(&destination_path).context(&destination_path)?;
     }
+    // Require at least one path separator in the destination path. This
+    // prevents the user from accidentally creating a workspace inside of the
+    // current workspace. If the user intends to create a workspace inside the
+    // current directory, they can explicitly use "./<name>" for the path
+    // instead of "<name>".
+    if !args.destination.contains(std::path::is_separator) {
+        let explicit_path = Path::new(".").join(&args.destination);
+        return Err(user_error_with_hint(
+            format!(
+                "Refusing to create workspace at \"{}\", since it is likely a mistake.",
+                explicit_path.display()
+            ),
+            format!(
+                "If that's what you intended, explicitly use \"{}\" instead to silence this error.",
+                explicit_path.display()
+            ),
+        ));
+    }
+    fs::create_dir(&destination_path).context(&destination_path)?;
     let name = if let Some(name) = &args.name {
         name.to_string()
     } else {

cli/tests/test_workspaces.rs

@@ -252,6 +252,54 @@ fn test_workspaces_add_workspace_multiple_revisions() {
     "###);
 }
 
+#[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 stderr = test_env.jj_cmd_failure(&main_path, &["workspace", "add", "secondary"]);
+    insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
+    Error: Refusing to create workspace at "./secondary", since it is likely a mistake.
+    Hint: If that's what you intended, explicitly use "./secondary" instead to silence this error.
+    "###);
+
+    // No workspace created
+    let stdout = test_env.jj_cmd_success(&main_path, &["workspace", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    default: rlvkpnrz 8183d0fc (empty) (no description set)
+    "###);
+
+    // Use explicit path as recommended in error
+    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"
+    Working copy now at: zxsnswpr 64746d4b (empty) (no description set)
+    Parent commit      : qpvuntsm 751b12b7 initial
+    Added 1 files, modified 0 files, removed 0 files
+    "###);
+
+    // Workspace should be created successfully
+    let stdout = test_env.jj_cmd_success(&main_path, &["workspace", "list"]);
+    insta::assert_snapshot!(stdout, @r###"
+    default: rlvkpnrz ec597995 (no description set)
+    secondary: zxsnswpr 64746d4b (empty) (no description set)
+    "###);
+
+    // Can see files from the other workspace in main workspace, since it is a
+    // child directory 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
+    "###);
+}
+
 /// Test making changes to the working copy in a workspace as it gets rewritten
 /// from another workspace
 #[test]