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

revsets: make function arguments always be revset expressions

Browse source 
Now that expressions may contain literal strings, we can simply have
functions accept only expressions arguments. That simplifies both the
grammar and the code.

A small drawback is that `description((foo), bar)` is now allowed and
does a search for the string "foo" (not "(foo)"). That seems
unlikely to trip up users.
This commit is contained in:
Martin von Zweigbergk 2021-04-23 10:37:42 -07:00
parent 5819687237
commit c894a7435f
2 changed files with 28 additions and 105 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
lib/src/revset.pest
View file

@ -34,19 +34,8 @@ difference_op = { "-" }
infix_op = _{ union_op | intersection_op | difference_op }
function_name = @{ (ASCII_ALPHANUMERIC | "_")+ }
// The grammar accepts a string literal or an expression for function
// arguments. We then decide when walking the parse tree if we
// should interpret the string value of the literal string or the expression
// as a string or an expression, or maybe as an integer. For example,
// parents(foo) might interpret "foo" as an expression, while limit(*:foo,5)
// might interpret its second argument as an integer. Also, parents("foo")
// might be disallowed, just as description(heads()) might be.
function_argument = {
literal_string
| expression
}
function_arguments = {
(whitespace* ~ function_argument ~ whitespace* ~ ",")* ~ whitespace* ~ function_argument ~ whitespace*
(whitespace* ~ expression ~ whitespace* ~ ",")* ~ whitespace* ~ expression ~ whitespace*
| whitespace*
}

120
lib/src/revset.rs
View file

@ -269,12 +269,9 @@ fn parse_function_expression(
match name.as_str() {
"parents" => {
if arg_count == 1 {
Ok(RevsetExpression::Parents(Box::new(
parse_function_argument_to_expression(
&name,
argument_pairs.next().unwrap().into_inner(),
)?,
)))
Ok(RevsetExpression::Parents(Box::new(parse_expression_rule(
argument_pairs.next().unwrap().into_inner(),
)?)))
} else {
Err(RevsetParseError::InvalidFunctionArguments {
name,
@ -284,10 +281,8 @@ fn parse_function_expression(
}
"children" => {
if arg_count == 1 {
let expression = parse_function_argument_to_expression(
&name,
argument_pairs.next().unwrap().into_inner(),
)?;
let expression =
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?;
Ok(RevsetExpression::Children {
base_expression: Box::new(expression),
candidates: RevsetExpression::non_obsolete_commits(),
@ -302,10 +297,7 @@ fn parse_function_expression(
"ancestors" => {
if arg_count == 1 {
Ok(RevsetExpression::Ancestors(Box::new(
parse_function_argument_to_expression(
&name,
argument_pairs.next().unwrap().into_inner(),
)?,
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?,
)))
} else {
Err(RevsetParseError::InvalidFunctionArguments {
@ -316,10 +308,8 @@ fn parse_function_expression(
}
"descendants" => {
if arg_count == 1 {
let expression = parse_function_argument_to_expression(
&name,
argument_pairs.next().unwrap().into_inner(),
)?;
let expression =
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?;
Ok(RevsetExpression::Descendants {
base_expression: Box::new(expression),
candidates: RevsetExpression::non_obsolete_commits(),
@ -348,10 +338,7 @@ fn parse_function_expression(
)))
} else if arg_count == 1 {
Ok(RevsetExpression::NonObsoleteHeads(Box::new(
parse_function_argument_to_expression(
&name,
argument_pairs.next().unwrap().into_inner(),
)?,
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?,
)))
} else {
Err(RevsetParseError::InvalidFunctionArguments {
@ -386,10 +373,7 @@ fn parse_function_expression(
RevsetExpression::AllHeads,
))))
} else {
parse_function_argument_to_expression(
&name,
argument_pairs.next().unwrap().into_inner(),
)?
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?
};
Ok(RevsetExpression::Description {
needle,
@ -400,73 +384,23 @@ fn parse_function_expression(
}
}
fn parse_function_argument_to_expression(
fn parse_function_argument_to_string(
name: &str,
mut pairs: Pairs<Rule>,
) -> Result<RevsetExpression, RevsetParseError> {
// Make a clone of the pairs for error messages
let pairs_clone = pairs.clone();
let first = pairs.next().unwrap();
assert!(pairs.next().is_none());
match first.as_rule() {
Rule::expression => Ok(parse_expression_rule(first.into_inner())?),
pairs: Pairs<Rule>,
) -> Result<String, RevsetParseError> {
let expression = parse_expression_rule(pairs.clone())?;
match expression {
RevsetExpression::Symbol(symbol) => Ok(symbol),
_ => Err(RevsetParseError::InvalidFunctionArguments {
name: name.to_string(),
message: format!(
"Expected function argument of type expression, found: {}",
pairs_clone.as_str()
"Expected function argument of type string, found: {}",
pairs.as_str()
),
}),
}
}
fn parse_function_argument_to_string(
name: &str,
mut pairs: Pairs<Rule>,
) -> Result<String, RevsetParseError> {
// Make a clone of the pairs for error messages
let pairs_clone = pairs.clone();
let first = pairs.next().unwrap();
assert!(pairs.next().is_none());
match first.as_rule() {
Rule::literal_string => {
return Ok(first
.as_str()
.strip_prefix('"')
.unwrap()
.strip_suffix('"')
.unwrap()
.to_owned());
}
Rule::expression => {
let first = first.into_inner().next().unwrap();
if first.as_rule() == Rule::infix_expression {
let first = first.into_inner().next().unwrap();
if first.as_rule() == Rule::postfix_expression {
let first = first.into_inner().next().unwrap();
if first.as_rule() == Rule::prefix_expression {
let first = first.into_inner().next().unwrap();
if first.as_rule() == Rule::primary {
let first = first.into_inner().next().unwrap();
if first.as_rule() == Rule::symbol {
return Ok(first.as_str().to_owned());
}
}
}
}
}
}
_ => {}
}
Err(RevsetParseError::InvalidFunctionArguments {
name: name.to_string(),
message: format!(
"Expected function argument of type string, found: {}",
pairs_clone.as_str()
),
})
}
pub fn parse(revset_str: &str) -> Result<RevsetExpression, RevsetParseError> {
let mut pairs = RevsetParser::parse(Rule::expression, revset_str)?;
let first = pairs.next().unwrap();
@ -1055,10 +989,9 @@ mod tests {
);
assert_eq!(
parse("parents(\"@\")"),
Err(RevsetParseError::InvalidFunctionArguments {
name: "parents".to_string(),
message: "Expected function argument of type expression, found: \"@\"".to_string()
})
Ok(RevsetExpression::Parents(Box::new(
RevsetExpression::Symbol("@".to_string())
)))
);
assert_eq!(
parse("ancestors(parents(@))"),
@ -1087,17 +1020,18 @@ mod tests {
})
);
assert_eq!(
parse("description(foo(),bar)"),
parse("description(all_heads(),bar)"),
Err(RevsetParseError::InvalidFunctionArguments {
name: "description".to_string(),
message: "Expected function argument of type string, found: foo()".to_string()
message: "Expected function argument of type string, found: all_heads()"
.to_string()
})
);
assert_eq!(
parse("description((foo),bar)"),
Err(RevsetParseError::InvalidFunctionArguments {
name: "description".to_string(),
message: "Expected function argument of type string, found: (foo)".to_string()
Ok(RevsetExpression::Description {
needle: "foo".to_string(),
base_expression: Box::new(RevsetExpression::Symbol("bar".to_string()))
})
);
assert_eq!(