Code review-tastic changes 🦪

This commit is contained in:
Martin Haug 2020-08-27 20:42:09 +02:00
parent 3de20f8d38
commit 3abb9ec319
2 changed files with 38 additions and 46 deletions

View File

@ -114,25 +114,22 @@ impl Parser<'_> {
fn parse_bracket_call(&mut self, chained: bool) -> Spanned<CallExpr> { fn parse_bracket_call(&mut self, chained: bool) -> Spanned<CallExpr> {
let before_bracket = self.pos(); let before_bracket = self.pos();
if !chained { if !chained {
self.start_group(Delimiter::Bracket); self.start_group(Group::Bracket);
self.tokens.push_mode(TokenMode::Header); self.tokens.push_mode(TokenMode::Header);
} }
let after_bracket = self.pos(); let before_name = self.pos();
self.start_group(Delimiter::Subheader); self.start_group(Group::Subheader);
self.skip_white(); self.skip_white();
let name = self.parse_ident().unwrap_or_else(|| { let name = self.parse_ident().unwrap_or_else(|| {
self.expected_found_or_at("function name", after_bracket); self.expected_found_or_at("function name", before_name);
Spanned::zero(Ident(String::new())) Spanned::zero(Ident(String::new()))
}); });
self.skip_white(); self.skip_white();
let mut last_was_chain = false;
let mut args = match self.eatv() { let mut args = match self.eatv() {
Some(Token::Colon) => { Some(Token::Colon) => self.parse_table_contents().0,
self.parse_table_contents().0
},
Some(_) => { Some(_) => {
self.expected_at("colon", name.span.end); self.expected_at("colon", name.span.end);
while self.eat().is_some() {} while self.eat().is_some() {}
@ -143,29 +140,22 @@ impl Parser<'_> {
self.end_group(); self.end_group();
self.skip_white(); self.skip_white();
if self.peek().is_some() { let (has_chained_child, end) = if self.peek().is_some() {
last_was_chain = true;
let item = self.parse_bracket_call(true); let item = self.parse_bracket_call(true);
let span = item.span; let span = item.span;
let t = vec![item.map(|f| SyntaxNode::Call(f))]; let t = vec![item.map(|f| SyntaxNode::Call(f))];
args.push(SpannedEntry::val(Spanned::new(Expr::Tree(t), span))); args.push(SpannedEntry::val(Spanned::new(Expr::Tree(t), span)));
} (true, span.end)
let end = if !last_was_chain {
self.tokens.pop_mode();
self.end_group().end
} else { } else {
args.last() self.tokens.pop_mode();
.expect("last_was_chain set, call expected") (false, self.end_group().end)
.1.val.span.end
}; };
let start = if chained { after_bracket } else { before_bracket }; let start = if chained { before_name } else { before_bracket };
let mut span = Span::new(start, end); let mut span = Span::new(start, end);
if self.check(Token::LeftBracket) && !has_chained_child {
if self.check(Token::LeftBracket) && !last_was_chain { self.start_group(Group::Bracket);
self.start_group(Delimiter::Bracket);
self.tokens.push_mode(TokenMode::Body); self.tokens.push_mode(TokenMode::Body);
let body = self.parse_body_contents(); let body = self.parse_body_contents();
@ -182,7 +172,7 @@ impl Parser<'_> {
} }
fn parse_paren_call(&mut self, name: Spanned<Ident>) -> Spanned<CallExpr> { fn parse_paren_call(&mut self, name: Spanned<Ident>) -> Spanned<CallExpr> {
self.start_group(Delimiter::Paren); self.start_group(Group::Paren);
let args = self.parse_table_contents().0; let args = self.parse_table_contents().0;
let args_span = self.end_group(); let args_span = self.end_group();
let span = Span::merge(name.span, args_span); let span = Span::merge(name.span, args_span);
@ -357,7 +347,7 @@ impl Parser<'_> {
// a table in any case and coerce the table into a value if it is // a table in any case and coerce the table into a value if it is
// coercable (length 1 and no trailing comma). // coercable (length 1 and no trailing comma).
Token::LeftParen => { Token::LeftParen => {
self.start_group(Delimiter::Paren); self.start_group(Group::Paren);
let (table, coercable) = self.parse_table_contents(); let (table, coercable) = self.parse_table_contents();
let span = self.end_group(); let span = self.end_group();
@ -374,7 +364,7 @@ impl Parser<'_> {
// This is a content expression. // This is a content expression.
Token::LeftBrace => { Token::LeftBrace => {
self.start_group(Delimiter::Brace); self.start_group(Group::Brace);
self.tokens.push_mode(TokenMode::Body); self.tokens.push_mode(TokenMode::Body);
let tree = self.parse_body_contents(); let tree = self.parse_body_contents();
@ -441,7 +431,7 @@ impl Parser<'_> {
// Parsing primitives. // Parsing primitives.
impl<'s> Parser<'s> { impl<'s> Parser<'s> {
fn start_group(&mut self, delimiter: Delimiter) { fn start_group(&mut self, delimiter: Group) {
let start = self.pos(); let start = self.pos();
if let Some(start_token) = delimiter.start() { if let Some(start_token) = delimiter.start() {
self.assert(start_token); self.assert(start_token);
@ -450,14 +440,16 @@ impl<'s> Parser<'s> {
} }
fn end_group(&mut self) -> Span { fn end_group(&mut self) -> Span {
if self.delimiters.last()
.expect("group was not started").1 != Token::FunctionLink {
assert_eq!(self.peek(), None, "unfinished group");
}
let (start, end_token) = self.delimiters.pop() let (start, end_token) = self.delimiters.pop()
.expect("group was not started"); .expect("group was not started");
if end_token != Token::Chain {
if self.peek() != None {
self.delimiters.push((start, end_token));
assert_eq!(self.peek(), None, "unfinished group");
}
}
match self.peeked.unwrap() { match self.peeked.unwrap() {
Some(token) if token.v == end_token => { Some(token) if token.v == end_token => {
self.peeked = None; self.peeked = None;
@ -465,7 +457,7 @@ impl<'s> Parser<'s> {
} }
_ => { _ => {
let end = self.pos(); let end = self.pos();
if end_token != Token::FunctionLink { if end_token != Token::Chain {
error!( error!(
@self.feedback, Span::at(end), @self.feedback, Span::at(end),
"expected {}", end_token.name(), "expected {}", end_token.name(),
@ -532,7 +524,7 @@ impl<'s> Parser<'s> {
let token = (*self.peeked.get_or_insert_with(|| tokens.next()))?; let token = (*self.peeked.get_or_insert_with(|| tokens.next()))?;
// Check for unclosed groups. // Check for unclosed groups.
if Delimiter::is_delimiter(token.v) { if Group::is_delimiter(token.v) {
if self.delimiters.iter().rev().any(|&(_, end)| token.v == end) { if self.delimiters.iter().rev().any(|&(_, end)| token.v == end) {
return None; return None;
} }
@ -550,19 +542,19 @@ impl<'s> Parser<'s> {
} }
#[derive(Debug, Copy, Clone, Eq, PartialEq)] #[derive(Debug, Copy, Clone, Eq, PartialEq)]
enum Delimiter { enum Group {
Paren, Paren,
Bracket, Bracket,
Brace, Brace,
Subheader, Subheader,
} }
impl Delimiter { impl Group {
fn is_delimiter(token: Token<'_>) -> bool { fn is_delimiter(token: Token<'_>) -> bool {
matches!( matches!(
token, token,
Token::RightParen | Token::RightBracket Token::RightParen | Token::RightBracket
| Token::RightBrace | Token::FunctionLink | Token::RightBrace | Token::Chain
) )
} }
@ -580,7 +572,7 @@ impl Delimiter {
Self::Paren => Token::RightParen, Self::Paren => Token::RightParen,
Self::Bracket => Token::RightBracket, Self::Bracket => Token::RightBracket,
Self::Brace => Token::RightBrace, Self::Brace => Token::RightBrace,
Self::Subheader => Token::FunctionLink, Self::Subheader => Token::Chain,
} }
} }
} }
@ -884,6 +876,7 @@ mod tests {
t!("[hi: (5.0, 2.1 >> you]" => P![F!("hi"; Table![Num(5.0), Num(2.1)], Tree![F!("you")])]); t!("[hi: (5.0, 2.1 >> you]" => P![F!("hi"; Table![Num(5.0), Num(2.1)], Tree![F!("you")])]);
t!("[bold: 400, >> emph >> sub: 1cm]" => P![F!("bold"; Num(400.0), Tree![F!("emph"; Tree!(F!("sub"; Len(Length::cm(1.0)))))])]); t!("[bold: 400, >> emph >> sub: 1cm]" => P![F!("bold"; Num(400.0), Tree![F!("emph"; Tree!(F!("sub"; Len(Length::cm(1.0)))))])]);
t!("[box >> pad: 1pt][Hi]" => P![F!("box"; Tree![F!("pad"; Len(Length::pt(1.0)), Tree!(P![T("Hi")]))])]); t!("[box >> pad: 1pt][Hi]" => P![F!("box"; Tree![F!("pad"; Len(Length::pt(1.0)), Tree!(P![T("Hi")]))])]);
t!("[box >>][Hi]" => P![F!("box"; Tree![P![T("Hi")]])]);
// Errors for unclosed / empty predecessor groups // Errors for unclosed / empty predecessor groups
e!("[hi: (5.0, 2.1 >> you]" => s(0, 15, 0, 15, "expected closing paren")); e!("[hi: (5.0, 2.1 >> you]" => s(0, 15, 0, 15, "expected closing paren"));

View File

@ -35,7 +35,7 @@ pub enum Token<'s> {
/// A right brace in a function header: `}`. /// A right brace in a function header: `}`.
RightBrace, RightBrace,
/// A double forward chevron in a function header: `>>`. /// A double forward chevron in a function header: `>>`.
FunctionLink, Chain,
/// A colon in a function header: `:`. /// A colon in a function header: `:`.
Colon, Colon,
@ -110,7 +110,7 @@ impl<'s> Token<'s> {
RightParen => "closing paren", RightParen => "closing paren",
LeftBrace => "opening brace", LeftBrace => "opening brace",
RightBrace => "closing brace", RightBrace => "closing brace",
FunctionLink => "function chain operator", Chain => "function chain operator",
Colon => "colon", Colon => "colon",
Comma => "comma", Comma => "comma",
Equals => "equals sign", Equals => "equals sign",
@ -222,7 +222,7 @@ impl<'s> Iterator for Tokens<'s> {
',' if self.mode == Header => Comma, ',' if self.mode == Header => Comma,
'=' if self.mode == Header => Equals, '=' if self.mode == Header => Equals,
'>' if self.mode == Header && self.peek() == Some('>') => '>' if self.mode == Header && self.peek() == Some('>') =>
self.read_function_chain(), self.read_chain(),
// Expression operators. // Expression operators.
'+' if self.mode == Header => Plus, '+' if self.mode == Header => Plus,
@ -313,10 +313,9 @@ impl<'s> Tokens<'s> {
}, true, 0, -2).0) }, true, 0, -2).0)
} }
fn read_function_chain(&mut self) -> Token<'s> { fn read_chain(&mut self) -> Token<'s> {
let second = self.eat(); assert!(self.eat() == Some('>'));
debug_assert!(second == Some('>')); Chain
FunctionLink
} }
fn read_whitespace(&mut self, start: Pos) -> Token<'s> { fn read_whitespace(&mut self, start: Pos) -> Token<'s> {
@ -501,7 +500,7 @@ mod tests {
LeftBracket as L, RightBracket as R, LeftBracket as L, RightBracket as R,
LeftParen as LP, RightParen as RP, LeftParen as LP, RightParen as RP,
LeftBrace as LB, RightBrace as RB, LeftBrace as LB, RightBrace as RB,
FunctionLink, Chain,
Ident as Id, Ident as Id,
Bool, Bool,
Number as Num, Number as Num,
@ -591,7 +590,7 @@ mod tests {
t!(Header, ">main" => Invalid(">main")); t!(Header, ">main" => Invalid(">main"));
t!(Header, ".func.box" => Id(".func.box")); t!(Header, ".func.box" => Id(".func.box"));
t!(Header, "arg, _b, _1" => Id("arg"), Comma, S(0), Id("_b"), Comma, S(0), Id("_1")); t!(Header, "arg, _b, _1" => Id("arg"), Comma, S(0), Id("_b"), Comma, S(0), Id("_1"));
t!(Header, "f: arg >> g" => Id("f"), Colon, S(0), Id("arg"), S(0), FunctionLink, S(0), Id("g")); t!(Header, "f: arg >> g" => Id("f"), Colon, S(0), Id("arg"), S(0), Chain, S(0), Id("g"));
t!(Header, "12_pt, 12pt" => Invalid("12_pt"), Comma, S(0), Len(Length::pt(12.0))); t!(Header, "12_pt, 12pt" => Invalid("12_pt"), Comma, S(0), Len(Length::pt(12.0)));
t!(Header, "1e5in" => Len(Length::inches(100000.0))); t!(Header, "1e5in" => Len(Length::inches(100000.0)));
t!(Header, "2.3cm" => Len(Length::cm(2.3))); t!(Header, "2.3cm" => Len(Length::cm(2.3)));