martinvonz · bnjmnt4n · Dec 12, 2024 · Dec 10, 2024 · Dec 11, 2024
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   `snapshot.max-new-file-size` config option. It will print a warning and large
   files will be left untracked.
 
+* Templates now support the `>=`, `>`, `<=`, and `<` relational operators for
+  `Integer` types.
+
 ### Fixed bugs
 
 * The `$NO_COLOR` environment variable must now be non-empty to be respected.

diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs
@@ -14,6 +14,7 @@
 
 use std::any::Any;
 use std::cmp::max;
+use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::io;
 use std::rc::Rc;
@@ -427,6 +428,27 @@ impl<'repo> IntoTemplateProperty<'repo> for CommitTemplatePropertyKind<'repo> {
             (CommitTemplatePropertyKind::TreeDiff(_), _) => None,
         }
     }
+
+    fn try_into_cmp(
+        self,
+        other: Self,
+    ) -> Option<Box<dyn TemplateProperty<Output = Ordering> + 'repo>> {
+        match (self, other) {
+            (CommitTemplatePropertyKind::Core(lhs), CommitTemplatePropertyKind::Core(rhs)) => {
+                lhs.try_into_cmp(rhs)
+            }
+            (CommitTemplatePropertyKind::Core(_), _) => None,
+            (CommitTemplatePropertyKind::Commit(_), _) => None,
+            (CommitTemplatePropertyKind::CommitOpt(_), _) => None,
+            (CommitTemplatePropertyKind::CommitList(_), _) => None,
+            (CommitTemplatePropertyKind::RefName(_), _) => None,
+            (CommitTemplatePropertyKind::RefNameOpt(_), _) => None,
+            (CommitTemplatePropertyKind::RefNameList(_), _) => None,
+            (CommitTemplatePropertyKind::CommitOrChangeId(_), _) => None,
+            (CommitTemplatePropertyKind::ShortestIdPrefix(_), _) => None,
+            (CommitTemplatePropertyKind::TreeDiff(_), _) => None,
+        }
+    }
 }
 
 /// Table of functions that translate method call node of self type `T`.

diff --git a/cli/src/generic_templater.rs b/cli/src/generic_templater.rs
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::cmp::Ordering;
 use std::collections::HashMap;
 
 use crate::template_builder;
@@ -177,6 +178,19 @@ impl<'a, C: 'a> IntoTemplateProperty<'a> for GenericTemplatePropertyKind<'a, C>
             (GenericTemplatePropertyKind::Self_(_), _) => None,
         }
     }
+
+    fn try_into_cmp(
+        self,
+        other: Self,
+    ) -> Option<Box<dyn TemplateProperty<Output = Ordering> + 'a>> {
+        match (self, other) {
+            (GenericTemplatePropertyKind::Core(lhs), GenericTemplatePropertyKind::Core(rhs)) => {
+                lhs.try_into_cmp(rhs)
+            }
+            (GenericTemplatePropertyKind::Core(_), _) => None,
+            (GenericTemplatePropertyKind::Self_(_), _) => None,
+        }
+    }
 }
 
 /// Function that translates keyword (or 0-ary method call node of the self type

diff --git a/cli/src/operation_templater.rs b/cli/src/operation_templater.rs
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use std::any::Any;
+use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::io;
 
@@ -200,6 +201,18 @@ impl IntoTemplateProperty<'static> for OperationTemplatePropertyKind {
             (OperationTemplatePropertyKind::OperationId(_), _) => None,
         }
     }
+
+    fn try_into_cmp(self, other: Self) -> Option<Box<dyn TemplateProperty<Output = Ordering>>> {
+        match (self, other) {
+            (
+                OperationTemplatePropertyKind::Core(lhs),
+                OperationTemplatePropertyKind::Core(rhs),
+            ) => lhs.try_into_cmp(rhs),
+            (OperationTemplatePropertyKind::Core(_), _) => None,
+            (OperationTemplatePropertyKind::Operation(_), _) => None,
+            (OperationTemplatePropertyKind::OperationId(_), _) => None,
+        }
+    }
 }
 
 /// Table of functions that translate method call node of self type `T`.

