diff --git a/CHANGELOG.md b/CHANGELOG.md index 338fc9576..77aa8f2f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,7 +63,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). deprecation from `jj 0.26`, as the community feedback was mostly negative. * The revset function `exactly(x, n)` will now evaluate `x` and error if it does - not have exactly `n` elements. + not have the range specified by `n`. * `jj util exec` now matches the exit status of the program it runs, and doesn't print anything. diff --git a/docs/revsets.md b/docs/revsets.md index 6cbe4f040..ee5db07fc 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -318,9 +318,13 @@ revsets (expressions) as arguments. set are descendants. The current implementation deals somewhat poorly with non-linear history. -* `exactly(x, count)`: Evaluates `x`, and errors if it is not of exactly size - `count`. Otherwise, returns `x`. This is useful in particular with `count=1` - when you want to ensure that some revset expression has exactly one target. +* `exactly(x, range)`: Evaluates `x`, and errors if it is not within `range`. + Otherwise, returns `x`. This is primarily useful to enforce invariants in + your revsets. + - `range` may be an integer literal, in which case `x` must always resolve + to exactly that number of revisions. e.g. `exactly(root(), 1)` + - `range` may be a half-open interval `n..x`, equivalent to `[n, x)` (`x` is excluded) + - `range` may be a closed interval `n..=x`, equivalent to `[n, x]` (`x` is included) * `merges()`: Merge commits. diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 52bb47359..e832e4660 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -994,32 +994,48 @@ impl EvaluationContext<'_> { let candidate_set = self.evaluate(candidates)?; Ok(Box::new(self.take_latest_revset(&*candidate_set, *count)?)) } - ResolvedExpression::HasSize { candidates, count } => { + ResolvedExpression::HasSize { candidates, range } => { let set = self.evaluate(candidates)?; + // Take at most count.end elements to check the range + // We take one extra to detect if there are more elements than allowed let positions: Vec<_> = set .positions() .attach(index) - .take(count.saturating_add(1)) + .take(range.end.saturating_add(1)) .try_collect()?; - if positions.len() != *count { + + if !range.contains(&positions.len()) { // https://github.com/jj-vcs/jj/pull/7252#pullrequestreview-3236259998 // in the default engine we have to evaluate the entire // revset (which may be very large) to get an exact count; // we would need to remove .take() above. instead just give // a vaguely approximate error message - let determiner = if positions.len() > *count { - "more" + let expected = if range.start == range.end.saturating_sub(1) { + // Range represents exactly one value + format!("exactly {} elements", range.start) } else { - "less" + format!( + "between {} and {} elements", + range.start, + range.end.saturating_sub(1) + ) }; + + let actual = if positions.len() > range.end { + "more".to_string() + } else { + format!("{}", positions.len()) + }; + return Err(RevsetEvaluationError::Other( format!( - "The revset was expected to have {count} elements, but {determiner} \ - were provided", + "The revset was expected to have {expected}, but {actual} were \ + provided" ) .into(), )); } + Ok(Box::new(EagerRevset { positions })) } ResolvedExpression::Coalesce(expression1, expression2) => { diff --git a/lib/src/revset.rs b/lib/src/revset.rs index dd54302ad..4950f5afc 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -298,7 +298,7 @@ pub enum RevsetExpression { Bisect(Arc), HasSize { candidates: Arc, - count: usize, + range: Range, }, Latest { candidates: Arc, @@ -533,10 +533,10 @@ impl RevsetExpression { } /// Commits in `self`, the number of which must be exactly equal to `count`. - pub fn has_size(self: &Arc, count: usize) -> Arc { + pub fn has_size(self: &Arc, range: Range) -> Arc { Arc::new(Self::HasSize { candidates: self.clone(), - count, + range, }) } @@ -736,7 +736,7 @@ pub enum ResolvedExpression { Bisect(Box), HasSize { candidates: Box, - count: usize, + range: Range, }, Latest { candidates: Box, @@ -948,10 +948,13 @@ static BUILTIN_FUNCTION_MAP: LazyLock> = LazyLock: Ok(RevsetExpression::bisect(&expression)) }); map.insert("exactly", |diagnostics, function, context| { - let ([candidates_arg, count_arg], []) = function.expect_arguments()?; + let ([candidates_arg, range_arg], []) = function.expect_arguments()?; let candidates = lower_expression(diagnostics, candidates_arg, context)?; - let count = expect_literal("integer", count_arg)?; - Ok(candidates.has_size(count)) + let range_str: String = expect_literal("string", range_arg)?; + let range = parse_range(&range_str) + .map_err(|msg| RevsetParseError::expression(msg, range_arg.span))?; + + Ok(candidates.has_size(range)) }); map.insert("merges", |_diagnostics, function, _context| { function.expect_no_arguments()?; @@ -1161,6 +1164,44 @@ pub fn expect_date_pattern( }) } +fn parse_range(s: &str) -> Result, String> { + if let Ok(n) = s.parse::() { + return Ok(n..n + 1); + } + + // inclusive range syntax (a..=b) + if let Some((start_str, end_str)) = s.split_once("..=") { + let start = start_str + .parse::() + .map_err(|_| format!("Invalid start of range: {start_str}"))?; + let end = end_str + .parse::() + .map_err(|_| format!("Invalid end of range: {end_str}"))?; + if start > end { + return Err(format!("Invalid range: start ({start}) > end ({end})")); + } + return Ok(start..end + 1); + } + + // exclusive range syntax (a..b) + if let Some((start_str, end_str)) = s.split_once("..") { + let start = start_str + .parse::() + .map_err(|_| format!("Invalid start of range: {start_str}"))?; + let end = end_str + .parse::() + .map_err(|_| format!("Invalid end of range: {end_str}"))?; + if start > end { + return Err(format!("Invalid range: start ({start}) > end ({end})")); + } + return Ok(start..end); + } + + Err(format!( + "Invalid range syntax: '{s}'. Expected format: 'n', 'n..m', or 'n..=m'" + )) +} + fn parse_remote_bookmarks_arguments( diagnostics: &mut RevsetDiagnostics, function: &FunctionCallNode, @@ -1472,10 +1513,10 @@ fn try_transform_expression( RevsetExpression::Bisect(expression) => { transform_rec(expression, pre, post)?.map(RevsetExpression::Bisect) } - RevsetExpression::HasSize { candidates, count } => { + RevsetExpression::HasSize { candidates, range } => { transform_rec(candidates, pre, post)?.map(|candidates| RevsetExpression::HasSize { candidates, - count: *count, + range: range.clone(), }) } RevsetExpression::Latest { candidates, count } => transform_rec(candidates, pre, post)? @@ -1719,11 +1760,11 @@ where let expression = folder.fold_expression(expression)?; RevsetExpression::Bisect(expression).into() } - RevsetExpression::HasSize { candidates, count } => { + RevsetExpression::HasSize { candidates, range } => { let candidates = folder.fold_expression(candidates)?; RevsetExpression::HasSize { candidates, - count: *count, + range: range.clone(), } .into() } @@ -3047,9 +3088,9 @@ impl VisibilityResolutionContext<'_> { candidates: self.resolve(candidates).into(), count: *count, }, - RevsetExpression::HasSize { candidates, count } => ResolvedExpression::HasSize { + RevsetExpression::HasSize { candidates, range } => ResolvedExpression::HasSize { candidates: self.resolve(candidates).into(), - count: *count, + range: range.clone(), }, RevsetExpression::Filter(_) | RevsetExpression::AsFilter(_) => { // Top-level filter without intersection: e.g. "~author(_)" is represented as diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 7fc6c55e7..b37772c4d 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -3062,6 +3062,7 @@ fn test_evaluate_expression_exactly() { let commit1 = write_random_commit(mut_repo); let commit2 = write_random_commit_with_parents(mut_repo, &[&commit1]); + // basics assert!(try_evaluate_expression(mut_repo, "exactly(none(), 0)").is_ok()); assert!(try_evaluate_expression(mut_repo, "exactly(none(), 1)").is_err()); assert!(try_evaluate_expression(mut_repo, &format!("exactly({}, 1)", commit1.id())).is_ok()); @@ -3077,6 +3078,11 @@ fn test_evaluate_expression_exactly() { resolve_commit_ids(mut_repo, &format!("exactly({}, 1)", commit1.id())), vec![commit1.id().clone()] ); + // ranges + assert!(try_evaluate_expression(mut_repo, r#"exactly(none(), "0..=1")"#).is_ok()); + assert!(try_evaluate_expression(mut_repo, r#"exactly(none(), "1..=1")"#).is_err()); + assert!(try_evaluate_expression(mut_repo, r#"exactly(root(), "1..2")"#).is_ok()); + assert!(try_evaluate_expression(mut_repo, r#"exactly(root()::, "1..=3")"#).is_ok()); } #[test]