diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cecb229d8..28204c9c8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 move` to ensure that the target branch already exists. [#3584](https://github.com/martinvonz/jj/issues/3584) +* The `description(pattern)`, `author(pattern)` and `committer(pattern)` revsets + now match case‐insensitively by default; to override this, pass + `case_sensitive=true`. `mine()` uses case‐insensitive matching + unconditionally. Only ASCII case folding is currently implemented, but this + will likely change in the future. + ### Deprecations * Replacing `-l` shorthand for `--limit` with `-n` in `jj log`, `jj op log` and `jj obslog`. diff --git a/docs/revsets.md b/docs/revsets.md index b7676ec561..9203535a9c 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -245,17 +245,17 @@ revsets (expressions) as arguments. * `merges()`: Merge commits. -* `description(pattern)`: Commits that have a description matching the given - [string pattern](#string-patterns). +* `description(pattern, [case_sensitive=false])`: Commits that have a + description matching the given [string pattern](#string-patterns). -* `author(pattern)`: Commits with the author's name or email matching the given - [string pattern](#string-patterns). +* `author(pattern, [case_sensitive=false])`: Commits with the author's name or + email matching the given [string pattern](#string-patterns). * `mine()`: Commits where the author's email matches the email of the current user. -* `committer(pattern)`: Commits with the committer's name or email matching the -given [string pattern](#string-patterns). +* `committer(pattern, [case_sensitive=false])`: Commits with the committer's + name or email matching the given [string pattern](#string-patterns). * `empty()`: Commits modifying no files. This also includes `merges()` without user modifications and `root()`. diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 13a93e79a0..037d8f3408 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -1041,32 +1041,35 @@ fn build_predicate_fn( parent_count_range.contains(&entry.num_parents()) }) } - RevsetFilterPredicate::Description(pattern) => { + RevsetFilterPredicate::Description(pattern, case_sensitivity) => { let pattern = pattern.clone(); + let &case_sensitivity = case_sensitivity; box_pure_predicate_fn(move |index, pos| { let entry = index.entry_by_pos(pos); let commit = store.get_commit(&entry.commit_id()).unwrap(); - pattern.matches(commit.description()) + pattern.matches_with(commit.description(), case_sensitivity) }) } - RevsetFilterPredicate::Author(pattern) => { + RevsetFilterPredicate::Author(pattern, case_sensitivity) => { let pattern = pattern.clone(); + let &case_sensitivity = case_sensitivity; // TODO: Make these functions that take a needle to search for accept some - // syntax for specifying whether it's a regex and whether it's - // case-sensitive. + // syntax for specifying whether it's a regex. box_pure_predicate_fn(move |index, pos| { let entry = index.entry_by_pos(pos); let commit = store.get_commit(&entry.commit_id()).unwrap(); - pattern.matches(&commit.author().name) || pattern.matches(&commit.author().email) + pattern.matches_with(&commit.author().name, case_sensitivity) + || pattern.matches_with(&commit.author().email, case_sensitivity) }) } - RevsetFilterPredicate::Committer(pattern) => { + RevsetFilterPredicate::Committer(pattern, case_sensitivity) => { let pattern = pattern.clone(); + let &case_sensitivity = case_sensitivity; box_pure_predicate_fn(move |index, pos| { let entry = index.entry_by_pos(pos); let commit = store.get_commit(&entry.commit_id()).unwrap(); - pattern.matches(&commit.committer().name) - || pattern.matches(&commit.committer().email) + pattern.matches_with(&commit.committer().name, case_sensitivity) + || pattern.matches_with(&commit.committer().email, case_sensitivity) }) } RevsetFilterPredicate::File(expr) => { diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 079a883e24..0a9be892af 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -42,7 +42,7 @@ pub use crate::revset_parser::{ RevsetParseError, RevsetParseErrorKind, UnaryOp, }; use crate::store::Store; -use crate::str_util::StringPattern; +use crate::str_util::{CaseSensitivity, StringPattern}; use crate::{dsl_util, revset_parser}; /// Error occurred during symbol resolution. @@ -126,11 +126,11 @@ pub enum RevsetFilterPredicate { /// Commits with number of parents in the range. ParentCount(Range), /// Commits with description containing the needle. - Description(StringPattern), + Description(StringPattern, CaseSensitivity), /// Commits with author's name or email containing the needle. - Author(StringPattern), + Author(StringPattern, CaseSensitivity), /// Commits with committer's name or email containing the needle. - Committer(StringPattern), + Committer(StringPattern, CaseSensitivity), /// Commits modifying the paths specified by the fileset. File(FilesetExpression), /// Commits with conflicts @@ -672,30 +672,30 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: )) }); map.insert("description", |function, _context| { - let [arg] = function.expect_exact_arguments()?; - let pattern = expect_string_pattern(arg)?; + let (pattern, case_sensitivity) = expect_pattern_args(function)?; Ok(RevsetExpression::filter( - RevsetFilterPredicate::Description(pattern), + RevsetFilterPredicate::Description(pattern, case_sensitivity), )) }); map.insert("author", |function, _context| { - let [arg] = function.expect_exact_arguments()?; - let pattern = expect_string_pattern(arg)?; + let (pattern, case_sensitivity) = expect_pattern_args(function)?; Ok(RevsetExpression::filter(RevsetFilterPredicate::Author( pattern, + case_sensitivity, ))) }); map.insert("mine", |function, context| { function.expect_no_arguments()?; Ok(RevsetExpression::filter(RevsetFilterPredicate::Author( StringPattern::Exact(context.user_email.to_owned()), + CaseSensitivity::Insensitive, ))) }); map.insert("committer", |function, _context| { - let [arg] = function.expect_exact_arguments()?; - let pattern = expect_string_pattern(arg)?; + let (pattern, case_sensitivity) = expect_pattern_args(function)?; Ok(RevsetExpression::filter(RevsetFilterPredicate::Committer( pattern, + case_sensitivity, ))) }); map.insert("empty", |function, _context| { @@ -730,6 +730,24 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: map }); +fn expect_pattern_args( + function: &FunctionCallNode, +) -> Result<(StringPattern, CaseSensitivity), RevsetParseError> { + let ([pattern_arg], [case_sensitive_opt_arg]) = + function.expect_named_arguments(&["", "case_sensitive"])?; + let pattern = expect_string_pattern(pattern_arg)?; + let case_sensitivity = if case_sensitive_opt_arg + .map(|arg| expect_literal("boolean", arg)) + .transpose()? + .unwrap_or(false) + { + CaseSensitivity::Sensitive + } else { + CaseSensitivity::Insensitive + }; + Ok((pattern, case_sensitivity)) +} + pub fn expect_file_pattern( node: &ExpressionNode, path_converter: &RepoPathUiConverter, @@ -2293,7 +2311,14 @@ mod tests { @r###"Expression("Expected expression of string pattern")"###); insta::assert_debug_snapshot!( parse(r#"author("foo@")"#).unwrap(), - @r###"Filter(Author(Substring("foo@")))"###); + @r###" + Filter( + Author( + Substring("foo@"), + Insensitive, + ), + ) + "###); // Parse a single symbol insta::assert_debug_snapshot!( parse("foo").unwrap(), @@ -2508,20 +2533,48 @@ mod tests { assert!(parse("root(a)").is_err()); insta::assert_debug_snapshot!( parse(r#"description("")"#).unwrap(), - @r###"Filter(Description(Substring("")))"###); + @r###" + Filter( + Description( + Substring(""), + Insensitive, + ), + ) + "###); insta::assert_debug_snapshot!( parse("description(foo)").unwrap(), - @r###"Filter(Description(Substring("foo")))"###); + @r###" + Filter( + Description( + Substring("foo"), + Insensitive, + ), + ) + "###); insta::assert_debug_snapshot!( parse("description(visible_heads())").unwrap_err(), @r###"Expression("Expected expression of string pattern")"###); insta::assert_debug_snapshot!( parse("description(\"(foo)\")").unwrap(), - @r###"Filter(Description(Substring("(foo)")))"###); + @r###" + Filter( + Description( + Substring("(foo)"), + Insensitive, + ), + ) + "###); assert!(parse("mine(foo)").is_err()); insta::assert_debug_snapshot!( parse("mine()").unwrap(), - @r###"Filter(Author(Exact("test.user@example.com")))"###); + @r###" + Filter( + Author( + Exact("test.user@example.com"), + Insensitive, + ), + ) + "###); insta::assert_debug_snapshot!( parse_with_workspace("empty()", &WorkspaceId::default()).unwrap(), @"NotIn(Filter(File(All)))"); @@ -2631,12 +2684,26 @@ mod tests { // Alias can be substituted to string pattern. insta::assert_debug_snapshot!( parse_with_aliases("author(A)", [("A", "a")]).unwrap(), - @r###"Filter(Author(Substring("a")))"###); + @r###" + Filter( + Author( + Substring("a"), + Insensitive, + ), + ) + "###); // However, parentheses are required because top-level x:y is parsed as // program modifier. insta::assert_debug_snapshot!( parse_with_aliases("author(A)", [("A", "(exact:a)")]).unwrap(), - @r###"Filter(Author(Exact("a")))"###); + @r###" + Filter( + Author( + Exact("a"), + Insensitive, + ), + ) + "###); // Sub-expression alias cannot be substituted to modifier expression. insta::assert_debug_snapshot!( @@ -2653,8 +2720,18 @@ mod tests { insta::assert_debug_snapshot!( parse_with_aliases("F(a)", [("F(x)", "author(x)|committer(x)")]).unwrap(), @r###" Union( - Filter(Author(Substring("a"))), - Filter(Committer(Substring("a"))), + Filter( + Author( + Substring("a"), + Insensitive, + ), + ), + Filter( + Committer( + Substring("a"), + Insensitive, + ), + ), ) "###); } @@ -2995,7 +3072,12 @@ mod tests { CommitRef(Symbol("baz")), CommitRef(Symbol("bar")), ), - Filter(Author(Substring("foo"))), + Filter( + Author( + Substring("foo"), + Insensitive, + ), + ), ) "###); @@ -3004,7 +3086,12 @@ mod tests { optimize(parse("~foo & author(bar)").unwrap()), @r###" Intersection( NotIn(CommitRef(Symbol("foo"))), - Filter(Author(Substring("bar"))), + Filter( + Author( + Substring("bar"), + Insensitive, + ), + ), ) "###); insta::assert_debug_snapshot!( @@ -3013,7 +3100,12 @@ mod tests { NotIn(CommitRef(Symbol("foo"))), AsFilter( Union( - Filter(Author(Substring("bar"))), + Filter( + Author( + Substring("bar"), + Insensitive, + ), + ), CommitRef(Symbol("baz")), ), ), @@ -3025,7 +3117,12 @@ mod tests { optimize(parse("author(foo) ~ bar").unwrap()), @r###" Intersection( NotIn(CommitRef(Symbol("bar"))), - Filter(Author(Substring("foo"))), + Filter( + Author( + Substring("foo"), + Insensitive, + ), + ), ) "###); } @@ -3036,25 +3133,52 @@ mod tests { let _guard = settings.bind_to_scope(); insta::assert_debug_snapshot!( - optimize(parse("author(foo)").unwrap()), @r###"Filter(Author(Substring("foo")))"###); + optimize(parse("author(foo)").unwrap()), @r###" + Filter( + Author( + Substring("foo"), + Insensitive, + ), + ) + "###); insta::assert_debug_snapshot!(optimize(parse("foo & description(bar)").unwrap()), @r###" Intersection( CommitRef(Symbol("foo")), - Filter(Description(Substring("bar"))), + Filter( + Description( + Substring("bar"), + Insensitive, + ), + ), ) "###); insta::assert_debug_snapshot!(optimize(parse("author(foo) & bar").unwrap()), @r###" Intersection( CommitRef(Symbol("bar")), - Filter(Author(Substring("foo"))), + Filter( + Author( + Substring("foo"), + Insensitive, + ), + ), ) "###); insta::assert_debug_snapshot!( optimize(parse("author(foo) & committer(bar)").unwrap()), @r###" Intersection( - Filter(Author(Substring("foo"))), - Filter(Committer(Substring("bar"))), + Filter( + Author( + Substring("foo"), + Insensitive, + ), + ), + Filter( + Committer( + Substring("bar"), + Insensitive, + ), + ), ) "###); @@ -3063,9 +3187,19 @@ mod tests { Intersection( Intersection( CommitRef(Symbol("foo")), - Filter(Description(Substring("bar"))), + Filter( + Description( + Substring("bar"), + Insensitive, + ), + ), + ), + Filter( + Author( + Substring("baz"), + Insensitive, + ), ), - Filter(Author(Substring("baz"))), ) "###); insta::assert_debug_snapshot!( @@ -3073,9 +3207,19 @@ mod tests { Intersection( Intersection( CommitRef(Symbol("bar")), - Filter(Committer(Substring("foo"))), + Filter( + Committer( + Substring("foo"), + Insensitive, + ), + ), + ), + Filter( + Author( + Substring("baz"), + Insensitive, + ), ), - Filter(Author(Substring("baz"))), ) "###); insta::assert_debug_snapshot!( @@ -3083,7 +3227,12 @@ mod tests { Intersection( Intersection( CommitRef(Symbol("baz")), - Filter(Committer(Substring("foo"))), + Filter( + Committer( + Substring("foo"), + Insensitive, + ), + ), ), Filter(File(Pattern(PrefixPath("bar")))), ) @@ -3092,10 +3241,20 @@ mod tests { optimize(parse_with_workspace("committer(foo) & file(bar) & author(baz)", &WorkspaceId::default()).unwrap()), @r###" Intersection( Intersection( - Filter(Committer(Substring("foo"))), + Filter( + Committer( + Substring("foo"), + Insensitive, + ), + ), Filter(File(Pattern(PrefixPath("bar")))), ), - Filter(Author(Substring("baz"))), + Filter( + Author( + Substring("baz"), + Insensitive, + ), + ), ) "###); insta::assert_debug_snapshot!(optimize(parse_with_workspace("foo & file(bar) & baz", &WorkspaceId::default()).unwrap()), @r###" @@ -3116,9 +3275,19 @@ mod tests { CommitRef(Symbol("foo")), CommitRef(Symbol("qux")), ), - Filter(Description(Substring("bar"))), + Filter( + Description( + Substring("bar"), + Insensitive, + ), + ), + ), + Filter( + Author( + Substring("baz"), + Insensitive, + ), ), - Filter(Author(Substring("baz"))), ) "###); insta::assert_debug_snapshot!( @@ -3128,13 +3297,23 @@ mod tests { Intersection( CommitRef(Symbol("foo")), Ancestors { - heads: Filter(Author(Substring("baz"))), + heads: Filter( + Author( + Substring("baz"), + Insensitive, + ), + ), generation: 1..2, }, ), CommitRef(Symbol("qux")), ), - Filter(Description(Substring("bar"))), + Filter( + Description( + Substring("bar"), + Insensitive, + ), + ), ) "###); insta::assert_debug_snapshot!( @@ -3145,12 +3324,22 @@ mod tests { Ancestors { heads: Intersection( CommitRef(Symbol("qux")), - Filter(Author(Substring("baz"))), + Filter( + Author( + Substring("baz"), + Insensitive, + ), + ), ), generation: 1..2, }, ), - Filter(Description(Substring("bar"))), + Filter( + Description( + Substring("bar"), + Insensitive, + ), + ), ) "###); @@ -3167,11 +3356,26 @@ mod tests { ), CommitRef(Symbol("c")), ), - Filter(Author(Substring("A"))), + Filter( + Author( + Substring("A"), + Insensitive, + ), + ), + ), + Filter( + Author( + Substring("B"), + Insensitive, + ), + ), + ), + Filter( + Author( + Substring("C"), + Insensitive, ), - Filter(Author(Substring("B"))), ), - Filter(Author(Substring("C"))), ) "###); insta::assert_debug_snapshot!( @@ -3190,11 +3394,26 @@ mod tests { ), CommitRef(Symbol("d")), ), - Filter(Author(Substring("A"))), + Filter( + Author( + Substring("A"), + Insensitive, + ), + ), + ), + Filter( + Author( + Substring("B"), + Insensitive, + ), + ), + ), + Filter( + Author( + Substring("C"), + Insensitive, ), - Filter(Author(Substring("B"))), ), - Filter(Author(Substring("C"))), ) "###); @@ -3205,9 +3424,19 @@ mod tests { Intersection( Intersection( CommitRef(Symbol("foo")), - Filter(Description(Substring("bar"))), + Filter( + Description( + Substring("bar"), + Insensitive, + ), + ), + ), + Filter( + Author( + Substring("baz"), + Insensitive, + ), ), - Filter(Author(Substring("baz"))), ) "###); } @@ -3223,7 +3452,12 @@ mod tests { CommitRef(Symbol("baz")), AsFilter( Union( - Filter(Author(Substring("foo"))), + Filter( + Author( + Substring("foo"), + Insensitive, + ), + ), CommitRef(Symbol("bar")), ), ), @@ -3238,11 +3472,21 @@ mod tests { AsFilter( Union( CommitRef(Symbol("foo")), - Filter(Committer(Substring("bar"))), + Filter( + Committer( + Substring("bar"), + Insensitive, + ), + ), ), ), ), - Filter(Description(Substring("baz"))), + Filter( + Description( + Substring("baz"), + Insensitive, + ), + ), ) "###); @@ -3258,7 +3502,12 @@ mod tests { Present( Intersection( CommitRef(Symbol("bar")), - Filter(Author(Substring("foo"))), + Filter( + Author( + Substring("foo"), + Insensitive, + ), + ), ), ), ), @@ -3287,21 +3536,36 @@ mod tests { ), AsFilter( Union( - Filter(Author(Substring("A"))), + Filter( + Author( + Substring("A"), + Insensitive, + ), + ), CommitRef(Symbol("0")), ), ), ), AsFilter( Union( - Filter(Author(Substring("B"))), + Filter( + Author( + Substring("B"), + Insensitive, + ), + ), CommitRef(Symbol("1")), ), ), ), AsFilter( Union( - Filter(Author(Substring("C"))), + Filter( + Author( + Substring("C"), + Insensitive, + ), + ), CommitRef(Symbol("2")), ), ), diff --git a/lib/src/str_util.rs b/lib/src/str_util.rs index 2cef8bae9b..b225369ee1 100644 --- a/lib/src/str_util.rs +++ b/lib/src/str_util.rs @@ -44,6 +44,16 @@ pub enum StringPattern { Substring(String), } +/// Case‐sensitivity option for [`StringPattern::matches_with()`]. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum CaseSensitivity { + /// Match case sensitively. + Sensitive, + /// Match case insensitively (currently only ignores ASCII case differences; + /// see [`StringPattern::matches_with()`]). + Insensitive, +} + impl StringPattern { /// Pattern that matches any string. pub const fn everything() -> Self { @@ -134,6 +144,51 @@ impl StringPattern { } } + /// Returns true if this pattern matches the `haystack`, ignoring case + /// differences. Only ASCII case differences are currently folded. + pub fn matches_case_insensitive(&self, haystack: &str) -> bool { + // TODO: Unicode case folding is complicated and can be locale‐specific. The + // `glob` crate and Gitoxide only deal with ASCII case folding, so we do + // the same here; a more elaborate case folding system will require + // making sure those behave in a matching manner where relevant. + // + // Care will need to be taken regarding normalization and the choice of an + // appropriate case‐insensitive comparison scheme (`toNFKC_Casefold`?) to ensure + // that it is compatible with the standard case‐insensitivity of haystack + // components (like internationalized domain names in email addresses). The + // availability of normalization and case folding schemes in database backends + // will also need to be considered. A locale‐specific case folding + // scheme would likely not be appropriate for Jujutsu. + // + // For some discussion of this topic, see: + // + match self { + StringPattern::Exact(literal) => { + haystack.to_ascii_lowercase() == literal.to_ascii_lowercase() + } + StringPattern::Glob(pattern) => pattern.matches_with( + haystack, + glob::MatchOptions { + case_sensitive: false, + ..glob::MatchOptions::new() + }, + ), + StringPattern::Substring(needle) => haystack + .to_ascii_lowercase() + .contains(&needle.to_ascii_lowercase()), + } + } + + /// Returns true if this pattern matches the `haystack`, according to the + /// specified `case_sensitivity`. See + /// [`StringPattern::matches_case_sensitive()`] for caveats. + pub fn matches_with(&self, haystack: &str, case_sensitivity: CaseSensitivity) -> bool { + match case_sensitivity { + CaseSensitivity::Sensitive => self.matches(haystack), + CaseSensitivity::Insensitive => self.matches_case_insensitive(haystack), + } + } + /// Iterates entries of the given `map` whose keys matches this pattern. pub fn filter_btree_map<'a: 'b, 'b, K: Borrow + Ord, V>( &'b self, diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 8341837ff1..e678339f8e 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -2395,9 +2395,18 @@ fn test_evaluate_expression_author() { vec![commit2.id().clone()] ); assert_eq!( - resolve_commit_ids(mut_repo, "author(\"name3\")"), + resolve_commit_ids(mut_repo, "author(\"eMaIl3\")"), vec![commit3.id().clone()] ); + // Can match case‐sensitively + assert_eq!( + resolve_commit_ids(mut_repo, "author(\"name2\", case_sensitive=true)"), + vec![commit2.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "author(\"eMaIl3\", case_sensitive=true)"), + vec![] + ); // Searches only among candidates if specified assert_eq!( resolve_commit_ids(mut_repo, "visible_heads() & author(\"name2\")"), @@ -2452,7 +2461,7 @@ fn test_evaluate_expression_mine() { .set_parents(vec![commit2.id().clone()]) .set_author(Signature { name: "name3".to_string(), - email: settings.user_email(), + email: settings.user_email().to_ascii_uppercase(), timestamp, }) .write() @@ -2536,9 +2545,18 @@ fn test_evaluate_expression_committer() { vec![commit2.id().clone()] ); assert_eq!( - resolve_commit_ids(mut_repo, "committer(\"name3\")"), + resolve_commit_ids(mut_repo, "committer(\"eMaIl3\")"), vec![commit3.id().clone()] ); + // Can match case‐sensitively + assert_eq!( + resolve_commit_ids(mut_repo, "committer(\"name2\", case_sensitive=true)"), + vec![commit2.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "committer(\"eMaIl3\", case_sensitive=true)"), + vec![] + ); // Searches only among candidates if specified assert_eq!( resolve_commit_ids(mut_repo, "visible_heads() & committer(\"name2\")"),