diff --git a/cli/src/template.pest b/cli/src/template.pest
@@ -42,10 +42,23 @@ logical_or_op = { "||" }
 logical_and_op = { "&&" }
 logical_eq_op = { "==" }
 logical_ne_op = { "!=" }
+ge_op = { ">=" }
+gt_op = { ">" }
+le_op = { "<=" }
+lt_op = { "<" }
 logical_not_op = { "!" }
 negate_op = { "-" }
 prefix_ops = _{ logical_not_op | negate_op }
-infix_ops = _{ logical_or_op | logical_and_op | logical_eq_op | logical_ne_op }
+infix_ops = _{
+  logical_or_op
+  | logical_and_op
+  | logical_eq_op
+  | logical_ne_op
+  | ge_op
+  | gt_op
+  | le_op
+  | lt_op
+}
 
 function = { identifier ~ "(" ~ whitespace* ~ function_arguments ~ whitespace* ~ ")" }
 keyword_argument = { identifier ~ whitespace* ~ "=" ~ whitespace* ~ template }

diff --git a/cli/src/template_builder.rs b/cli/src/template_builder.rs
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::io;
 
@@ -165,6 +166,10 @@ pub trait IntoTemplateProperty<'a> {
 
     /// Transforms into a property that will evaluate to `self == other`.
     fn try_into_eq(self, other: Self) -> Option<Box<dyn TemplateProperty<Output = bool> + 'a>>;
+
+    /// Transforms into a property that will evaluate to an [`Ordering`].
+    fn try_into_cmp(self, other: Self)
+        -> Option<Box<dyn TemplateProperty<Output = Ordering> + 'a>>;
 }
 
 pub enum CoreTemplatePropertyKind<'a> {
@@ -294,6 +299,28 @@ impl<'a> IntoTemplateProperty<'a> for CoreTemplatePropertyKind<'a> {
             (CoreTemplatePropertyKind::ListTemplate(_), _) => None,
         }
     }
+
+    fn try_into_cmp(
+        self,
+        other: Self,
+    ) -> Option<Box<dyn TemplateProperty<Output = Ordering> + 'a>> {
+        match (self, other) {
+            (CoreTemplatePropertyKind::Integer(lhs), CoreTemplatePropertyKind::Integer(rhs)) => {
+                Some(Box::new((lhs, rhs).map(|(l, r)| l.cmp(&r))))
+            }
+            (CoreTemplatePropertyKind::String(_), _) => None,
+            (CoreTemplatePropertyKind::StringList(_), _) => None,
+            (CoreTemplatePropertyKind::Boolean(_), _) => None,
+            (CoreTemplatePropertyKind::Integer(_), _) => None,
+            (CoreTemplatePropertyKind::IntegerOpt(_), _) => None,
+            (CoreTemplatePropertyKind::Signature(_), _) => None,
+            (CoreTemplatePropertyKind::SizeHint(_), _) => None,
+            (CoreTemplatePropertyKind::Timestamp(_), _) => None,
+            (CoreTemplatePropertyKind::TimestampRange(_), _) => None,
+            (CoreTemplatePropertyKind::Template(_), _) => None,
+            (CoreTemplatePropertyKind::ListTemplate(_), _) => None,
+        }
+    }
 }
 
 /// Function that translates global function call node.
@@ -540,6 +567,13 @@ impl<'a, P: IntoTemplateProperty<'a>> Expression<P> {
     pub fn try_into_eq(self, other: Self) -> Option<Box<dyn TemplateProperty<Output = bool> + 'a>> {
         self.property.try_into_eq(other.property)
     }
+
+    pub fn try_into_cmp(
+        self,
+        other: Self,
+    ) -> Option<Box<dyn TemplateProperty<Output = Ordering> + 'a>> {
+        self.property.try_into_cmp(other.property)
+    }
 }
 
 pub struct BuildContext<'i, P> {
@@ -653,6 +687,23 @@ fn build_binary_operation<'a, L: TemplateLanguage<'a> + ?Sized>(
                 _ => unreachable!(),
             }
         }
