From 7cd01b27a756be6cd22a77dea8ace49cf2e65fc0 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 22 Dec 2022 19:34:08 +0900
Subject: [PATCH] revset: parse hg-like '-'/'+' infix operators and show hint

Suggested by @arxanas.

Actually, it's easier to support these infix ops than erroring out, but I
don't want to make revset syntax more cryptic. "x- y" can't be handled by
this rule because "x-" is parsed as a parents expression.
---
 lib/src/revset.pest         |  4 ++-
 lib/src/revset.rs           | 49 +++++++++++++++++++++++++++++++++++--
 tests/test_revset_output.rs | 10 ++++++++
 3 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/lib/src/revset.pest b/lib/src/revset.pest
index 5f8e98ee8..e6af7b2c9 100644
--- a/lib/src/revset.pest
+++ b/lib/src/revset.pest
@@ -41,7 +41,9 @@ negate_op = { "~" }
 union_op = { "|" }
 intersection_op = { "&" }
 difference_op = { "~" }
-infix_op = _{ union_op | intersection_op | difference_op }
+compat_add_op = { "+" }
+compat_sub_op = { "-" }
+infix_op = _{ union_op | intersection_op | difference_op | compat_add_op | compat_sub_op }
 
 function_name = @{ (ASCII_ALPHANUMERIC | "_")+ }
 function_arguments = {
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 12245cab4..fc5426e93 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -215,6 +215,12 @@ pub struct RevsetParseError {
 pub enum RevsetParseErrorKind {
     #[error("Syntax error")]
     SyntaxError,
+    #[error("'{op}' is not an infix operator (Did you mean '{similar_op}' for {description}?)")]
+    NotInfixOperator {
+        op: String,
+        similar_op: String,
+        description: String,
+    },
     #[error("Revset function \"{0}\" doesn't exist")]
     NoSuchFunction(String),
     #[error("Invalid arguments to revset function \"{name}\": {message}")]
@@ -695,11 +701,28 @@ fn parse_expression_rule(
     pairs: Pairs<Rule>,
     state: ParseState,
 ) -> Result<Rc<RevsetExpression>, RevsetParseError> {
+    fn not_infix_op(
+        op: &Pair<Rule>,
+        similar_op: impl Into<String>,
+        description: impl Into<String>,
+    ) -> RevsetParseError {
+        RevsetParseError::with_span(
+            RevsetParseErrorKind::NotInfixOperator {
+                op: op.as_str().to_owned(),
+                similar_op: similar_op.into(),
+                description: description.into(),
+            },
+            op.as_span(),
+        )
+    }
+
     static PRATT: Lazy<PrattParser<Rule>> = Lazy::new(|| {
         PrattParser::new()
-            .op(Op::infix(Rule::union_op, Assoc::Left))
+            .op(Op::infix(Rule::union_op, Assoc::Left)
+                | Op::infix(Rule::compat_add_op, Assoc::Left))
             .op(Op::infix(Rule::intersection_op, Assoc::Left)
-                | Op::infix(Rule::difference_op, Assoc::Left))
+                | Op::infix(Rule::difference_op, Assoc::Left)
+                | Op::infix(Rule::compat_sub_op, Assoc::Left))
             .op(Op::prefix(Rule::negate_op))
             // Ranges can't be nested without parentheses. Associativity doesn't matter.
             .op(Op::infix(Rule::dag_range_op, Assoc::Left) | Op::infix(Rule::range_op, Assoc::Left))
@@ -724,8 +747,10 @@ fn parse_expression_rule(
         })
         .map_infix(|lhs, op, rhs| match op.as_rule() {
             Rule::union_op => Ok(lhs?.union(&rhs?)),
+            Rule::compat_add_op => Err(not_infix_op(&op, "|", "union")),
             Rule::intersection_op => Ok(lhs?.intersection(&rhs?)),
             Rule::difference_op => Ok(lhs?.minus(&rhs?)),
+            Rule::compat_sub_op => Err(not_infix_op(&op, "~", "difference")),
             Rule::dag_range_op => Ok(lhs?.dag_range_to(&rhs?)),
             Rule::range_op => Ok(lhs?.range(&rhs?)),
             r => panic!("unexpected infix operator rule {r:?}"),
@@ -2288,6 +2313,26 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_parse_revset_compat_operator() {
+        assert_eq!(
+            parse("foo + bar"),
+            Err(RevsetParseErrorKind::NotInfixOperator {
+                op: "+".to_owned(),
+                similar_op: "|".to_owned(),
+                description: "union".to_owned(),
+            })
+        );
+        assert_eq!(
+            parse("foo - bar"),
+            Err(RevsetParseErrorKind::NotInfixOperator {
+                op: "-".to_owned(),
+                similar_op: "~".to_owned(),
+                description: "difference".to_owned(),
+            })
+        );
+    }
+
     #[test]
     fn test_parse_revset_operator_combinations() {
         let foo_symbol = RevsetExpression::symbol("foo".to_string());
diff --git a/tests/test_revset_output.rs b/tests/test_revset_output.rs
index 666deb1d0..40b62ea1a 100644
--- a/tests/test_revset_output.rs
+++ b/tests/test_revset_output.rs
@@ -31,6 +31,16 @@ fn test_syntax_error() {
       |
       = expected dag_range_pre_op, range_pre_op, negate_op, or primary
     "###);
+
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "x - y"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: Failed to parse revset:  --> 1:3
+      |
+    1 | x - y
+      |   ^
+      |
+      = '-' is not an infix operator (Did you mean '~' for difference?)
+    "###);
 }
 
 #[test]