Skip to content

Commit

Permalink
workspace: warn if destination doesn't contain path separator
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
scott2000 committed Jul 26, 2024
1 parent caea3e2 commit 71114c4
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 0 deletions.
9 changes: 9 additions & 0 deletions cli/src/commands/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
55 changes: 55 additions & 0 deletions cli/tests/test_workspaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit 71114c4

Please sign in to comment.