+        BinaryOp::Ge | BinaryOp::Gt | BinaryOp::Le | BinaryOp::Lt => {
+            let lhs = build_expression(language, diagnostics, build_ctx, lhs_node)?;
+            let rhs = build_expression(language, diagnostics, build_ctx, rhs_node)?;
+            let lty = lhs.type_name();
+            let rty = rhs.type_name();
+            let out = lhs.try_into_cmp(rhs).ok_or_else(|| {
+                let message = format!(r#"Cannot compare expressions of type "{lty}" and "{rty}""#);
+                TemplateParseError::expression(message, span)
+            })?;
+            match op {
+                BinaryOp::Ge => Ok(L::wrap_boolean(out.map(|ordering| ordering.is_ge()))),
+                BinaryOp::Gt => Ok(L::wrap_boolean(out.map(|ordering| ordering.is_gt()))),
+                BinaryOp::Le => Ok(L::wrap_boolean(out.map(|ordering| ordering.is_le()))),
+                BinaryOp::Lt => Ok(L::wrap_boolean(out.map(|ordering| ordering.is_lt()))),
+                _ => unreachable!(),
+            }
+        }
     }
 }
 
@@ -1732,14 +1783,14 @@ mod tests {
         env.add_keyword("description", || L::wrap_string(Literal("".to_owned())));
         env.add_keyword("empty", || L::wrap_boolean(Literal(true)));
 
-        insta::assert_snapshot!(env.parse_err(r#"description ()"#), @r"
+        insta::assert_snapshot!(env.parse_err(r#"description ()"#), @r#"
          --> 1:13
           |
         1 | description ()
           |             ^---
           |
-          = expected <EOI>, `++`, `||`, `&&`, `==`, or `!=`
-        ");
+          = expected <EOI>, `++`, `||`, `&&`, `==`, `!=`, `>=`, `>`, `<=`, or `<`
+        "#);
 
         insta::assert_snapshot!(env.parse_err(r#"foo"#), @r###"
          --> 1:1
@@ -1839,6 +1890,14 @@ mod tests {
           |
           = Cannot compare expressions of type "String" and "Template"
         "#);
+        insta::assert_snapshot!(env.parse_err(r#"'a' > 1"#), @r#"
+         --> 1:1
+          |
+        1 | 'a' > 1
+          | ^-----^
+          |
+          = Cannot compare expressions of type "String" and "Integer"
+        "#);
 
         insta::assert_snapshot!(env.parse_err(r#"description.first_line().foo()"#), @r###"
          --> 1:26
@@ -2049,6 +2108,20 @@ mod tests {
             @"<Error: Attempt to negate with overflow>");
     }
 
+    #[test]
+    fn test_relational_operation() {
+        let env = TestTemplateEnv::new();
+
+        insta::assert_snapshot!(env.render_ok(r#"1 >= 1"#), @"true");
+        insta::assert_snapshot!(env.render_ok(r#"0 >= 1"#), @"false");
+        insta::assert_snapshot!(env.render_ok(r#"2 > 1"#), @"true");
+        insta::assert_snapshot!(env.render_ok(r#"1 > 1"#), @"false");
+        insta::assert_snapshot!(env.render_ok(r#"1 <= 1"#), @"true");
+        insta::assert_snapshot!(env.render_ok(r#"2 <= 1"#), @"false");
+        insta::assert_snapshot!(env.render_ok(r#"0 < 1"#), @"true");
+        insta::assert_snapshot!(env.render_ok(r#"1 < 1"#), @"false");
+    }
+
     #[test]
     fn test_logical_operation() {
         let mut env = TestTemplateEnv::new();

diff --git a/cli/src/template_parser.rs b/cli/src/template_parser.rs
@@ -76,6 +76,10 @@ impl Rule {
             Rule::logical_and_op => Some("&&"),
             Rule::logical_eq_op => Some("=="),
             Rule::logical_ne_op => Some("!="),
+            Rule::ge_op => Some(">="),
+            Rule::gt_op => Some(">"),
+            Rule::le_op => Some("<="),
+            Rule::lt_op => Some("<"),
             Rule::logical_not_op => Some("!"),
             Rule::negate_op => Some("-"),
             Rule::prefix_ops => None,
@@ -380,6 +384,14 @@ pub enum BinaryOp {
     LogicalEq,
     /// `!=`
     LogicalNe,
+    /// `>=`
+    Ge,
+    /// `>`
+    Gt,
+    /// `<=`
+    Le,
+    /// `<`
+    Lt,
 }
 
 pub type ExpressionNode<'i> = dsl_util::ExpressionNode<'i, ExpressionKind<'i>>;
@@ -512,6 +524,10 @@ fn parse_expression_node(pair: Pair<Rule>) -> TemplateParseResult<ExpressionNode
             .op(Op::infix(Rule::logical_and_op, Assoc::Left))
             .op(Op::infix(Rule::logical_eq_op, Assoc::Left)
                 | Op::infix(Rule::logical_ne_op, Assoc::Left))
+            .op(Op::infix(Rule::ge_op, Assoc::Left)
+                | Op::infix(Rule::gt_op, Assoc::Left)
+                | Op::infix(Rule::le_op, Assoc::Left)
+                | Op::infix(Rule::lt_op, Assoc::Left))
             .op(Op::prefix(Rule::logical_not_op) | Op::prefix(Rule::negate_op))
     });
     PRATT
@@ -533,6 +549,10 @@ fn parse_expression_node(pair: Pair<Rule>) -> TemplateParseResult<ExpressionNode
                 Rule::logical_and_op => BinaryOp::LogicalAnd,
                 Rule::logical_eq_op => BinaryOp::LogicalEq,
                 Rule::logical_ne_op => BinaryOp::LogicalNe,
+                Rule::ge_op => BinaryOp::Ge,
+                Rule::gt_op => BinaryOp::Gt,
+                Rule::le_op => BinaryOp::Le,
+                Rule::lt_op => BinaryOp::Lt,
                 r => panic!("unexpected infix operator rule {r:?}"),
             };
             let lhs = Box::new(lhs?);
@@ -861,8 +881,14 @@ mod tests {
             parse_normalized("(!(x.f())) || (!(g()))"),
         );
         assert_eq!(
-            parse_normalized("!x.f() == !x.f() || !g() != !g()"),
-            parse_normalized("((!(x.f())) == (!(x.f()))) || ((!(g())) != (!(g())))"),
+            parse_normalized("!x.f() <= !x.f()"),
+            parse_normalized("((!(x.f())) <= (!(x.f())))"),
+        );
+        assert_eq!(
+            parse_normalized("!x.f() < !x.f() == !x.f() >= !x.f() || !g() != !g()"),
+            parse_normalized(
+                "((!(x.f()) < (!(x.f()))) == ((!(x.f())) >= (!(x.f())))) || ((!(g())) != (!(g())))"
+            ),
         );
         assert_eq!(
             parse_normalized("x.f() || y == y || z"),

diff --git a/cli/tests/test_templater.rs b/cli/tests/test_templater.rs
@@ -25,15 +25,15 @@ fn test_templater_parse_error() {
     let repo_path = test_env.env_root().join("repo");
     let render_err = |template| test_env.jj_cmd_failure(&repo_path, &["log", "-T", template]);
 
-    insta::assert_snapshot!(render_err(r#"description ()"#), @r"
+    insta::assert_snapshot!(render_err(r#"description ()"#), @r#"
     Error: Failed to parse template: Syntax error
     Caused by:  --> 1:13
       |
     1 | description ()
       |             ^---
       |
-      = expected <EOI>, `++`, `||`, `&&`, `==`, or `!=`
-    ");
+      = expected <EOI>, `++`, `||`, `&&`, `==`, `!=`, `>=`, `>`, `<=`, or `<`
+    "#);
 
     // Typo
     test_env.add_config(

diff --git a/docs/templates.md b/docs/templates.md
@@ -31,6 +31,8 @@ The following operators are supported.
 * `x.f()`: Method call.
 * `-x`: Negate integer value.
 * `!x`: Logical not.
+* `x >= y`, `x > y`, `x <= y`, `x < y`: Greater than or equal/greater than/
+  lesser than or equal/lesser than. Operands must be `Integer`s.
 * `x == y`, `x != y`: Logical equal/not equal. Operands must be either
   `Boolean`, `Integer`, or `String`.
 * `x && y`: Logical and, short-circuiting.