diff --git a/lib/src/revset.pest b/lib/src/revset.pest index 86e222f14..c0703c4f9 100644 --- a/lib/src/revset.pest +++ b/lib/src/revset.pest @@ -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* } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index c09a53ea3..0136c8584 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -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, -) -> Result { - // 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, +) -> Result { + 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, -) -> Result { - // 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 { 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!(