Skip to content

Commit

Permalink
fix another \r\n bug
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
evmar committed Jul 16, 2024
1 parent 056985e commit a00b815
Showing 1 changed file with 21 additions and 10 deletions.
31 changes: 21 additions & 10 deletions src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,12 @@ impl<'text> Parser<'text> {
/// Read and interpret the text following a '$' escape character.
fn read_escape(&mut self) -> ParseResult<EvalPart<&'text str>> {
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))
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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", .. })
));
},
);
}
}

0 comments on commit a00b815

Please sign in to comment.