ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

revset: parse @ like operator (but without alias substitution)

Browse source 
This is what I proposed in #2095. @ is now an operator to concatenate symbols.

Unlike the other operators, lhs/rhs of @ is not a target of alias substitution.
'x' in 'x@y' doesn't look like a named variable, though it's technically
possible to allow definition of an alias expanded to a symbol of specific remote
or vice versa. This will probably apply to the kind:pattern syntax, where
aliases are expanded due to the current implementation restriction. I've added
a TODO comment about that.
This commit is contained in:
Yuya Nishihara 2023-08-23 15:48:45 +09:00
parent 8c2baafe5c
commit 75ebdf69a1
3 changed files with 106 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
CHANGELOG.md
View file

@ -42,10 +42,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
Older binaries may not warn user when attempting to `git push` commits
with such signatures.
* In revsets, the working-copy symbols (such as `@` and `workspace_id@`) can
no longer be quoted and will not be resolved as a revset aliases or revset
function parameter. For example, `author(foo@)` is now an error, and
a revset alias `'revset-aliases.foo@' = '@'` will be ignored.
* In revsets, the working-copy or remote symbols (such as `@`, `workspace_id@`,
and `branch@remote`) can no longer be quoted as a unit. If a workspace or
branch name contains whitespace, quote the name like `"branch name"@remote`.
Also, these symbols will not be resolved as revset aliases or function
parameters. For example, `author(foo@)` is now an error, and the revset alias
`'revset-aliases.foo@' = '@'` will be failed to parse.
* `jj git push` will now push all branches in the range `remote_branches()..@`
instead of only branches pointing to `@` or `@-`.

