feat(config): add ability to have trailing commas #32
Conversation
src/config/parser.rs
Outdated
| if iter.peek().map(|x| x.toktype == ty).unwrap_or(false) { | ||
| iter.next(); | ||
| true | ||
| } else { | ||
| false | ||
| } |
There was a problem hiding this comment.
A match statement would probably be more readable:
| if iter.peek().map(|x| x.toktype == ty).unwrap_or(false) { | |
| iter.next(); | |
| true | |
| } else { | |
| false | |
| } | |
| match iter.peek().map(|x| x.toktype == ty) { | |
| Some(true) => { | |
| iter.next(); | |
| true | |
| } | |
| _ => false, | |
| } |
src/config/parser.rs
Outdated
| @@ -182,48 +171,60 @@ impl VariantExpr { | |||
| .unwrap_or(false) | |||
| { | |||
| return Err(ParseError::from(ParseErrorType::Custom( | |||
| "Must have at least one option", | |||
| "Variant expression have at least one option", | |||
There was a problem hiding this comment.
This should probably read "- expression must have -".
src/config/parser.rs
Outdated
| } | ||
| } | ||
| expect(iter, &[TokType::RBracket])?; | ||
| Ok(VariantExpr { specs }) | ||
| expect(iter, &[TokType::LBrace])?; |
There was a problem hiding this comment.
I think it would be better to call expect(iter, &[TokType::LBrace]) as the top of the parse function (first line).
| impl<T: SimpleParse> CommaList<T> { | ||
| pub fn parse<I: Iterator<Item = Token>>( | ||
| iter: &mut Peekable<I>, | ||
| end: TokType, |
There was a problem hiding this comment.
Clarify what end is and if applicable, what TokType it should be.
src/config/parser.rs
Outdated
| loop { | ||
| // Allow trailing commas | ||
| if eat!(iter, RBrace) { | ||
| // Allow trailing comma |
There was a problem hiding this comment.
Explain how this permits a trailing comma.
There is also a repetitive if statement in this loop:
if eat(iter, end) {
break;
}This should be refactored if possible.
| #[test] | ||
| fn variant_trailing_comma() { | ||
| success( | ||
| &toklist![ | ||
| TokType::LBracket, | ||
| "a", | ||
| TokType::Comma, | ||
| TokType::RBracket, | ||
| TokType::Semicolon | ||
| ], | ||
| &[Entry { | ||
| left: Spec { | ||
| string: None, | ||
| spectype: SpecType::variant_expr(vec![Spec::from("a")], None), | ||
| }, | ||
| right: None, | ||
| }], | ||
| ) | ||
| } | ||
|
|
||
| #[test] | ||
| fn match_trailing_comma() { | ||
| success( | ||
| &toklist![ | ||
| TokType::LBrace, | ||
| "linux", | ||
| TokType::Colon, | ||
| "a", | ||
| TokType::Comma, | ||
| TokType::RBrace, | ||
| TokType::Semicolon | ||
| ], | ||
| &[Entry { | ||
| left: Spec { | ||
| string: None, | ||
| spectype: SpecType::match_expr( | ||
| vec![(ExprType::Linux.into(), Spec::from("a"))], | ||
| None, | ||
| ), | ||
| }, | ||
| right: None, | ||
| }], | ||
| ) | ||
| } |
There was a problem hiding this comment.
Add a comment to each of these test functions explaining that trailing commas are valid syntax and should be successfully parsed.
This closes #23.