From 5da2ab1b7d698c98cf7ce8a673bc7220c4f74f06 Mon Sep 17 00:00:00 2001 From: Fernando Herrera Date: Tue, 31 Aug 2021 20:33:41 +0100 Subject: [PATCH 1/3] comments with a newline dont get together --- crates/nu-parser/src/lex.rs | 12 ++- crates/nu-parser/src/lite_parse.rs | 6 +- crates/nu-parser/tests/test_lex.rs | 46 ++++++++++- crates/nu-parser/tests/test_lite_parser.rs | 91 ++++++++++++++++++---- 4 files changed, 134 insertions(+), 21 deletions(-) diff --git a/crates/nu-parser/src/lex.rs b/crates/nu-parser/src/lex.rs index 5a33d87bb7..854a626379 100644 --- a/crates/nu-parser/src/lex.rs +++ b/crates/nu-parser/src/lex.rs @@ -273,7 +273,15 @@ pub fn lex( if *input == b'\n' || *input == b'\r' { output.push(Token::new( TokenContents::Comment, - Span::new(start, curr_offset), + Span::new(start, curr_offset - 1), + )); + + // Adding an end of line token after a comment + // This helps during lite_parser to avoid losing a command + // in a statement + output.push(Token::new( + TokenContents::Eol, + Span::new(curr_offset - 1, curr_offset), )); start = curr_offset; @@ -307,4 +315,4 @@ pub fn lex( } } (output, error) -} \ No newline at end of file +} diff --git a/crates/nu-parser/src/lite_parse.rs b/crates/nu-parser/src/lite_parse.rs index f702701b36..a81c4c3fa0 100644 --- a/crates/nu-parser/src/lite_parse.rs +++ b/crates/nu-parser/src/lite_parse.rs @@ -27,6 +27,10 @@ impl LiteCommand { pub fn is_empty(&self) -> bool { self.parts.is_empty() } + + pub fn is_empty_comments(&self) -> bool { + self.comments.is_empty() + } } #[derive(Debug)] @@ -94,7 +98,7 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { } } TokenContents::Eol | TokenContents::Semicolon => { - if !curr_command.is_empty() { + if !curr_command.is_empty() || !curr_command.is_empty_comments() { curr_pipeline.push(curr_command); } curr_command = LiteCommand::new(); diff --git a/crates/nu-parser/tests/test_lex.rs b/crates/nu-parser/tests/test_lex.rs index 7646db7c86..7c68c1bf72 100644 --- a/crates/nu-parser/tests/test_lex.rs +++ b/crates/nu-parser/tests/test_lex.rs @@ -57,7 +57,7 @@ fn lex_comment() { output.0.get(4).unwrap(), &Token { contents: TokenContents::Comment, - span: Span { start: 12, end: 25 } + span: Span { start: 12, end: 24 } } ); } @@ -91,3 +91,47 @@ fn lex_incomplete_quote() { let err = output.1.unwrap(); assert!(matches!(err, ParseError::UnexpectedEof(v, _) if v == "'")); } + +#[test] +fn lex_comments() { + // Comments should keep the end of line token + // Code: + // let z = 4 + // let x = 4 #comment + // let y = 1 # comment + let file = b"let z = 4 #comment \n let x = 4 # comment\n let y = 1 # comment"; + + let output = lex(file, 0, &[], &[]); + + assert_eq!( + output.0.get(4).unwrap(), + &Token { + contents: TokenContents::Comment, + span: Span { start: 10, end: 19 } + } + ); + assert_eq!( + output.0.get(5).unwrap(), + &Token { + contents: TokenContents::Eol, + span: Span { start: 19, end: 20 } + } + ); + + // When there is no space between the comment and the new line the span + // for the command and the EOL overlaps + assert_eq!( + output.0.get(10).unwrap(), + &Token { + contents: TokenContents::Comment, + span: Span { start: 31, end: 40 } + } + ); + assert_eq!( + output.0.get(11).unwrap(), + &Token { + contents: TokenContents::Eol, + span: Span { start: 40, end: 41 } + } + ); +} diff --git a/crates/nu-parser/tests/test_lite_parser.rs b/crates/nu-parser/tests/test_lite_parser.rs index 81c415048f..e20e578b50 100644 --- a/crates/nu-parser/tests/test_lite_parser.rs +++ b/crates/nu-parser/tests/test_lite_parser.rs @@ -16,20 +16,25 @@ fn lite_parse_helper(input: &[u8]) -> Result { #[test] fn comment_before() -> Result<(), ParseError> { + // Code: + // # this is a comment + // def foo bar let input = b"# this is a comment\ndef foo bar"; let lite_block = lite_parse_helper(input)?; - assert_eq!(lite_block.block.len(), 1); + assert_eq!(lite_block.block.len(), 2); assert_eq!(lite_block.block[0].commands.len(), 1); assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); - assert_eq!(lite_block.block[0].commands[0].parts.len(), 3); + assert_eq!(lite_block.block[1].commands[0].parts.len(), 3); Ok(()) } #[test] fn comment_beside() -> Result<(), ParseError> { + // Code: + // def foo bar # this is a comment let input = b"def foo bar # this is a comment"; let lite_block = lite_parse_helper(input)?; @@ -44,39 +49,63 @@ fn comment_beside() -> Result<(), ParseError> { #[test] fn comments_stack() -> Result<(), ParseError> { + // Code: + // # this is a comment + // # another comment + // # def foo bar let input = b"# this is a comment\n# another comment\ndef foo bar "; let lite_block = lite_parse_helper(input)?; - assert_eq!(lite_block.block.len(), 1); - assert_eq!(lite_block.block[0].commands.len(), 1); - assert_eq!(lite_block.block[0].commands[0].comments.len(), 2); - assert_eq!(lite_block.block[0].commands[0].parts.len(), 3); + assert_eq!(lite_block.block.len(), 3); + assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); + assert_eq!(lite_block.block[0].commands[0].parts.len(), 0); + + assert_eq!(lite_block.block[1].commands[0].comments.len(), 1); + assert_eq!(lite_block.block[1].commands[0].parts.len(), 0); + + assert_eq!(lite_block.block[2].commands[0].comments.len(), 0); + assert_eq!(lite_block.block[2].commands[0].parts.len(), 3); Ok(()) } #[test] fn separated_comments_dont_stack() -> Result<(), ParseError> { + // Code: + // # this is a comment + // + // # another comment + // # def foo bar let input = b"# this is a comment\n\n# another comment\ndef foo bar "; let lite_block = lite_parse_helper(input)?; - assert_eq!(lite_block.block.len(), 1); - assert_eq!(lite_block.block[0].commands.len(), 1); + assert_eq!(lite_block.block.len(), 3); assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); + assert_eq!(lite_block.block[0].commands[0].parts.len(), 0); + + assert_eq!(lite_block.block[1].commands[0].comments.len(), 1); + assert_eq!(lite_block.block[1].commands[0].parts.len(), 0); + + assert_eq!(lite_block.block[2].commands[0].comments.len(), 0); + assert_eq!(lite_block.block[2].commands[0].parts.len(), 3); + assert_eq!( lite_block.block[0].commands[0].comments[0], - Span { start: 21, end: 39 } + Span { start: 0, end: 19 } + ); + assert_eq!( + lite_block.block[1].commands[0].comments[0], + Span { start: 21, end: 38 } ); - assert_eq!(lite_block.block[0].commands[0].parts.len(), 3); Ok(()) } #[test] fn multiple_statements() -> Result<(), ParseError> { - // Code : + // Code: // # A comment // let a = ( 3 + ( // 4 + @@ -86,16 +115,20 @@ fn multiple_statements() -> Result<(), ParseError> { let lite_block = lite_parse_helper(input)?; - assert_eq!(lite_block.block.len(), 2); - assert_eq!(lite_block.block[0].commands.len(), 1); + assert_eq!(lite_block.block.len(), 3); + assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); assert_eq!( lite_block.block[0].commands[0].comments[0], - Span { start: 0, end: 11 } + Span { start: 0, end: 10 } ); - assert_eq!(lite_block.block[1].commands.len(), 1); + assert_eq!(lite_block.block[1].commands[0].comments.len(), 0); + assert_eq!(lite_block.block[1].commands[0].parts.len(), 4); + + assert_eq!(lite_block.block[2].commands[0].comments.len(), 1); + assert_eq!(lite_block.block[2].commands[0].parts.len(), 4); assert_eq!( - lite_block.block[1].commands[0].comments[0], + lite_block.block[2].commands[0].comments[0], Span { start: 52, end: 61 } ); @@ -105,7 +138,7 @@ fn multiple_statements() -> Result<(), ParseError> { #[test] fn multiple_commands() -> Result<(), ParseError> { // Pipes add commands to the lite parser - // Code : + // Code: // let a = ls | where name == 1 // let b = 1 # comment let input = b"let a = ls | where name == 1 \n let b = 1 # comment"; @@ -123,3 +156,27 @@ fn multiple_commands() -> Result<(), ParseError> { Ok(()) } + +#[test] +fn multiple_commands_with_comment() -> Result<(), ParseError> { + // Pipes add commands to the lite parser + // The comments are attached to the commands next to them + // Code: + // let a = ls | where name == 1 # comment + // let b = 1 # comment + //let a = ls | where name == 1 # comment \n let b = 1 # comment + let input = b"let a = ls | where name == 1 # comment\n let b = 1 # comment"; + + let lite_block = lite_parse_helper(input)?; + + assert_eq!(lite_block.block.len(), 2); + assert_eq!(lite_block.block[0].commands.len(), 2); + assert_eq!(lite_block.block[1].commands.len(), 1); + + assert_eq!( + lite_block.block[0].commands[1].comments[0], + Span { start: 29, end: 38 } + ); + + Ok(()) +} From 73f6a57b12d5e3ad2374e2a360d9cb403f83ab29 Mon Sep 17 00:00:00 2001 From: Fernando Herrera Date: Wed, 1 Sep 2021 21:05:37 +0100 Subject: [PATCH 2/3] upper comments get attached to command --- crates/nu-parser/src/lite_parse.rs | 8 +- crates/nu-parser/tests/test_lite_parser.rs | 114 ++++++++++++++++----- 2 files changed, 94 insertions(+), 28 deletions(-) diff --git a/crates/nu-parser/src/lite_parse.rs b/crates/nu-parser/src/lite_parse.rs index a81c4c3fa0..ad3408ab7d 100644 --- a/crates/nu-parser/src/lite_parse.rs +++ b/crates/nu-parser/src/lite_parse.rs @@ -98,15 +98,17 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { } } TokenContents::Eol | TokenContents::Semicolon => { - if !curr_command.is_empty() || !curr_command.is_empty_comments() { + if !curr_command.is_empty() { curr_pipeline.push(curr_command); + + curr_command = LiteCommand::new(); } - curr_command = LiteCommand::new(); if !curr_pipeline.is_empty() { block.push(curr_pipeline); + + curr_pipeline = LiteStatement::new(); } - curr_pipeline = LiteStatement::new(); } TokenContents::Comment => { curr_command.comments.push(token.span); diff --git a/crates/nu-parser/tests/test_lite_parser.rs b/crates/nu-parser/tests/test_lite_parser.rs index e20e578b50..7193501ed9 100644 --- a/crates/nu-parser/tests/test_lite_parser.rs +++ b/crates/nu-parser/tests/test_lite_parser.rs @@ -23,10 +23,15 @@ fn comment_before() -> Result<(), ParseError> { let lite_block = lite_parse_helper(input)?; - assert_eq!(lite_block.block.len(), 2); + assert_eq!(lite_block.block.len(), 1); assert_eq!(lite_block.block[0].commands.len(), 1); assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); - assert_eq!(lite_block.block[1].commands[0].parts.len(), 3); + assert_eq!(lite_block.block[0].commands[0].parts.len(), 3); + + assert_eq!( + lite_block.block[0].commands[0].comments[0], + Span { start: 0, end: 19 } + ); Ok(()) } @@ -44,6 +49,11 @@ fn comment_beside() -> Result<(), ParseError> { assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); assert_eq!(lite_block.block[0].commands[0].parts.len(), 3); + assert_eq!( + lite_block.block[0].commands[0].comments[0], + Span { start: 12, end: 31 } + ); + Ok(()) } @@ -57,15 +67,19 @@ fn comments_stack() -> Result<(), ParseError> { let lite_block = lite_parse_helper(input)?; - assert_eq!(lite_block.block.len(), 3); - assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); - assert_eq!(lite_block.block[0].commands[0].parts.len(), 0); + assert_eq!(lite_block.block.len(), 1); + assert_eq!(lite_block.block[0].commands[0].comments.len(), 2); + assert_eq!(lite_block.block[0].commands[0].parts.len(), 3); - assert_eq!(lite_block.block[1].commands[0].comments.len(), 1); - assert_eq!(lite_block.block[1].commands[0].parts.len(), 0); + assert_eq!( + lite_block.block[0].commands[0].comments[0], + Span { start: 0, end: 19 } + ); - assert_eq!(lite_block.block[2].commands[0].comments.len(), 0); - assert_eq!(lite_block.block[2].commands[0].parts.len(), 3); + assert_eq!( + lite_block.block[0].commands[0].comments[1], + Span { start: 20, end: 37 } + ); Ok(()) } @@ -81,22 +95,17 @@ fn separated_comments_dont_stack() -> Result<(), ParseError> { let lite_block = lite_parse_helper(input)?; - assert_eq!(lite_block.block.len(), 3); - assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); - assert_eq!(lite_block.block[0].commands[0].parts.len(), 0); - - assert_eq!(lite_block.block[1].commands[0].comments.len(), 1); - assert_eq!(lite_block.block[1].commands[0].parts.len(), 0); - - assert_eq!(lite_block.block[2].commands[0].comments.len(), 0); - assert_eq!(lite_block.block[2].commands[0].parts.len(), 3); + assert_eq!(lite_block.block.len(), 1); + assert_eq!(lite_block.block[0].commands[0].comments.len(), 2); + assert_eq!(lite_block.block[0].commands[0].parts.len(), 3); assert_eq!( lite_block.block[0].commands[0].comments[0], Span { start: 0, end: 19 } ); + assert_eq!( - lite_block.block[1].commands[0].comments[0], + lite_block.block[0].commands[0].comments[1], Span { start: 21, end: 38 } ); @@ -115,20 +124,18 @@ fn multiple_statements() -> Result<(), ParseError> { let lite_block = lite_parse_helper(input)?; - assert_eq!(lite_block.block.len(), 3); + assert_eq!(lite_block.block.len(), 2); assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); + assert_eq!(lite_block.block[0].commands[0].parts.len(), 4); assert_eq!( lite_block.block[0].commands[0].comments[0], Span { start: 0, end: 10 } ); - assert_eq!(lite_block.block[1].commands[0].comments.len(), 0); + assert_eq!(lite_block.block[1].commands[0].comments.len(), 1); assert_eq!(lite_block.block[1].commands[0].parts.len(), 4); - - assert_eq!(lite_block.block[2].commands[0].comments.len(), 1); - assert_eq!(lite_block.block[2].commands[0].parts.len(), 4); assert_eq!( - lite_block.block[2].commands[0].comments[0], + lite_block.block[1].commands[0].comments[0], Span { start: 52, end: 61 } ); @@ -180,3 +187,60 @@ fn multiple_commands_with_comment() -> Result<(), ParseError> { Ok(()) } + +#[test] +fn multiple_commands_with_pipes() -> Result<(), ParseError> { + // The comments inside () get encapsulated in the whole item + // Code: + // # comment 1 + // # comment 2 + // let a = ( ls + // | where name =~ some # another comment + // | each { |file| rm file.name } # final comment + // ) + // # comment A + // let b = 0; + let input = b"# comment 1 +# comment 2 +let a = ( ls +| where name =~ some # another comment +| each { |file| rm file.name }) # final comment +# comment A +let b = 0 +"; + + let lite_block = lite_parse_helper(input)?; + + assert_eq!(lite_block.block.len(), 2); + assert_eq!(lite_block.block[0].commands[0].comments.len(), 3); + assert_eq!(lite_block.block[0].commands[0].parts.len(), 4); + + assert_eq!( + lite_block.block[0].commands[0].parts[3], + Span { + start: 32, + end: 107 + } + ); + + assert_eq!( + lite_block.block[0].commands[0].comments[2], + Span { + start: 108, + end: 123 + } + ); + + assert_eq!(lite_block.block[1].commands[0].comments.len(), 1); + assert_eq!(lite_block.block[1].commands[0].parts.len(), 4); + + assert_eq!( + lite_block.block[1].commands[0].comments[0], + Span { + start: 124, + end: 135 + } + ); + + Ok(()) +} From 4ed79614acc209c25f3b2c8e068aa63ecdfd9ac9 Mon Sep 17 00:00:00 2001 From: Fernando Herrera Date: Wed, 1 Sep 2021 21:34:16 +0100 Subject: [PATCH 3/3] removed unused empty function --- crates/nu-parser/src/lite_parse.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/nu-parser/src/lite_parse.rs b/crates/nu-parser/src/lite_parse.rs index ad3408ab7d..e8992b547d 100644 --- a/crates/nu-parser/src/lite_parse.rs +++ b/crates/nu-parser/src/lite_parse.rs @@ -27,10 +27,6 @@ impl LiteCommand { pub fn is_empty(&self) -> bool { self.parts.is_empty() } - - pub fn is_empty_comments(&self) -> bool { - self.comments.is_empty() - } } #[derive(Debug)]