From a00b815b8ac75ef9a449d99f05b5699f6e57cb5e Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Tue, 16 Jul 2024 08:24:55 -0700 Subject: [PATCH] fix another \r\n bug The n2 handling of escapes around newlines is very unsatisfying and hacky. The underlying issue is that I am not clear on the Ninja rules for when an escaped newline is allowed. One idea is that escapes only happen in file names or on the right side of variable definitions, while another is that you can escape a newline anywhere in the text where you have a newline that you want to skip. In particular, consider build$ foo: ... is that legal because it's the token `build` followed by (escaped) whitespace followed by a filename, or illegal because we expect a space after `build`? In any case this change preserves the existing n2 behavior, just making it consistent in a test we already had but for \r\n newlines. --- src/parse.rs | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/parse.rs b/src/parse.rs index 04a8397..15dba7c 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -392,7 +392,12 @@ impl<'text> Parser<'text> { /// Read and interpret the text following a '$' escape character. fn read_escape(&mut self) -> ParseResult> { Ok(match self.scanner.read() { - '\n' | '\r' => { + '\n' => { + self.scanner.skip_spaces(); + EvalPart::Literal(self.scanner.slice(0, 0)) + } + '\r' if self.scanner.peek() == '\n' => { + self.scanner.next(); self.scanner.skip_spaces(); EvalPart::Literal(self.scanner.slice(0, 0)) } @@ -425,11 +430,12 @@ impl<'text> Parser<'text> { match self.scanner.read() { ' ' => {} '$' => { - if self.scanner.peek() != '\n' { + if !self.scanner.peek_newline() { self.scanner.back(); return; } - self.scanner.next(); + self.scanner.skip('\r'); + self.scanner.skip('\n'); } _ => { self.scanner.back(); @@ -503,12 +509,17 @@ mod tests { #[test] fn parse_trailing_newline() { - let mut buf = test_case_buffer("build$\n foo$\n : $\n touch $\n\n"); - let mut parser = Parser::new(&mut buf); - let stmt = parser.read().unwrap().unwrap(); - assert!(matches!( - stmt, - Statement::Build(Build { rule: "touch", .. }) - )); + test_for_line_endings( + &["build$", " foo$", " : $", " touch $", "", ""], + |test_case| { + let mut buf = test_case_buffer(test_case); + let mut parser = Parser::new(&mut buf); + let stmt = parser.read().unwrap().unwrap(); + assert!(matches!( + stmt, + Statement::Build(Build { rule: "touch", .. }) + )); + }, + ); } }