Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: branch: drop support for deprecated --glob option #3931

Merged
merged 1 commit into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `jj fix` now defaults to the broader revset `-s reachable(@, mutable())`
instead of `-s @`.

* Dropped support for deprecated `jj branch delete`/`forget` `--glob` option.

### Deprecations

* Replacing `-l` shorthand for `--limit` with `-n` in `jj log`, `jj op log` and `jj obslog`.
Expand Down
15 changes: 2 additions & 13 deletions cli/src/commands/branch/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,8 @@ pub struct BranchDeleteArgs {
/// By default, the specified name matches exactly. Use `glob:` prefix to
/// select branches by wildcard pattern. For details, see
/// https://github.com/martinvonz/jj/blob/main/docs/revsets.md#string-patterns.
#[arg(required_unless_present_any(&["glob"]), value_parser = StringPattern::parse)]
#[arg(required = true, value_parser = StringPattern::parse)]
names: Vec<StringPattern>,

/// Deprecated. Please prefix the pattern with `glob:` instead.
#[arg(long, hide = true, value_parser = StringPattern::glob)]
glob: Vec<StringPattern>,
}

pub fn cmd_branch_delete(
Expand All @@ -44,14 +40,7 @@ pub fn cmd_branch_delete(
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let view = workspace_command.repo().view();
if !args.glob.is_empty() {
writeln!(
ui.warning_default(),
"--glob has been deprecated. Please prefix the pattern with `glob:` instead."
)?;
}
let name_patterns = [&args.names[..], &args.glob[..]].concat();
let names = find_local_branches(view, &name_patterns)?;
let names = find_local_branches(view, &args.names)?;
let mut tx = workspace_command.start_transaction();
for branch_name in names.iter() {
tx.mut_repo()
Expand Down
15 changes: 2 additions & 13 deletions cli/src/commands/branch/forget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,8 @@ pub struct BranchForgetArgs {
/// By default, the specified name matches exactly. Use `glob:` prefix to
/// select branches by wildcard pattern. For details, see
/// https://github.com/martinvonz/jj/blob/main/docs/revsets.md#string-patterns.
#[arg(required_unless_present_any(&["glob"]), value_parser = StringPattern::parse)]
#[arg(required = true, value_parser = StringPattern::parse)]
names: Vec<StringPattern>,

/// Deprecated. Please prefix the pattern with `glob:` instead.
#[arg(long, hide = true, value_parser = StringPattern::glob)]
glob: Vec<StringPattern>,
}

pub fn cmd_branch_forget(
Expand All @@ -47,14 +43,7 @@ pub fn cmd_branch_forget(
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let view = workspace_command.repo().view();
if !args.glob.is_empty() {
writeln!(
ui.warning_default(),
"--glob has been deprecated. Please prefix the pattern with `glob:` instead."
)?;
}
let name_patterns = [&args.names[..], &args.glob[..]].concat();
let names = find_forgettable_branches(view, &name_patterns)?;
let names = find_forgettable_branches(view, &args.names)?;
let mut tx = workspace_command.start_transaction();
for branch_name in names.iter() {
tx.mut_repo().remove_branch(branch_name);
Expand Down
4 changes: 2 additions & 2 deletions cli/tests/cli-reference@.md.snap
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ Create a new branch

Delete an existing branch and propagate the deletion to remotes on the next push

**Usage:** `jj branch delete [NAMES]...`
**Usage:** `jj branch delete <NAMES>...`

###### **Arguments:**

Expand All @@ -281,7 +281,7 @@ Forget everything about a branch, including its local and remote targets

A forgotten branch will not impact remotes on future pushes. It will be recreated on future pulls if it still exists in the remote.

**Usage:** `jj branch forget [NAMES]...`
**Usage:** `jj branch forget <NAMES>...`

###### **Arguments:**

Expand Down
32 changes: 11 additions & 21 deletions cli/tests/test_branch_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,11 +376,9 @@ fn test_branch_forget_glob() {
@ bar-2 foo-1 foo-3 foo-4 230dd059e1b0
◉ 000000000000
"###);
let (stdout, stderr) =
test_env.jj_cmd_ok(&repo_path, &["branch", "forget", "--glob", "foo-[1-3]"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "forget", "glob:foo-[1-3]"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Warning: --glob has been deprecated. Please prefix the pattern with `glob:` instead.
Forgot 2 branches.
"###);
test_env.jj_cmd_ok(&repo_path, &["undo"]);
Expand All @@ -398,12 +396,10 @@ fn test_branch_forget_glob() {
// multiple glob patterns, shouldn't produce an error.
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&["branch", "forget", "foo-4", "--glob", "foo-*", "glob:foo-*"],
&["branch", "forget", "foo-4", "glob:foo-*", "glob:foo-*"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Warning: --glob has been deprecated. Please prefix the pattern with `glob:` instead.
"###);
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ bar-2 230dd059e1b0
◉ 000000000000
Expand All @@ -412,18 +408,17 @@ fn test_branch_forget_glob() {
// Malformed glob
let stderr = test_env.jj_cmd_cli_error(&repo_path, &["branch", "forget", "glob:foo-[1-3"]);
insta::assert_snapshot!(stderr, @r###"
error: invalid value 'glob:foo-[1-3' for '[NAMES]...': Pattern syntax error near position 4: invalid range pattern
error: invalid value 'glob:foo-[1-3' for '<NAMES>...': Pattern syntax error near position 4: invalid range pattern

For more information, try '--help'.
"###);

// We get an error if none of the globs match anything
let stderr = test_env.jj_cmd_failure(
&repo_path,
&["branch", "forget", "glob:bar*", "glob:baz*", "--glob=boom*"],
&["branch", "forget", "glob:bar*", "glob:baz*", "glob:boom*"],
);
insta::assert_snapshot!(stderr, @r###"
Warning: --glob has been deprecated. Please prefix the pattern with `glob:` instead.
Error: No matching branches for patterns: baz*, boom*
"###);
}
Expand Down Expand Up @@ -458,11 +453,9 @@ fn test_branch_delete_glob() {
@ bar-2 foo-1 foo-3 foo-4 6fbf398c2d59
◉ 000000000000
"###);
let (stdout, stderr) =
test_env.jj_cmd_ok(&repo_path, &["branch", "delete", "--glob", "foo-[1-3]"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "delete", "glob:foo-[1-3]"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Warning: --glob has been deprecated. Please prefix the pattern with `glob:` instead.
Deleted 2 branches.
"###);
test_env.jj_cmd_ok(&repo_path, &["undo"]);
Expand All @@ -478,22 +471,19 @@ fn test_branch_delete_glob() {

// We get an error if none of the globs match live branches. Unlike `jj branch
// forget`, it's not allowed to delete already deleted branches.
let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "delete", "--glob=foo-[1-3]"]);
let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "delete", "glob:foo-[1-3]"]);
insta::assert_snapshot!(stderr, @r###"
Warning: --glob has been deprecated. Please prefix the pattern with `glob:` instead.
Error: No matching branches for patterns: foo-[1-3]
"###);

// Deleting a branch via both explicit name and glob pattern, or with
// multiple glob patterns, shouldn't produce an error.
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&["branch", "delete", "foo-4", "--glob", "foo-*", "glob:foo-*"],
&["branch", "delete", "foo-4", "glob:foo-*", "glob:foo-*"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Warning: --glob has been deprecated. Please prefix the pattern with `glob:` instead.
"###);
insta::assert_snapshot!(stderr, @"");
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ bar-2 foo-1@origin foo-3@origin foo-4@origin 6fbf398c2d59
◉ 000000000000
Expand All @@ -514,15 +504,15 @@ fn test_branch_delete_glob() {
// Malformed glob
let stderr = test_env.jj_cmd_cli_error(&repo_path, &["branch", "delete", "glob:foo-[1-3"]);
insta::assert_snapshot!(stderr, @r###"
error: invalid value 'glob:foo-[1-3' for '[NAMES]...': Pattern syntax error near position 4: invalid range pattern
error: invalid value 'glob:foo-[1-3' for '<NAMES>...': Pattern syntax error near position 4: invalid range pattern

For more information, try '--help'.
"###);

// Unknown pattern kind
let stderr = test_env.jj_cmd_cli_error(&repo_path, &["branch", "forget", "whatever:branch"]);
insta::assert_snapshot!(stderr, @r###"
error: invalid value 'whatever:branch' for '[NAMES]...': Invalid string pattern kind "whatever:"
error: invalid value 'whatever:branch' for '<NAMES>...': Invalid string pattern kind "whatever:"

For more information, try '--help'.
Hint: Try prefixing with one of `exact:`, `glob:` or `substring:`
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/test_git_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1190,7 +1190,7 @@ fn test_git_fetch_removed_parent_branch() {
"###);

// Remove all branches in origin.
test_env.jj_cmd_ok(&source_git_repo_path, &["branch", "forget", "--glob", "*"]);
test_env.jj_cmd_ok(&source_git_repo_path, &["branch", "forget", "glob:*"]);

// Fetch branches master, trunk1 and a1 from origin and check that only those
// branches have been removed and that others were not rebased because of
Expand Down