revset: add literal:"string" pattern syntax

The syntax is slightly different from Mercurial. In Mercurial, a pattern must
be quoted like "<kind>:<needle>". In JJ, <kind> is a separate parsing node, and
it must not appear in a quoted string. This allows us to report unknown prefix
as an error.

There's another subtle behavior difference. In Mercurial, branch(unknown) is
an error, whereas our branches(literal:unknown) is resolved to an empty set.
I think erroring out doesn't make sense for JJ since branches() by default
performs substring matching, so its behavior is more like a filter.

The parser abuses DAG range syntax for now. It can be rewritten once we remove
the deprecated x:y range syntax.
This commit is contained in:
Yuya Nishihara 2023-08-16 18:41:21 +09:00
parent 5b3c73dfc4
commit 81f1ae38b3
5 changed files with 159 additions and 4 deletions

View file

@ -74,6 +74,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Revsets gained a new function `mine()` that aliases `author([your_email])`. * Revsets gained a new function `mine()` that aliases `author([your_email])`.
* `branches()`/`remote_branches()`/`author()`/`committer()`/`description()`
revsets now support literal matching. For example, `branch(literal:main)`
selects the branch named "main", but not "maint". `description(literal:"")`
selects commits whose description is empty.
### Fixed bugs ### Fixed bugs
* `jj config set --user` and `jj config edit --user` can now be used outside of any repository. * `jj config set --user` and `jj config edit --user` can now be used outside of any repository.

View file

