From c183b94aeffc4c01cc658f15b84874abba9a7591 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Thu, 27 Jul 2023 17:33:54 -0700 Subject: [PATCH] cli: warn when using `:` revset operator --- CHANGELOG.md | 6 +-- examples/custom-command/main.rs | 2 +- src/cli_util.rs | 83 +++++++++++++++++++++++++++++---- src/commands/bench.rs | 10 ++-- src/commands/branch.rs | 6 +-- src/commands/git.rs | 8 ++-- src/commands/mod.rs | 73 ++++++++++++++++------------- tests/test_abandon_command.rs | 2 +- tests/test_branch_command.rs | 2 +- tests/test_diff_command.rs | 8 +++- tests/test_duplicate_command.rs | 8 ++-- tests/test_log_command.rs | 57 ++++++++++++++++++++-- 12 files changed, 197 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c2b363e4..f9e37d534 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * The storage format of branches, tags, and git refs has changed. Newly-stored repository data will no longer be loadable by older binaries. +* The `:` revset operator is deprecated. Use `::` instead. We plan to delete the + `:` form in jj 0.15+. + ### New features * `jj config edit --user` and `jj config set --user` will now pick a default @@ -22,9 +25,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj log` output is now topologically grouped. [#242](https://github.com/martinvonz/jj/issues/242) -* The `:` revset operator can now be written as `::` instead. We plan to - delete the `:` form in jj 0.15+. - ### Fixed bugs ## [0.8.0] - 2023-07-09 diff --git a/examples/custom-command/main.rs b/examples/custom-command/main.rs index 1d6169de1..32e57a1c9 100644 --- a/examples/custom-command/main.rs +++ b/examples/custom-command/main.rs @@ -36,7 +36,7 @@ fn run_custom_command( match command { CustomCommands::Frobnicate(args) => { let mut workspace_command = command_helper.workspace_helper(ui)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; let mut tx = workspace_command.start_transaction("Frobnicate"); let new_commit = tx .mut_repo() diff --git a/src/cli_util.rs b/src/cli_util.rs index 8d169e112..afe323117 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -957,8 +957,12 @@ impl WorkspaceCommandHelper { ) } - pub fn resolve_single_rev(&self, revision_str: &str) -> Result { - let revset_expression = self.parse_revset(revision_str)?; + pub fn resolve_single_rev( + &self, + revision_str: &str, + ui: &mut Ui, + ) -> Result { + let revset_expression = self.parse_revset(revision_str, Some(ui))?; let revset = self.evaluate_revset(revset_expression)?; let mut iter = revset.iter().commits(self.repo().store()).fuse(); match (iter.next(), iter.next()) { @@ -987,8 +991,12 @@ impl WorkspaceCommandHelper { } } - pub fn resolve_revset(&self, revision_str: &str) -> Result, CommandError> { - let revset_expression = self.parse_revset(revision_str)?; + pub fn resolve_revset( + &self, + revision_str: &str, + ui: &mut Ui, + ) -> Result, CommandError> { + let revset_expression = self.parse_revset(revision_str, Some(ui))?; let revset = self.evaluate_revset(revset_expression)?; Ok(revset.iter().commits(self.repo().store()).try_collect()?) } @@ -996,12 +1004,69 @@ impl WorkspaceCommandHelper { pub fn parse_revset( &self, revision_str: &str, + ui: Option<&mut Ui>, ) -> Result, RevsetParseError> { let expression = revset::parse( revision_str, &self.revset_aliases_map, Some(&self.revset_context()), )?; + if let Some(ui) = ui { + fn has_legacy_rule(expression: &Rc) -> bool { + match expression.as_ref() { + RevsetExpression::None => false, + RevsetExpression::All => false, + RevsetExpression::Commits(_) => false, + RevsetExpression::CommitRef(_) => false, + RevsetExpression::Ancestors { + heads, + generation: _, + is_legacy, + } => *is_legacy || has_legacy_rule(heads), + RevsetExpression::Descendants { + roots, + generation: _, + is_legacy, + } => *is_legacy || has_legacy_rule(roots), + RevsetExpression::Range { + roots, + heads, + generation: _, + } => has_legacy_rule(roots) || has_legacy_rule(heads), + RevsetExpression::DagRange { + roots, + heads, + is_legacy, + } => *is_legacy || has_legacy_rule(roots) || has_legacy_rule(heads), + RevsetExpression::Heads(expression) => has_legacy_rule(expression), + RevsetExpression::Roots(expression) => has_legacy_rule(expression), + RevsetExpression::Latest { + candidates, + count: _, + } => has_legacy_rule(candidates), + RevsetExpression::Filter(_) => false, + RevsetExpression::AsFilter(expression) => has_legacy_rule(expression), + RevsetExpression::Present(expression) => has_legacy_rule(expression), + RevsetExpression::NotIn(expression) => has_legacy_rule(expression), + RevsetExpression::Union(expression1, expression2) => { + has_legacy_rule(expression1) || has_legacy_rule(expression2) + } + RevsetExpression::Intersection(expression1, expression2) => { + has_legacy_rule(expression1) || has_legacy_rule(expression2) + } + RevsetExpression::Difference(expression1, expression2) => { + has_legacy_rule(expression1) || has_legacy_rule(expression2) + } + } + } + if has_legacy_rule(&expression) { + writeln!( + ui.warning(), + "The `:` revset operator is deprecated. Please switch to `::`." + ) + .ok(); + } + } Ok(revset::optimize(expression)) } @@ -1046,7 +1111,7 @@ impl WorkspaceCommandHelper { .get_string("revsets.short-prefixes") .unwrap_or_else(|_| self.settings.default_revset()); if !revset_string.is_empty() { - let disambiguation_revset = self.parse_revset(&revset_string).unwrap(); + let disambiguation_revset = self.parse_revset(&revset_string, None).unwrap(); context = context .disambiguate_within(disambiguation_revset, Some(self.workspace_id().clone())); } @@ -1763,10 +1828,11 @@ fn load_revset_aliases( pub fn resolve_multiple_nonempty_revsets( revision_args: &[RevisionArg], workspace_command: &WorkspaceCommandHelper, + ui: &mut Ui, ) -> Result, CommandError> { let mut acc = IndexSet::new(); for revset in revision_args { - let revisions = workspace_command.resolve_revset(revset)?; + let revisions = workspace_command.resolve_revset(revset, ui)?; workspace_command.check_non_empty(&revisions)?; acc.extend(revisions); } @@ -1775,16 +1841,17 @@ pub fn resolve_multiple_nonempty_revsets( pub fn resolve_multiple_nonempty_revsets_flag_guarded( workspace_command: &WorkspaceCommandHelper, + ui: &mut Ui, revisions: &[RevisionArg], allow_plural_revsets: bool, ) -> Result, CommandError> { if allow_plural_revsets { - resolve_multiple_nonempty_revsets(revisions, workspace_command) + resolve_multiple_nonempty_revsets(revisions, workspace_command, ui) } else { let mut commits = IndexSet::new(); for revision_str in revisions { let commit = workspace_command - .resolve_single_rev(revision_str) + .resolve_single_rev(revision_str, ui) .map_err(append_large_revsets_hint_if_multiple_revisions)?; let commit_hash = short_commit_hash(commit.id()); if !commits.insert(commit) { diff --git a/src/commands/bench.rs b/src/commands/bench.rs index 5b078e69b..040790128 100644 --- a/src/commands/bench.rs +++ b/src/commands/bench.rs @@ -131,8 +131,8 @@ pub(crate) fn cmd_bench( match subcommand { BenchCommands::CommonAncestors(command_matches) => { let workspace_command = command.workspace_helper(ui)?; - let commit1 = workspace_command.resolve_single_rev(&command_matches.revision1)?; - let commit2 = workspace_command.resolve_single_rev(&command_matches.revision2)?; + let commit1 = workspace_command.resolve_single_rev(&command_matches.revision1, ui)?; + let commit2 = workspace_command.resolve_single_rev(&command_matches.revision2, ui)?; let index = workspace_command.repo().index(); let routine = || index.common_ancestors(&[commit1.id().clone()], &[commit2.id().clone()]); @@ -149,9 +149,9 @@ pub(crate) fn cmd_bench( BenchCommands::IsAncestor(command_matches) => { let workspace_command = command.workspace_helper(ui)?; let ancestor_commit = - workspace_command.resolve_single_rev(&command_matches.ancestor)?; + workspace_command.resolve_single_rev(&command_matches.ancestor, ui)?; let descendant_commit = - workspace_command.resolve_single_rev(&command_matches.descendant)?; + workspace_command.resolve_single_rev(&command_matches.descendant, ui)?; let index = workspace_command.repo().index(); let routine = || index.is_ancestor(ancestor_commit.id(), descendant_commit.id()); run_bench( @@ -208,7 +208,7 @@ fn bench_revset( revset: &str, ) -> Result<(), CommandError> { writeln!(ui, "----------Testing revset: {revset}----------")?; - let expression = workspace_command.parse_revset(revset)?; + let expression = workspace_command.parse_revset(revset, Some(ui))?; // Time both evaluation and iteration. let routine = |workspace_command: &WorkspaceCommandHelper, expression| { workspace_command diff --git a/src/commands/branch.rs b/src/commands/branch.rs index 2b337023f..acb35caa1 100644 --- a/src/commands/branch.rs +++ b/src/commands/branch.rs @@ -151,7 +151,7 @@ fn cmd_branch_create( } let target_commit = - workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"), ui)?; workspace_command.check_rewritable(&target_commit)?; let mut tx = workspace_command.start_transaction(&format!( "create {} pointing to commit {}", @@ -182,7 +182,7 @@ fn cmd_branch_set( } let target_commit = - workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"), ui)?; workspace_command.check_rewritable(&target_commit)?; if !args.allow_backwards && !branch_names.iter().all(|branch_name| { @@ -344,7 +344,7 @@ fn cmd_branch_list( let filter_expressions: Vec<_> = args .revisions .iter() - .map(|revision_str| workspace_command.parse_revset(revision_str)) + .map(|revision_str| workspace_command.parse_revset(revision_str, Some(ui))) .try_collect()?; let filter_expression = RevsetExpression::union_all(&filter_expressions); // Intersects with the set of all branch targets to minimize the lookup space. diff --git a/src/commands/git.rs b/src/commands/git.rs index fb9ad1f4c..600c712f8 100644 --- a/src/commands/git.rs +++ b/src/commands/git.rs @@ -612,12 +612,12 @@ fn cmd_git_push( let change_commits: Vec<_> = args .change .iter() - .map(|change_str| workspace_command.resolve_single_rev(change_str)) + .map(|change_str| workspace_command.resolve_single_rev(change_str, ui)) .try_collect()?; // TODO: Narrow search space to local target commits. // TODO: Remove redundant CommitId -> Commit -> CommitId round trip. let revision_commit_ids: HashSet<_> = - resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command)? + resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command, ui)? .iter() .map(|commit| commit.id().clone()) .collect(); @@ -692,7 +692,7 @@ fn cmd_git_push( let short_change_id = short_change_hash(commit.change_id()); if tx .base_workspace_helper() - .resolve_single_rev(&short_change_id) + .resolve_single_rev(&short_change_id, ui) .is_ok() { // Short change ID is not ambiguous, so update the branch name to use it. @@ -985,7 +985,7 @@ fn cmd_git_submodule_print_gitmodules( ) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; let repo = workspace_command.repo(); - let commit = workspace_command.resolve_single_rev(&args.revisions)?; + let commit = workspace_command.resolve_single_rev(&args.revisions, ui)?; let gitmodules_path = RepoPath::from_internal_string(".gitmodules"); let mut gitmodules_file = match commit.tree().path_value(&gitmodules_path) { None => { diff --git a/src/commands/mod.rs b/src/commands/mod.rs index b6d711d57..ee086e2dc 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1274,7 +1274,7 @@ fn cmd_checkout( args: &CheckoutArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let target = workspace_command.resolve_single_rev(&args.revision)?; + let target = workspace_command.resolve_single_rev(&args.revision, ui)?; let mut tx = workspace_command.start_transaction(&format!("check out commit {}", target.id().hex())); let commit_builder = tx @@ -1365,7 +1365,7 @@ Make sure they're ignored, then try again.", #[instrument(skip_all)] fn cmd_files(ui: &mut Ui, command: &CommandHelper, args: &FilesArgs) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; let matcher = workspace_command.matcher_from_values(&args.paths)?; ui.request_pager(); for (name, _value) in commit.tree().entries_matching(matcher.as_ref()) { @@ -1377,7 +1377,7 @@ fn cmd_files(ui: &mut Ui, command: &CommandHelper, args: &FilesArgs) -> Result<( #[instrument(skip_all)] fn cmd_cat(ui: &mut Ui, command: &CommandHelper, args: &CatArgs) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; let path = workspace_command.parse_file_path(&args.path)?; let repo = workspace_command.repo(); match commit.tree().path_value(&path) { @@ -1411,13 +1411,13 @@ fn cmd_diff(ui: &mut Ui, command: &CommandHelper, args: &DiffArgs) -> Result<(), let from_tree; let to_tree; if args.from.is_some() || args.to.is_some() { - let from = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?; + let from = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"), ui)?; from_tree = from.tree(); - let to = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"))?; + let to = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"), ui)?; to_tree = to.tree(); } else { let commit = - workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"), ui)?; let parents = commit.parents(); from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &parents)?; to_tree = commit.tree() @@ -1438,7 +1438,7 @@ fn cmd_diff(ui: &mut Ui, command: &CommandHelper, args: &DiffArgs) -> Result<(), #[instrument(skip_all)] fn cmd_show(ui: &mut Ui, command: &CommandHelper, args: &ShowArgs) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; let template_string = command.settings().config().get_string("templates.show")?; let template = workspace_command.parse_commit_template(&template_string)?; ui.request_pager(); @@ -1560,12 +1560,12 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C let revset_expression = { let mut expression = if args.revisions.is_empty() { - workspace_command.parse_revset(&command.settings().default_revset())? + workspace_command.parse_revset(&command.settings().default_revset(), Some(ui))? } else { let expressions: Vec<_> = args .revisions .iter() - .map(|revision_str| workspace_command.parse_revset(revision_str)) + .map(|revision_str| workspace_command.parse_revset(revision_str, Some(ui))) .try_collect()?; RevsetExpression::union_all(&expressions) }; @@ -1720,7 +1720,7 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C fn cmd_obslog(ui: &mut Ui, command: &CommandHelper, args: &ObslogArgs) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; - let start_commit = workspace_command.resolve_single_rev(&args.revision)?; + let start_commit = workspace_command.resolve_single_rev(&args.revision, ui)?; let wc_commit_id = workspace_command.get_wc_commit_id(); let diff_formats = @@ -1823,8 +1823,8 @@ fn cmd_interdiff( args: &InterdiffArgs, ) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; - let from = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?; - let to = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"))?; + let from = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"), ui)?; + let to = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"), ui)?; let from_tree = rebase_to_dest_parent(&workspace_command, &from, &to)?; let matcher = workspace_command.matcher_from_values(&args.paths)?; @@ -1976,7 +1976,7 @@ fn cmd_describe( args: &DescribeArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; workspace_command.check_rewritable(&commit)?; let description = if args.stdin { let mut buffer = String::new(); @@ -2059,7 +2059,7 @@ fn cmd_duplicate( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let to_duplicate: IndexSet = - resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command)?; + resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command, ui)?; to_duplicate .iter() .map(|commit| workspace_command.check_rewritable(commit)) @@ -2117,7 +2117,7 @@ fn cmd_abandon( args: &AbandonArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let to_abandon = resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command)?; + let to_abandon = resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command, ui)?; to_abandon .iter() .map(|commit| workspace_command.check_rewritable(commit)) @@ -2164,7 +2164,7 @@ fn cmd_abandon( #[instrument(skip_all)] fn cmd_edit(ui: &mut Ui, command: &CommandHelper, args: &EditArgs) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let new_commit = workspace_command.resolve_single_rev(&args.revision)?; + let new_commit = workspace_command.resolve_single_rev(&args.revision, ui)?; workspace_command.check_rewritable(&new_commit)?; if workspace_command.get_wc_commit_id() == Some(new_commit.id()) { ui.write("Already editing that commit\n")?; @@ -2181,11 +2181,13 @@ fn cmd_edit(ui: &mut Ui, command: &CommandHelper, args: &EditArgs) -> Result<(), /// to be rewriteable. fn resolve_destination_revs( workspace_command: &WorkspaceCommandHelper, + ui: &mut Ui, revisions: &[RevisionArg], allow_plural_revsets: bool, ) -> Result, CommandError> { let commits = resolve_multiple_nonempty_revsets_flag_guarded( workspace_command, + ui, revisions, allow_plural_revsets, )?; @@ -2206,6 +2208,7 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C ); let target_commits = resolve_destination_revs( &workspace_command, + ui, &args.revisions, args.allow_large_revsets, )? @@ -2352,9 +2355,9 @@ fn combine_messages( #[instrument(skip_all)] fn cmd_move(ui: &mut Ui, command: &CommandHelper, args: &MoveArgs) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let source = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?; + let source = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"), ui)?; let mut destination = - workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"))?; + workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"), ui)?; if source.id() == destination.id() { return Err(user_error("Source and destination cannot be the same.")); } @@ -2441,7 +2444,7 @@ from the source will be moved into the destination. #[instrument(skip_all)] fn cmd_squash(ui: &mut Ui, command: &CommandHelper, args: &SquashArgs) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; workspace_command.check_rewritable(&commit)?; let parents = commit.parents(); if parents.len() != 1 { @@ -2535,7 +2538,7 @@ fn cmd_unsquash( args: &UnsquashArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; workspace_command.check_rewritable(&commit)?; let parents = commit.parents(); if parents.len() != 1 { @@ -2612,7 +2615,7 @@ fn cmd_chmod(ui: &mut Ui, command: &CommandHelper, args: &ChmodArgs) -> Result<( .iter() .map(|path| workspace_command.parse_file_path(path)) .try_collect()?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; workspace_command.check_rewritable(&commit)?; let mut tx = workspace_command.start_transaction(&format!( @@ -2697,7 +2700,7 @@ fn cmd_resolve( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let matcher = workspace_command.matcher_from_values(&args.paths)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; let tree = commit.merged_tree()?; let conflicts = tree .conflicts() @@ -2850,12 +2853,12 @@ fn cmd_restore( let mut workspace_command = command.workspace_helper(ui)?; let (from_tree, to_commit); if args.from.is_some() || args.to.is_some() { - to_commit = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"))?; + to_commit = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"), ui)?; from_tree = workspace_command - .resolve_single_rev(args.from.as_deref().unwrap_or("@"))? + .resolve_single_rev(args.from.as_deref().unwrap_or("@"), ui)? .tree(); } else { - to_commit = workspace_command.resolve_single_rev("@")?; + to_commit = workspace_command.resolve_single_rev("@", ui)?; from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &to_commit.parents())?; } workspace_command.check_rewritable(&to_commit)?; @@ -2908,16 +2911,17 @@ fn cmd_diffedit( let (target_commit, base_commits, diff_description); if args.from.is_some() || args.to.is_some() { - target_commit = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"))?; + target_commit = + workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"), ui)?; base_commits = - vec![workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?]; + vec![workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"), ui)?]; diff_description = format!( "The diff initially shows the commit's changes relative to:\n{}", workspace_command.format_commit_summary(&base_commits[0]) ); } else { target_commit = - workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"), ui)?; base_commits = target_commit.parents(); diff_description = "The diff initially shows the commit's changes.".to_string(); }; @@ -3006,7 +3010,7 @@ fn diff_summary_to_description(bytes: &[u8]) -> String { #[instrument(skip_all)] fn cmd_split(ui: &mut Ui, command: &CommandHelper, args: &SplitArgs) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let commit = workspace_command.resolve_single_rev(&args.revision)?; + let commit = workspace_command.resolve_single_rev(&args.revision, ui)?; workspace_command.check_rewritable(&commit)?; let matcher = workspace_command.matcher_from_values(&args.paths)?; let mut tx = @@ -3112,6 +3116,7 @@ fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result let mut workspace_command = command.workspace_helper(ui)?; let new_parents = resolve_destination_revs( &workspace_command, + ui, &args.destination, args.allow_large_revsets, )? @@ -3122,6 +3127,7 @@ fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result } else if !args.source.is_empty() { let source_commits = resolve_multiple_nonempty_revsets_flag_guarded( &workspace_command, + ui, &args.source, args.allow_large_revsets, )?; @@ -3134,10 +3140,11 @@ fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result )?; } else { let branch_commits = if args.branch.is_empty() { - IndexSet::from([workspace_command.resolve_single_rev("@")?]) + IndexSet::from([workspace_command.resolve_single_rev("@", ui)?]) } else { resolve_multiple_nonempty_revsets_flag_guarded( &workspace_command, + ui, &args.branch, args.allow_large_revsets, )? @@ -3220,7 +3227,7 @@ fn rebase_revision( new_parents: &[Commit], rev_str: &str, ) -> Result<(), CommandError> { - let old_commit = workspace_command.resolve_single_rev(rev_str)?; + let old_commit = workspace_command.resolve_single_rev(rev_str, ui)?; workspace_command.check_rewritable(&old_commit)?; check_rebase_destinations(workspace_command.repo(), new_parents, &old_commit)?; let children_expression = RevsetExpression::commit(old_commit.id().clone()).children(); @@ -3318,10 +3325,10 @@ fn cmd_backout( args: &BackoutArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let commit_to_back_out = workspace_command.resolve_single_rev(&args.revision)?; + let commit_to_back_out = workspace_command.resolve_single_rev(&args.revision, ui)?; let mut parents = vec![]; for revision_str in &args.destination { - let destination = workspace_command.resolve_single_rev(revision_str)?; + let destination = workspace_command.resolve_single_rev(revision_str, ui)?; parents.push(destination); } let mut tx = workspace_command.start_transaction(&format!( diff --git a/tests/test_abandon_command.rs b/tests/test_abandon_command.rs index fcd9c40cc..b08e52f18 100644 --- a/tests/test_abandon_command.rs +++ b/tests/test_abandon_command.rs @@ -130,7 +130,7 @@ fn test_rebase_branch_with_merge() { // Test abandoning the same commit twice indirectly test_env.jj_cmd_success(&repo_path, &["undo"]); - let stdout = test_env.jj_cmd_success(&repo_path, &["abandon", "d:", "a:"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["abandon", "d::", "a::"]); insta::assert_snapshot!(stdout, @r###" Abandoned the following commits: 5557ece3e631 e diff --git a/tests/test_branch_command.rs b/tests/test_branch_command.rs index c3a36015a..4a92a5755 100644 --- a/tests/test_branch_command.rs +++ b/tests/test_branch_command.rs @@ -473,7 +473,7 @@ fn test_branch_list_filtered_by_revset() { insta::assert_snapshot!( test_env.jj_cmd_success( &local_path, - &["log", "-r:(branches() | remote_branches())", "-T", template], + &["log", "-r::(branches() | remote_branches())", "-T", template], ), @r###" ◉ e31634b64294 remote-rewrite* diff --git a/tests/test_diff_command.rs b/tests/test_diff_command.rs index 44e140803..9c4a4307e 100644 --- a/tests/test_diff_command.rs +++ b/tests/test_diff_command.rs @@ -389,7 +389,13 @@ fn test_color_words_diff_missing_newline() { let stdout = test_env.jj_cmd_success( &repo_path, - &["log", "-Tdescription", "-pr:@-", "--no-graph", "--reversed"], + &[ + "log", + "-Tdescription", + "-pr::@-", + "--no-graph", + "--reversed", + ], ); insta::assert_snapshot!(stdout, @r###" === Empty diff --git a/tests/test_duplicate_command.rs b/tests/test_duplicate_command.rs index 0676d394a..0f4a51fd0 100644 --- a/tests/test_duplicate_command.rs +++ b/tests/test_duplicate_command.rs @@ -109,7 +109,7 @@ fn test_duplicate_many() { ◉ 000000000000 "###); - let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "b:"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "b::"]); insta::assert_snapshot!(stdout, @r###" Duplicated 1394f625cbbd as 3b74d9691015 b Duplicated 921dde6e55c0 as 8348ddcec733 e @@ -150,7 +150,7 @@ fn test_duplicate_many() { // Try specifying the same commit twice indirectly test_env.jj_cmd_success(&repo_path, &["undo"]); - let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "b:", "d:"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "b::", "d::"]); insta::assert_snapshot!(stdout, @r###" Duplicated 1394f625cbbd as fa167d18a83a b Duplicated ebd06dba20ec as 2181781b4f81 d @@ -185,7 +185,7 @@ fn test_duplicate_many() { ◉ 2443ea76b0b1 a ◉ 000000000000 "###); - let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "d:", "a"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "d::", "a"]); insta::assert_snapshot!(stdout, @r###" Duplicated 2443ea76b0b1 as c6f7f8c4512e a Duplicated ebd06dba20ec as d94e4c55a68b d @@ -210,7 +210,7 @@ fn test_duplicate_many() { // Check for BUG -- makes too many 'a'-s, etc. test_env.jj_cmd_success(&repo_path, &["undo"]); - let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "a:"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["duplicate", "a::"]); insta::assert_snapshot!(stdout, @r###" Duplicated 2443ea76b0b1 as 0fe67a05989e a Duplicated 1394f625cbbd as e13ac0adabdf b diff --git a/tests/test_log_command.rs b/tests/test_log_command.rs index 8bef483e7..6e2509e6a 100644 --- a/tests/test_log_command.rs +++ b/tests/test_log_command.rs @@ -30,6 +30,55 @@ fn test_log_with_empty_revision() { "###); } +#[test] +fn test_log_legacy_range_operator() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r=@:"]); + insta::assert_snapshot!(stdout, @r###" + @ qpvuntsmwlqt test.user@example.com 2001-02-03 04:05:07.000 +07:00 230dd059e1b0 + │ (empty) (no description set) + ~ + "###); + insta::assert_snapshot!(stderr, @r###" + The `:` revset operator is deprecated. Please switch to `::`. + "###); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r=:@"]); + insta::assert_snapshot!(stdout, @r###" + @ qpvuntsmwlqt test.user@example.com 2001-02-03 04:05:07.000 +07:00 230dd059e1b0 + │ (empty) (no description set) + ◉ zzzzzzzzzzzz 1970-01-01 00:00:00.000 +00:00 000000000000 + (empty) (no description set) + "###); + insta::assert_snapshot!(stderr, @r###" + The `:` revset operator is deprecated. Please switch to `::`. + "###); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "-r=root:@"]); + insta::assert_snapshot!(stdout, @r###" + @ qpvuntsmwlqt test.user@example.com 2001-02-03 04:05:07.000 +07:00 230dd059e1b0 + │ (empty) (no description set) + ◉ zzzzzzzzzzzz 1970-01-01 00:00:00.000 +00:00 000000000000 + (empty) (no description set) + "###); + insta::assert_snapshot!(stderr, @r###" + The `:` revset operator is deprecated. Please switch to `::`. + "###); + let (stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["log", "-r=x", "--config-toml", "revset-aliases.x = '@:'"], + ); + insta::assert_snapshot!(stdout, @r###" + @ qpvuntsmwlqt test.user@example.com 2001-02-03 04:05:07.000 +07:00 230dd059e1b0 + │ (empty) (no description set) + ~ + "###); + insta::assert_snapshot!(stderr, @r###" + The `:` revset operator is deprecated. Please switch to `::`. + "###); +} + #[test] fn test_log_with_or_without_diff() { let test_env = TestEnvironment::default(); @@ -307,7 +356,7 @@ fn test_log_shortest_accessors() { @"qpv[untsmwlqt] ba1[a30916d29]"); insta::assert_snapshot!( - render(":@", r#"change_id.shortest() ++ " " ++ commit_id.shortest() ++ "\n""#), + render("::@", r#"change_id.shortest() ++ " " ++ commit_id.shortest() ++ "\n""#), @r###" wq 03 km f7 @@ -323,7 +372,7 @@ fn test_log_shortest_accessors() { "###); insta::assert_snapshot!( - render(":@", r#"format_id(change_id) ++ " " ++ format_id(commit_id) ++ "\n""#), + render("::@", r#"format_id(change_id) ++ " " ++ format_id(commit_id) ++ "\n""#), @r###" wq[nwkozpkust] 03[f51310b83e] km[kuslswpqwq] f7[7fb1909080] @@ -341,7 +390,7 @@ fn test_log_shortest_accessors() { // Can get shorter prefixes in configured revset test_env.add_config(r#"revsets.short-prefixes = "(@----):""#); insta::assert_snapshot!( - render(":@", r#"format_id(change_id) ++ " " ++ format_id(commit_id) ++ "\n""#), + render("::@", r#"format_id(change_id) ++ " " ++ format_id(commit_id) ++ "\n""#), @r###" w[qnwkozpkust] 03[f51310b83e] km[kuslswpqwq] f[77fb1909080] @@ -359,7 +408,7 @@ fn test_log_shortest_accessors() { // Can disable short prefixes by setting to empty string test_env.add_config(r#"revsets.short-prefixes = """#); insta::assert_snapshot!( - render(":@", r#"format_id(change_id) ++ " " ++ format_id(commit_id) ++ "\n""#), + render("::@", r#"format_id(change_id) ++ " " ++ format_id(commit_id) ++ "\n""#), @r###" wq[nwkozpkust] 03[f51310b83e] km[kuslswpqwq] f7[7fb1909080]