diff --git a/CHANGELOG.md b/CHANGELOG.md index 8db6d9580ae..dbe941cf8e5 100644 --- a/CHANGELOG.md +++ b/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 ` will now give an error if `` 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 ./` instead. + ### Deprecations ### New features diff --git a/cli/src/commands/workspace.rs b/cli/src/commands/workspace.rs index 7add4413bc2..8591935db86 100644 --- a/cli/src/commands/workspace.rs +++ b/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 "./" for the path + // instead of "". + 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 { diff --git a/cli/tests/test_workspaces.rs b/cli/tests/test_workspaces.rs index ef1847fb7af..89cf4f583f4 100644 --- a/cli/tests/test_workspaces.rs +++ b/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]