8
lib/src/revset.pest
View file

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
identifier_part = @{ (ASCII_ALPHANUMERIC | "_" | "@" | "/")+ }
identifier_part = @{ (ASCII_ALPHANUMERIC | "_" | "/")+ }
identifier = @{
identifier_part ~ ("." | "-" | "+" ) ~ identifier
| identifier_part
@ -24,6 +24,8 @@ symbol = {
literal_string = { "\"" ~ (!"\"" ~ ANY)* ~ "\"" }
whitespace = _{ " " | "\t" | "\r" | "\n" | "\x0c" }
at_op = { "@" }
parents_op = { "-" }
children_op = { "+" }
compat_parents_op = { "^" }
@ -65,7 +67,11 @@ formal_parameters = {
primary = {
function_name ~ "(" ~ whitespace* ~ function_arguments ~ whitespace* ~ ")"
| "(" ~ whitespace* ~ expression ~ whitespace* ~ ")"
// "@" operator cannot be nested
| symbol ~ at_op ~ symbol
| symbol ~ at_op
| symbol
| at_op
}
neighbors_expression = _{ primary ~ (parents_op | children_op | compat_parents_op)* }

122
lib/src/revset.rs
View file

@ -879,13 +879,36 @@ fn parse_primary_rule(
let arguments_pair = pairs.next().unwrap();
parse_function_expression(first, arguments_pair, state, span)
}
Rule::symbol => parse_symbol_rule(first.into_inner(), state),
// Symbol without "@" may be substituted by aliases. Primary expression including "@"
// is considered an indecomposable unit, and no alias substitution would be made.
Rule::symbol if pairs.peek().is_none() => parse_symbol_rule(first.into_inner(), state),
Rule::symbol => {
let name = parse_symbol_rule_as_literal(first.into_inner())?;
assert_eq!(pairs.next().unwrap().as_rule(), Rule::at_op);
if let Some(second) = pairs.next() {
// infix "<name>@<remote>"
assert_eq!(second.as_rule(), Rule::symbol);
let remote = parse_symbol_rule_as_literal(second.into_inner())?;
Ok(RevsetExpression::symbol(format!("{name}@{remote}"))) // TODO
} else {
// postfix "<workspace_id>@"
Ok(RevsetExpression::working_copy(WorkspaceId::new(name)))
}
}
Rule::at_op => {
// unary "@"
let ctx = state.workspace_ctx.as_ref().ok_or_else(|| {
RevsetParseError::new(RevsetParseErrorKind::WorkingCopyWithoutWorkspace)
})?;
Ok(RevsetExpression::working_copy(ctx.workspace_id.clone()))
}
_ => {
panic!("unexpected revset parse rule: {:?}", first.as_str());
}
}
}
/// Parses symbol to expression, expands aliases as needed.
fn parse_symbol_rule(
mut pairs: Pairs<Rule>,
state: ParseState,
@ -894,20 +917,7 @@ fn parse_symbol_rule(
match first.as_rule() {
Rule::identifier => {
let name = first.as_str();
if name.ends_with('@') {
let workspace_id = if name == "@" {
if let Some(ctx) = state.workspace_ctx {
ctx.workspace_id.clone()
} else {
return Err(RevsetParseError::new(
RevsetParseErrorKind::WorkingCopyWithoutWorkspace,
));
}
} else {
WorkspaceId::new(name.strip_suffix('@').unwrap().to_string())
};
Ok(RevsetExpression::working_copy(workspace_id))
} else if let Some(expr) = state.locals.get(name) {
if let Some(expr) = state.locals.get(name) {
Ok(expr.clone())
} else if let Some((id, defn)) = state.aliases_map.get_symbol(name) {
let locals = HashMap::new(); // Don't spill out the current scope
@ -925,6 +935,18 @@ fn parse_symbol_rule(
}
}
/// Parses part of compound symbol to string without alias substitution.
fn parse_symbol_rule_as_literal(mut pairs: Pairs<Rule>) -> Result<String, RevsetParseError> {
let first = pairs.next().unwrap();
match first.as_rule() {
Rule::identifier => Ok(first.as_str().to_owned()),
Rule::literal_string => parse_string_literal(first),
_ => {
panic!("unexpected symbol parse rule: {:?}", first.as_str());
}
}
}
// TODO: Add support for \-escape syntax
fn parse_string_literal(pair: Pair<Rule>) -> Result<String, RevsetParseError> {
assert_eq!(pair.as_rule(), Rule::literal_string);
@ -2634,11 +2656,38 @@ mod tests {
parse_with_workspace("main@", &other_workspace_id),
Ok(main_wc)
);
// Quoted "@" is not interpreted as a working copy
assert_eq!(
parse("main@origin"),
Ok(RevsetExpression::symbol("main@origin".to_string()))
);
// Quoted component in @ expression
assert_eq!(
parse(r#""foo bar"@"#),
Ok(RevsetExpression::working_copy(WorkspaceId::new(
"foo bar".to_string()
)))
);
assert_eq!(
parse(r#""foo bar"@origin"#),
Ok(RevsetExpression::symbol("foo bar@origin".to_string()))
);
assert_eq!(
parse(r#"main@"foo bar""#),
Ok(RevsetExpression::symbol("main@foo bar".to_string()))
);
// Quoted "@" is not interpreted as a working copy or remote symbol
assert_eq!(
parse(r#""@""#),
Ok(RevsetExpression::symbol("@".to_string()))
);
assert_eq!(
parse(r#""main@""#),
Ok(RevsetExpression::symbol("main@".to_string()))
);
assert_eq!(
parse(r#""main@origin""#),
Ok(RevsetExpression::symbol("main@origin".to_string()))
);
// "@" in function argument must be quoted
assert_eq!(
parse("author(foo@)"),
@ -2810,9 +2859,22 @@ mod tests {
);
}
#[test]
fn test_parse_revset_alias_symbol_decl() {
let mut aliases_map = RevsetAliasesMap::new();
// Working copy or remote symbol cannot be used as an alias name.
assert!(aliases_map.insert("@", "none()").is_err());
assert!(aliases_map.insert("a@", "none()").is_err());
assert!(aliases_map.insert("a@b", "none()").is_err());
}
#[test]
fn test_parse_revset_alias_formal_parameter() {
let mut aliases_map = RevsetAliasesMap::new();
// Working copy or remote symbol cannot be used as an parameter name.
assert!(aliases_map.insert("f(@)", "none()").is_err());
assert!(aliases_map.insert("f(a@)", "none()").is_err());
assert!(aliases_map.insert("f(a@b)", "none()").is_err());
// Trailing comma isn't allowed for empty parameter
assert!(aliases_map.insert("f(,)", "none()").is_err());
// Trailing comma is allowed for the last parameter
@ -3044,12 +3106,6 @@ mod tests {
parse_with_aliases(r#"A|"A""#, [("A", "a")]).unwrap(),
parse("a|A").unwrap()
);
// Working copy symbol should not be substituted with alias.
// TODO: Make it an error instead of being ignored
assert_eq!(
parse_with_aliases(r#"a@"#, [("a@", "a")]).unwrap(),
parse("a@").unwrap()
);
// Alias can be substituted to string literal.
assert_eq!(
@ -3067,11 +3123,26 @@ mod tests {
parse_with_aliases("author(A)", [("A", "exact:a")]).unwrap(),
parse("author(exact:a)").unwrap()
);
// TODO: Substitution of part of a string pattern seems confusing. Disable it.
assert_eq!(
parse_with_aliases("author(exact:A)", [("A", "a")]).unwrap(),
parse("author(exact:a)").unwrap()
);
// Part of @ symbol cannot be substituted.
assert_eq!(
parse_with_aliases("A@", [("A", "a")]).unwrap(),
parse("A@").unwrap()
);
assert_eq!(
parse_with_aliases("A@b", [("A", "a")]).unwrap(),
parse("A@b").unwrap()
);
assert_eq!(
parse_with_aliases("a@B", [("B", "b")]).unwrap(),
parse("a@B").unwrap()
);
// Multi-level substitution.
assert_eq!(
parse_with_aliases("A", [("A", "BC"), ("BC", "b|C"), ("C", "c")]).unwrap(),
@ -3126,13 +3197,6 @@ mod tests {
parse("(x|(x|a))&y").unwrap()
);
// Working copy symbol cannot be used as parameter name
// TODO: Make it an error instead of being ignored
assert_eq!(
parse_with_aliases("F(x)", [("F(a@)", "a@|y")]).unwrap(),
parse("a@|y").unwrap()
);
// Function parameter should precede the symbol alias.
assert_eq!(
parse_with_aliases("F(a)|X", [("F(X)", "X"), ("X", "x")]).unwrap(),