@ -141,6 +141,16 @@ fn test_bad_function_call() {
= Invalid file pattern: Path "../out" is not in the repo = Invalid file pattern: Path "../out" is not in the repo
"###); "###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "branches(bad:pattern)"]);
insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:10
|
1 | branches(bad:pattern)
| ^---------^
|
= Invalid arguments to revset function "branches": Invalid string pattern kind "bad"
"###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root::whatever()"]); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root::whatever()"]);
insta::assert_snapshot!(stderr, @r###" insta::assert_snapshot!(stderr, @r###"
Error: Failed to parse revset: --> 1:7 Error: Failed to parse revset: --> 1:7
@ -303,7 +313,7 @@ fn test_alias() {
1 | author(x) 1 | author(x)
| ^ | ^
| |
= Invalid arguments to revset function "author": Expected function argument of type string = Invalid arguments to revset function "author": Expected function argument of string pattern
"###); "###);
let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root & recurse"]); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "root & recurse"]);

View file

@ -137,6 +137,13 @@ revsets (expressions) as arguments.
* `present(x)`: Same as `x`, but evaluated to `none()` if any of the commits * `present(x)`: Same as `x`, but evaluated to `none()` if any of the commits
in `x` doesn't exist (e.g. is an unknown branch name.) in `x` doesn't exist (e.g. is an unknown branch name.)
## String patterns
Functions that perform string matching support the following pattern syntax.
* `"substring"`: Matches strings that contain `substring`.
* `literal:"string"`: Matches strings exactly equal to `string`.
## Aliases ## Aliases
New symbols and functions can be defined in the config file, by using any New symbols and functions can be defined in the config file, by using any

View file

@ -212,6 +212,8 @@ pub const GENERATION_RANGE_EMPTY: Range<u64> = 0..0;
/// branch name. /// branch name.
#[derive(Clone, Debug, Eq, PartialEq)] #[derive(Clone, Debug, Eq, PartialEq)]
pub enum StringPattern { pub enum StringPattern {
/// Matches strings exactly equal to `string`.
Literal(String),
/// Matches strings that contain `substring`. /// Matches strings that contain `substring`.
Substring(String), Substring(String),
} }
@ -225,6 +227,7 @@ impl StringPattern {
/// Returns true if this pattern matches the `haystack`. /// Returns true if this pattern matches the `haystack`.
pub fn matches(&self, haystack: &str) -> bool { pub fn matches(&self, haystack: &str) -> bool {
match self { match self {
StringPattern::Literal(literal) => haystack == literal,
StringPattern::Substring(needle) => haystack.contains(needle), StringPattern::Substring(needle) => haystack.contains(needle),
} }
} }
@ -1280,8 +1283,51 @@ fn parse_function_argument_to_string_pattern(
pair: Pair<Rule>, pair: Pair<Rule>,
state: ParseState, state: ParseState,
) -> Result<StringPattern, RevsetParseError> { ) -> Result<StringPattern, RevsetParseError> {
let needle = parse_function_argument_as_literal("string", name, pair, state)?; let span = pair.as_span();
Ok(StringPattern::Substring(needle)) let make_error = |message| {
RevsetParseError::with_span(
RevsetParseErrorKind::InvalidFunctionArguments {
name: name.to_string(),
message,
},
span,
)
};
let make_type_error = || make_error("Expected function argument of string pattern".to_owned());
let expression = parse_expression_rule(pair.into_inner(), state)?;
let pattern = match expression.as_ref() {
RevsetExpression::CommitRef(RevsetCommitRef::Symbol(symbol)) => {
let needle = symbol.to_owned();
StringPattern::Substring(needle)
}
// TODO: Add proper parsed node if we drop support for legacy x:y range
RevsetExpression::DagRange {
roots,
heads,
is_legacy: true,
} => {
// TODO: quoted string shouldn't be allowed as a pattern kind
let RevsetExpression::CommitRef(RevsetCommitRef::Symbol(kind)) = roots.as_ref() else {
return Err(make_type_error());
};
let RevsetExpression::CommitRef(RevsetCommitRef::Symbol(needle)) = heads.as_ref()
else {
return Err(make_type_error());
};
match kind.as_ref() {
"literal" => StringPattern::Literal(needle.clone()),
// TODO: maybe add explicit kind for substring match?
_ => {
// TODO: error span can be narrowed to the lhs node
return Err(make_error(format!(
r#"Invalid string pattern kind "{kind}""#
)));
}
}
}
_ => return Err(make_type_error()),
};
Ok(pattern)
} }
fn parse_function_argument_as_literal<T: FromStr>( fn parse_function_argument_as_literal<T: FromStr>(
@ -2621,6 +2667,42 @@ mod tests {
); );
} }
#[test]
fn test_parse_string_pattern() {
assert_eq!(
parse(r#"branches("foo")"#),
Ok(RevsetExpression::branches(StringPattern::Substring(
"foo".to_owned()
)))
);
assert_eq!(
parse(r#"branches(literal:"foo")"#),
Ok(RevsetExpression::branches(StringPattern::Literal(
"foo".to_owned()
)))
);
assert_eq!(
parse(r#"branches("literal:foo")"#),
Ok(RevsetExpression::branches(StringPattern::Substring(
"literal:foo".to_owned()
)))
);
assert_eq!(
parse(r#"branches(bad:"foo")"#),
Err(RevsetParseErrorKind::InvalidFunctionArguments {
name: "branches".to_owned(),
message: r#"Invalid string pattern kind "bad""#.to_owned()
})
);
assert_eq!(
parse(r#"branches(literal::"foo")"#),
Err(RevsetParseErrorKind::InvalidFunctionArguments {
name: "branches".to_owned(),
message: "Expected function argument of string pattern".to_owned()
})
);
}
#[test] #[test]
fn test_parse_revset_alias_formal_parameter() { fn test_parse_revset_alias_formal_parameter() {
let mut aliases_map = RevsetAliasesMap::new(); let mut aliases_map = RevsetAliasesMap::new();
@ -2743,7 +2825,7 @@ mod tests {
parse("description(visible_heads())"), parse("description(visible_heads())"),
Err(RevsetParseErrorKind::InvalidFunctionArguments { Err(RevsetParseErrorKind::InvalidFunctionArguments {
name: "description".to_string(), name: "description".to_string(),
message: "Expected function argument of type string".to_string() message: "Expected function argument of string pattern".to_string()
}) })
); );
assert_eq!( assert_eq!(
@ -2857,10 +2939,24 @@ mod tests {
); );
// Alias can be substituted to string literal. // Alias can be substituted to string literal.
assert_eq!(
parse_with_aliases("file(A)", [("A", "a")]).unwrap(),
parse("file(a)").unwrap()
);
// Alias can be substituted to string pattern.
assert_eq!( assert_eq!(
parse_with_aliases("author(A)", [("A", "a")]).unwrap(), parse_with_aliases("author(A)", [("A", "a")]).unwrap(),
parse("author(a)").unwrap() parse("author(a)").unwrap()
); );
assert_eq!(
parse_with_aliases("author(A)", [("A", "literal:a")]).unwrap(),
parse("author(literal:a)").unwrap()
);
assert_eq!(
parse_with_aliases("author(literal:A)", [("A", "a")]).unwrap(),
parse("author(literal:a)").unwrap()
);
// Multi-level substitution. // Multi-level substitution.
assert_eq!( assert_eq!(

View file

@ -1716,8 +1716,16 @@ fn test_evaluate_expression_branches(use_git: bool) {
resolve_commit_ids(mut_repo, "branches(branch)"), resolve_commit_ids(mut_repo, "branches(branch)"),
vec![commit2.id().clone(), commit1.id().clone()] vec![commit2.id().clone(), commit1.id().clone()]
); );
assert_eq!(
resolve_commit_ids(mut_repo, "branches(literal:branch1)"),
vec![commit1.id().clone()]
);
// Can silently resolve to an empty set if there's no matches // Can silently resolve to an empty set if there's no matches
assert_eq!(resolve_commit_ids(mut_repo, "branches(branch3)"), vec![]); assert_eq!(resolve_commit_ids(mut_repo, "branches(branch3)"), vec![]);
assert_eq!(
resolve_commit_ids(mut_repo, "branches(literal:ranch1)"),
vec![]
);
// Two branches pointing to the same commit does not result in a duplicate in // Two branches pointing to the same commit does not result in a duplicate in
// the revset // the revset
mut_repo.set_local_branch_target("branch3", RefTarget::normal(commit2.id().clone())); mut_repo.set_local_branch_target("branch3", RefTarget::normal(commit2.id().clone()));
@ -1788,6 +1796,10 @@ fn test_evaluate_expression_remote_branches(use_git: bool) {
resolve_commit_ids(mut_repo, "remote_branches(branch)"), resolve_commit_ids(mut_repo, "remote_branches(branch)"),
vec![commit2.id().clone(), commit1.id().clone()] vec![commit2.id().clone(), commit1.id().clone()]
); );
assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(literal:branch1)"),
vec![commit1.id().clone()]
);
// Can get branches from matching remotes // Can get branches from matching remotes
assert_eq!( assert_eq!(
resolve_commit_ids(mut_repo, r#"remote_branches("", origin)"#), resolve_commit_ids(mut_repo, r#"remote_branches("", origin)"#),
@ -1797,6 +1809,10 @@ fn test_evaluate_expression_remote_branches(use_git: bool) {
resolve_commit_ids(mut_repo, r#"remote_branches("", ri)"#), resolve_commit_ids(mut_repo, r#"remote_branches("", ri)"#),
vec![commit2.id().clone(), commit1.id().clone()] vec![commit2.id().clone(), commit1.id().clone()]
); );
assert_eq!(
resolve_commit_ids(mut_repo, r#"remote_branches("", literal:origin)"#),
vec![commit1.id().clone()]
);
// Can get branches with matching names from matching remotes // Can get branches with matching names from matching remotes
assert_eq!( assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(branch1, ri)"), resolve_commit_ids(mut_repo, "remote_branches(branch1, ri)"),
@ -1806,6 +1822,13 @@ fn test_evaluate_expression_remote_branches(use_git: bool) {
resolve_commit_ids(mut_repo, r#"remote_branches(branch, private)"#), resolve_commit_ids(mut_repo, r#"remote_branches(branch, private)"#),
vec![commit2.id().clone()] vec![commit2.id().clone()]
); );
assert_eq!(
resolve_commit_ids(
mut_repo,
r#"remote_branches(literal:branch1, literal:origin)"#
),
vec![commit1.id().clone()]
);
// Can silently resolve to an empty set if there's no matches // Can silently resolve to an empty set if there's no matches
assert_eq!( assert_eq!(
resolve_commit_ids(mut_repo, "remote_branches(branch3)"), resolve_commit_ids(mut_repo, "remote_branches(branch3)"),
@ -1819,6 +1842,20 @@ fn test_evaluate_expression_remote_branches(use_git: bool) {
resolve_commit_ids(mut_repo, r#"remote_branches(branch1, private)"#), resolve_commit_ids(mut_repo, r#"remote_branches(branch1, private)"#),
vec![] vec![]
); );
assert_eq!(
resolve_commit_ids(
mut_repo,
r#"remote_branches(literal:ranch1, literal:origin)"#
),
vec![]
);
assert_eq!(
resolve_commit_ids(
mut_repo,
r#"remote_branches(literal:branch1, literal:orig)"#
),
vec![]
);
// Two branches pointing to the same commit does not result in a duplicate in // Two branches pointing to the same commit does not result in a duplicate in
// the revset // the revset
mut_repo.set_remote_branch_target("branch3", "origin", RefTarget::normal(commit2.id().clone())); mut_repo.set_remote_branch_target("branch3", "origin", RefTarget::normal(commit2.id().clone()));