From ae91adbaf4e4c9b148660353582b0d5a07fd48ea Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 1 Apr 2024 16:24:57 +0900 Subject: [PATCH] cli: preserve RevisionArg type as much as possible Just for a bit of type safety. --- cli/src/commands/branch.rs | 4 ++-- cli/src/commands/debug.rs | 4 ++-- cli/src/commands/diff.rs | 10 ++++++---- cli/src/commands/diffedit.rs | 10 ++++++---- cli/src/commands/interdiff.rs | 5 +++-- cli/src/commands/move.rs | 6 ++++-- cli/src/commands/rebase.rs | 10 +++++----- cli/src/commands/restore.rs | 9 +++++---- cli/src/commands/squash.rs | 9 +++++---- 9 files changed, 38 insertions(+), 29 deletions(-) diff --git a/cli/src/commands/branch.rs b/cli/src/commands/branch.rs index 7892fbc96..78f7fd237 100644 --- a/cli/src/commands/branch.rs +++ b/cli/src/commands/branch.rs @@ -240,7 +240,7 @@ fn cmd_branch_create( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let target_commit = - workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + workspace_command.resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?; let view = workspace_command.repo().view(); let branch_names = &args.names; if let Some(branch_name) = branch_names @@ -341,7 +341,7 @@ fn cmd_branch_set( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let target_commit = - workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + workspace_command.resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?; let repo = workspace_command.repo().as_ref(); let is_fast_forward = |old_target: &RefTarget| { // Strictly speaking, "all" old targets should be ancestors, but we allow diff --git a/cli/src/commands/debug.rs b/cli/src/commands/debug.rs index 1c6589783..d408e379f 100644 --- a/cli/src/commands/debug.rs +++ b/cli/src/commands/debug.rs @@ -313,8 +313,8 @@ fn cmd_debug_tree( let tree = store.get_tree(&dir, &tree_id)?; MergedTree::resolved(tree) } else { - let commit = - workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + let commit = workspace_command + .resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?; commit.tree()? }; let matcher = workspace_command.matcher_from_values(&args.paths)?; diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 4677b5071..8a589f5c6 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -63,13 +63,15 @@ pub(crate) fn cmd_diff( 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_ref().unwrap_or(&RevisionArg::AT))?; 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_ref().unwrap_or(&RevisionArg::AT))?; to_tree = to.tree()?; } else { - let commit = - workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + let commit = workspace_command + .resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?; let parents = commit.parents(); from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &parents)?; to_tree = commit.tree()? diff --git a/cli/src/commands/diffedit.rs b/cli/src/commands/diffedit.rs index d97d6aed3..7d1c307d0 100644 --- a/cli/src/commands/diffedit.rs +++ b/cli/src/commands/diffedit.rs @@ -66,16 +66,18 @@ pub(crate) 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_ref().unwrap_or(&RevisionArg::AT))?; base_commits = - vec![workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?]; + vec![workspace_command + .resolve_single_rev(args.from.as_ref().unwrap_or(&RevisionArg::AT))?]; 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("@"))?; + target_commit = workspace_command + .resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?; base_commits = target_commit.parents(); diff_description = "The diff initially shows the commit's changes.".to_string(); }; diff --git a/cli/src/commands/interdiff.rs b/cli/src/commands/interdiff.rs index f551a87b1..09897c05f 100644 --- a/cli/src/commands/interdiff.rs +++ b/cli/src/commands/interdiff.rs @@ -49,8 +49,9 @@ pub(crate) 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_ref().unwrap_or(&RevisionArg::AT))?; + let to = workspace_command.resolve_single_rev(args.to.as_ref().unwrap_or(&RevisionArg::AT))?; let from_tree = rebase_to_dest_parent(workspace_command.repo().as_ref(), &from, &to)?; let to_tree = to.tree()?; diff --git a/cli/src/commands/move.rs b/cli/src/commands/move.rs index 1599f2cc2..c67ad628b 100644 --- a/cli/src/commands/move.rs +++ b/cli/src/commands/move.rs @@ -71,8 +71,10 @@ pub(crate) fn cmd_move( "`jj move` will be removed in a future version, and this will be a hard error" )?; let mut workspace_command = command.workspace_helper(ui)?; - let source = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?; - let destination = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"))?; + let source = + workspace_command.resolve_single_rev(args.from.as_ref().unwrap_or(&RevisionArg::AT))?; + let destination = + workspace_command.resolve_single_rev(args.to.as_ref().unwrap_or(&RevisionArg::AT))?; if source.id() == destination.id() { return Err(user_error("Source and destination cannot be the same.")); } diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index 444e52cdd..e8de4bfe9 100644 --- a/cli/src/commands/rebase.rs +++ b/cli/src/commands/rebase.rs @@ -197,7 +197,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets .resolve_some_revsets_default_single(&args.destination)? .into_iter() .collect_vec(); - if let Some(rev_str) = &args.revision { + if let Some(rev_arg) = &args.revision { assert_eq!( // In principle, `-r --skip-empty` could mean to abandon the `-r` // commit if it becomes empty. This seems internally consistent with @@ -218,7 +218,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets command.settings(), &mut workspace_command, &new_parents, - rev_str, + rev_arg, )?; } else if !args.source.is_empty() { let source_commits = workspace_command.resolve_some_revsets_default_single(&args.source)?; @@ -232,7 +232,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets )?; } else { let branch_commits = if args.branch.is_empty() { - IndexSet::from([workspace_command.resolve_single_rev("@")?]) + IndexSet::from([workspace_command.resolve_single_rev(&RevisionArg::AT)?]) } else { workspace_command.resolve_some_revsets_default_single(&args.branch)? }; @@ -350,9 +350,9 @@ fn rebase_revision( settings: &UserSettings, workspace_command: &mut WorkspaceCommandHelper, new_parents: &[Commit], - rev_str: &str, + rev_arg: &RevisionArg, ) -> Result<(), CommandError> { - let old_commit = workspace_command.resolve_single_rev(rev_str)?; + let old_commit = workspace_command.resolve_single_rev(rev_arg)?; workspace_command.check_rewritable([&old_commit])?; if new_parents.contains(&old_commit) { return Err(user_error(format!( diff --git a/cli/src/commands/restore.rs b/cli/src/commands/restore.rs index b3a599096..03fc4820e 100644 --- a/cli/src/commands/restore.rs +++ b/cli/src/commands/restore.rs @@ -86,13 +86,14 @@ pub(crate) fn cmd_restore( )); } 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_ref().unwrap_or(&RevisionArg::AT))?; from_tree = workspace_command - .resolve_single_rev(args.from.as_deref().unwrap_or("@"))? + .resolve_single_rev(args.from.as_ref().unwrap_or(&RevisionArg::AT))? .tree()?; } else { - to_commit = - workspace_command.resolve_single_rev(args.changes_in.as_deref().unwrap_or("@"))?; + to_commit = workspace_command + .resolve_single_rev(args.changes_in.as_ref().unwrap_or(&RevisionArg::AT))?; from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &to_commit.parents())?; } workspace_command.check_rewritable([&to_commit])?; diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs index 04c8be95f..36b3c60c6 100644 --- a/cli/src/commands/squash.rs +++ b/cli/src/commands/squash.rs @@ -85,10 +85,11 @@ pub(crate) fn cmd_squash( let destination; if args.from.is_some() || args.into.is_some() { sources = workspace_command - .parse_revset(args.from.as_deref().unwrap_or("@"))? + .parse_revset(args.from.as_ref().unwrap_or(&RevisionArg::AT))? .evaluate_to_commits()? .try_collect()?; - destination = workspace_command.resolve_single_rev(args.into.as_deref().unwrap_or("@"))?; + destination = + workspace_command.resolve_single_rev(args.into.as_ref().unwrap_or(&RevisionArg::AT))?; if sources.iter().any(|source| source.id() == destination.id()) { return Err(user_error("Source and destination cannot be the same")); } @@ -97,8 +98,8 @@ pub(crate) fn cmd_squash( // a little faster. sources.reverse(); } else { - let source = - workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + let source = workspace_command + .resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?; let mut parents = source.parents(); if parents.len() != 1 { return Err(user_error("Cannot squash merge commits"));