Skip to content

Commit

Permalink
cli: drop handling of legacy revset dag range operator
Browse files Browse the repository at this point in the history
This basically reverts the change c183b94 "cli: warn when using `:` revset
operator."
  • Loading branch information
yuja committed Feb 14, 2024
1 parent 8154375 commit c73a092
Show file tree
Hide file tree
Showing 31 changed files with 66 additions and 141 deletions.
2 changes: 1 addition & 1 deletion cli/examples/custom-command/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn run_custom_command(
match command {
CustomCommand::Frobnicate(args) => {
let mut workspace_command = command_helper.workspace_helper(ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision, ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision)?;
let mut tx = workspace_command.start_transaction();
let new_commit = tx
.mut_repo()
Expand Down
99 changes: 13 additions & 86 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,12 +1091,8 @@ impl WorkspaceCommandHelper {

/// Resolve a revset to a single revision. Return an error if the revset is
/// empty or has multiple revisions.
pub fn resolve_single_rev(
&self,
revision_str: &str,
ui: &mut Ui,
) -> Result<Commit, CommandError> {
let revset_expression = self.parse_revset(revision_str, Some(ui))?;
pub fn resolve_single_rev(&self, revision_str: &str) -> Result<Commit, CommandError> {
let revset_expression = self.parse_revset(revision_str)?;
let revset = self.evaluate_revset(revset_expression.clone())?;
let mut iter = revset.iter().commits(self.repo().store()).fuse();
match (iter.next(), iter.next()) {
Expand Down Expand Up @@ -1145,12 +1141,8 @@ Set which revision the branch points to with `jj branch set {branch_name} -r <RE
}

/// Resolve a revset any number of revisions (including 0).
pub fn resolve_revset(
&self,
revision_str: &str,
ui: &mut Ui,
) -> Result<Vec<Commit>, CommandError> {
let revset_expression = self.parse_revset(revision_str, Some(ui))?;
pub fn resolve_revset(&self, revision_str: &str) -> Result<Vec<Commit>, CommandError> {
let revset_expression = self.parse_revset(revision_str)?;
let revset = self.evaluate_revset(revset_expression)?;
Ok(revset.iter().commits(self.repo().store()).try_collect()?)
}
Expand All @@ -1161,13 +1153,12 @@ Set which revision the branch points to with `jj branch set {branch_name} -r <RE
pub fn resolve_revset_default_single(
&self,
revision_str: &str,
ui: &mut Ui,
) -> Result<Vec<Commit>, CommandError> {
// TODO: Let pest parse the prefix too once we've dropped support for `:`
if let Some(revision_str) = revision_str.strip_prefix("all:") {
self.resolve_revset(revision_str, ui)
self.resolve_revset(revision_str)
} else {
self.resolve_single_rev(revision_str, ui)
self.resolve_single_rev(revision_str)
.map_err(|err| match err {
CommandError::UserError { err, hint } => CommandError::UserError {
err,
Expand All @@ -1187,65 +1178,8 @@ Set which revision the branch points to with `jj branch set {branch_name} -r <RE
pub fn parse_revset(
&self,
revision_str: &str,
ui: Option<&mut Ui>,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
let expression = revset::parse(revision_str, &self.revset_parse_context())?;
if let Some(ui) = ui {
fn has_legacy_rule(expression: &Rc<RevsetExpression>) -> 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))
}

Expand Down Expand Up @@ -1293,12 +1227,9 @@ Set which revision the branch points to with `jj branch set {branch_name} -r <RE
.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, None).map_err(|err| {
CommandError::ConfigError(format!(
"Invalid `revsets.short-prefixes`: {err}"
))
})?;
let disambiguation_revset = self.parse_revset(&revset_string).map_err(|err| {
CommandError::ConfigError(format!("Invalid `revsets.short-prefixes`: {err}"))
})?;
context = context.disambiguate_within(disambiguation_revset);
}
Ok(context)
Expand Down Expand Up @@ -1373,7 +1304,7 @@ Set which revision the branch points to with `jj branch set {branch_name} -r <RE
r#"The `revset-aliases.immutable_heads()` function must be declared without arguments."#,
));
}
let immutable_heads_revset = self.parse_revset(immutable_heads_str, None)?;
let immutable_heads_revset = self.parse_revset(immutable_heads_str)?;
let immutable_revset = immutable_heads_revset
.ancestors()
.union(&RevsetExpression::commit(
Expand Down Expand Up @@ -2075,11 +2006,9 @@ pub fn parse_string_pattern(src: &str) -> Result<StringPattern, StringPatternPar
/// that take multiple parents.
pub fn resolve_all_revs(
workspace_command: &WorkspaceCommandHelper,
ui: &mut Ui,
revisions: &[RevisionArg],
) -> Result<IndexSet<Commit>, CommandError> {
let commits =
resolve_multiple_nonempty_revsets_default_single(workspace_command, ui, revisions)?;
let commits = resolve_multiple_nonempty_revsets_default_single(workspace_command, revisions)?;
let root_commit_id = workspace_command.repo().store().root_commit_id();
if commits.len() >= 2 && commits.iter().any(|c| c.id() == root_commit_id) {
Err(user_error("Cannot merge with root revision"))
Expand Down Expand Up @@ -2118,11 +2047,10 @@ fn load_revset_aliases(
pub fn resolve_multiple_nonempty_revsets(
revision_args: &[RevisionArg],
workspace_command: &WorkspaceCommandHelper,
ui: &mut Ui,
) -> Result<IndexSet<Commit>, CommandError> {
let mut acc = IndexSet::new();
for revset in revision_args {
let revisions = workspace_command.resolve_revset(revset, ui)?;
let revisions = workspace_command.resolve_revset(revset)?;
workspace_command.check_non_empty(&revisions)?;
acc.extend(revisions);
}
Expand All @@ -2131,12 +2059,11 @@ pub fn resolve_multiple_nonempty_revsets(

pub fn resolve_multiple_nonempty_revsets_default_single(
workspace_command: &WorkspaceCommandHelper,
ui: &mut Ui,
revisions: &[RevisionArg],
) -> Result<IndexSet<Commit>, CommandError> {
let mut all_commits = IndexSet::new();
for revision_str in revisions {
let commits = workspace_command.resolve_revset_default_single(revision_str, ui)?;
let commits = workspace_command.resolve_revset_default_single(revision_str)?;
workspace_command.check_non_empty(&commits)?;
for commit in commits {
let commit_hash = short_commit_hash(commit.id());
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/abandon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(crate) 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, ui)?;
let to_abandon = resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command)?;
workspace_command.check_rewritable(to_abandon.iter())?;
let mut tx = workspace_command.start_transaction();
for commit in &to_abandon {
Expand Down
4 changes: 2 additions & 2 deletions cli/src/commands/backout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ pub(crate) 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, ui)?;
let commit_to_back_out = workspace_command.resolve_single_rev(&args.revision)?;
let mut parents = vec![];
for revision_str in &args.destination {
let destination = workspace_command.resolve_single_rev(revision_str, ui)?;
let destination = workspace_command.resolve_single_rev(revision_str)?;
parents.push(destination);
}
let mut tx = workspace_command.start_transaction();
Expand Down
10 changes: 5 additions & 5 deletions cli/src/commands/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ pub(crate) fn cmd_bench(
match subcommand {
BenchCommand::CommonAncestors(args) => {
let workspace_command = command.workspace_helper(ui)?;
let commit1 = workspace_command.resolve_single_rev(&args.revision1, ui)?;
let commit2 = workspace_command.resolve_single_rev(&args.revision2, ui)?;
let commit1 = workspace_command.resolve_single_rev(&args.revision1)?;
let commit2 = workspace_command.resolve_single_rev(&args.revision2)?;
let index = workspace_command.repo().index();
let routine =
|| index.common_ancestors(&[commit1.id().clone()], &[commit2.id().clone()]);
Expand All @@ -146,8 +146,8 @@ pub(crate) fn cmd_bench(
}
BenchCommand::IsAncestor(args) => {
let workspace_command = command.workspace_helper(ui)?;
let ancestor_commit = workspace_command.resolve_single_rev(&args.ancestor, ui)?;
let descendant_commit = workspace_command.resolve_single_rev(&args.descendant, ui)?;
let ancestor_commit = workspace_command.resolve_single_rev(&args.ancestor)?;
let descendant_commit = workspace_command.resolve_single_rev(&args.descendant)?;
let index = workspace_command.repo().index();
let routine = || index.is_ancestor(ancestor_commit.id(), descendant_commit.id());
run_bench(
Expand Down Expand Up @@ -201,7 +201,7 @@ fn bench_revset<M: Measurement>(
revset: &str,
) -> Result<(), CommandError> {
writeln!(ui.stderr(), "----------Testing revset: {revset}----------")?;
let expression = workspace_command.parse_revset(revset, Some(ui))?;
let expression = workspace_command.parse_revset(revset)?;
// Time both evaluation and iteration.
let routine = |workspace_command: &WorkspaceCommandHelper, expression| {
workspace_command
Expand Down
6 changes: 3 additions & 3 deletions cli/src/commands/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,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("@"), ui)?;
workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?;
let view = workspace_command.repo().view();
let branch_names = &args.names;
if let Some(branch_name) = branch_names
Expand Down Expand Up @@ -333,7 +333,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("@"), ui)?;
workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?;
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
Expand Down Expand Up @@ -614,7 +614,7 @@ fn cmd_branch_list(
let filter_expressions: Vec<_> = args
.revisions
.iter()
.map(|revision_str| workspace_command.parse_revset(revision_str, Some(ui)))
.map(|revision_str| workspace_command.parse_revset(revision_str))
.try_collect()?;
let filter_expression = RevsetExpression::union_all(&filter_expressions);
// Intersects with the set of local branch targets to minimize the lookup space.
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/cat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub(crate) fn cmd_cat(
args: &CatArgs,
) -> Result<(), CommandError> {
let workspace_command = command.workspace_helper(ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision, ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision)?;
let tree = commit.tree()?;
let path = workspace_command.parse_file_path(&args.path)?;
let repo = workspace_command.repo();
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/checkout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub(crate) fn cmd_checkout(
"warning: `jj checkout` will be removed in a future version, and this will be a hard error"
)?;
let mut workspace_command = command.workspace_helper(ui)?;
let target = workspace_command.resolve_single_rev(&args.revision, ui)?;
let target = workspace_command.resolve_single_rev(&args.revision)?;
let mut tx = workspace_command.start_transaction();
let commit_builder = tx
.mut_repo()
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/chmod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub(crate) fn cmd_chmod(
.iter()
.map(|path| workspace_command.parse_file_path(path))
.try_collect()?;
let commit = workspace_command.resolve_single_rev(&args.revision, ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision)?;
workspace_command.check_rewritable([&commit])?;

let mut tx = workspace_command.start_transaction();
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ fn cmd_debug_tree(
args: &DebugTreeArgs,
) -> Result<(), CommandError> {
let workspace_command = command.workspace_helper(ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision, ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision)?;
let tree = commit.tree()?;
let matcher = workspace_command.matcher_from_values(&args.paths)?;
for (path, value) in tree.entries_matching(matcher.as_ref()) {
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/describe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub(crate) fn cmd_describe(
args: &DescribeArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision, ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision)?;
workspace_command.check_rewritable([&commit])?;
let description = if args.stdin {
let mut buffer = String::new();
Expand Down
6 changes: 3 additions & 3 deletions cli/src/commands/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ 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("@"), ui)?;
let from = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?;
from_tree = from.tree()?;
let to = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"), ui)?;
let to = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"))?;
to_tree = to.tree()?;
} else {
let commit =
workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"), ui)?;
workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?;
let parents = commit.parents();
from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &parents)?;
to_tree = commit.tree()?
Expand Down
7 changes: 3 additions & 4 deletions cli/src/commands/diffedit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,16 @@ 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("@"), ui)?;
target_commit = workspace_command.resolve_single_rev(args.to.as_deref().unwrap_or("@"))?;
base_commits =
vec![workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"), ui)?];
vec![workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?];
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("@"), ui)?;
workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?;
base_commits = target_commit.parents();
diff_description = "The diff initially shows the commit's changes.".to_string();
};
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/duplicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub(crate) fn cmd_duplicate(
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let to_duplicate: IndexSet<Commit> =
resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command, ui)?;
resolve_multiple_nonempty_revsets(&args.revisions, &workspace_command)?;
if to_duplicate
.iter()
.any(|commit| commit.id() == workspace_command.repo().store().root_commit_id())
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub(crate) fn cmd_edit(
args: &EditArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let new_commit = workspace_command.resolve_single_rev(&args.revision, ui)?;
let new_commit = workspace_command.resolve_single_rev(&args.revision)?;
workspace_command.check_rewritable([&new_commit])?;
if workspace_command.get_wc_commit_id() == Some(new_commit.id()) {
writeln!(ui.stderr(), "Already editing that commit")?;
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub(crate) fn cmd_files(
args: &FilesArgs,
) -> Result<(), CommandError> {
let workspace_command = command.workspace_helper(ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision, ui)?;
let commit = workspace_command.resolve_single_rev(&args.revision)?;
let tree = commit.tree()?;
let matcher = workspace_command.matcher_from_values(&args.paths)?;
ui.request_pager();
Expand Down
Loading

0 comments on commit c73a092

Please sign in to comment.