From 946dc36a98d05717edc0a757e37187d36ce5dc59 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Mon, 20 Dec 2021 19:31:16 -0500 Subject: [PATCH 1/8] Failing test of git grep parsing --- src/handlers/grep.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index c3d6da6cd..76e37132d 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -684,6 +684,17 @@ mod tests { submatches: None, }) ); + + assert_eq!( + parse_grep_line(r#"foo.rs-12- .x-"#), + Some(GrepLine { + path: "foo.rs".into(), + line_number: Some(12), + line_type: LineType::Context, + code: r#" .x-"#.into(), + submatches: None, + }) + ); } #[test] From b9fc7f8e7097f28914305b8d55526cb68ce5a89e Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Mon, 20 Dec 2021 20:04:06 -0500 Subject: [PATCH 2/8] Refactor --- src/handlers/grep.rs | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index 76e37132d..a05fd0591 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -348,30 +348,35 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex { } }; + let separator = r#" + (?: + ( + : # 2. match marker + (?:([0-9]+):)? # 3. optional: line number followed by second match marker + ) + | + ( + - # 4. nomatch marker + (?:([0-9]+)-)? # 5. optional: line number followed by second nomatch marker + ) + | + ( + = # 6. match marker + (?:([0-9]+)=)? # 7. optional: line number followed by second header marker + ) + ) + "#; + Regex::new(&format!( "(?x) ^ {file_path} -(?: - ( - : # 2. match marker - (?:([0-9]+):)? # 3. optional: line number followed by second match marker - ) - | - ( - - # 4. nomatch marker - (?:([0-9]+)-)? # 5. optional: line number followed by second nomatch marker - ) - | - ( - = # 6. match marker - (?:([0-9]+)=)? # 7. optional: line number followed by second header marker - ) -) +{separator} (.*) # 8. code (i.e. line contents) $ ", - file_path = file_path + file_path = file_path, + separator = separator, )) .unwrap() } From 6f52fdca0e83da8d1bbbb1886466308fbab502db Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Mon, 20 Dec 2021 20:06:03 -0500 Subject: [PATCH 3/8] Demand a non-space before extension --- src/handlers/grep.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index a05fd0591..8787691ae 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -333,7 +333,7 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex { ( # 1. file name (colons not allowed) [^:|\ ] # try to be strict about what a file path can start with [^:]* # anything - \.[^.\ :=-]{1,6} # extension + [^\ ]\.[^.\ :=-]{1,6} # extension ) " } From 374151c5464920aa897175b329a48e0d81db6400 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Mon, 20 Dec 2021 20:23:41 -0500 Subject: [PATCH 4/8] Add another test case --- src/handlers/grep.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index 8787691ae..b824a9891 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -700,6 +700,17 @@ mod tests { submatches: None, }) ); + + assert_eq!( + parse_grep_line(r#"foo.rs-12-.x-"#), + Some(GrepLine { + path: "foo.rs".into(), + line_number: Some(12), + line_type: LineType::Context, + code: r#".x-"#.into(), + submatches: None, + }) + ); } #[test] From 7b3cf478059104c0ea27e89f4080bcbdc8f1108e Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Mon, 20 Dec 2021 20:18:32 -0500 Subject: [PATCH 5/8] New grep parse heuristic If something like any of the following are seen then that is assumed to be a file name with an extension followed by a line number. I.e. we do not support file names with such patterns internally. .xx-7- .xx=7= .xx:7: --- src/handlers/grep.rs | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index b824a9891..607caf6d0 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -288,10 +288,16 @@ fn make_output_config() -> GrepOutputConfig { } enum GrepLineRegex { + FilePathWithFileExtensionAndLineNumber, FilePathWithFileExtension, FilePathWithoutSeparatorCharacters, } +lazy_static! { + static ref GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION_AND_LINE_NUMBER: Regex = + make_grep_line_regex(GrepLineRegex::FilePathWithFileExtensionAndLineNumber); +} + lazy_static! { static ref GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION: Regex = make_grep_line_regex(GrepLineRegex::FilePathWithFileExtension); @@ -328,7 +334,8 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex { // Make-7-file-7-xxx let file_path = match regex_variant { - GrepLineRegex::FilePathWithFileExtension => { + GrepLineRegex::FilePathWithFileExtensionAndLineNumber + | GrepLineRegex::FilePathWithFileExtension => { r" ( # 1. file name (colons not allowed) [^:|\ ] # try to be strict about what a file path can start with @@ -348,7 +355,29 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex { } }; - let separator = r#" + let separator = match regex_variant { + GrepLineRegex::FilePathWithFileExtensionAndLineNumber => { + r#" + (?: + ( + : # 2. match marker + ([0-9]+): # 3. line number followed by second match marker + ) + | + ( + - # 4. nomatch marker + ([0-9]+)- # 5. line number followed by second nomatch marker + ) + | + ( + = # 6. match marker + ([0-9]+)= # 7. line number followed by second header marker + ) + ) + "# + } + _ => { + r#" (?: ( : # 2. match marker @@ -365,7 +394,9 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex { (?:([0-9]+)=)? # 7. optional: line number followed by second header marker ) ) - "#; + "# + } + }; Regex::new(&format!( "(?x) @@ -387,6 +418,7 @@ pub fn parse_grep_line(line: &str) -> Option { } else { match &*process::calling_process() { process::CallingProcess::GitGrep(_) | process::CallingProcess::OtherGrep => [ + &*GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION_AND_LINE_NUMBER, &*GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION, &*GREP_LINE_REGEX_ASSUMING_NO_INTERNAL_SEPARATOR_CHARS, ] From 25b93c8c66d197a33d23e1ccd1cf5573dee745ff Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Mon, 20 Dec 2021 20:26:58 -0500 Subject: [PATCH 6/8] Clippy: rename --- src/handlers/grep.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index 607caf6d0..27db2bc38 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -288,24 +288,24 @@ fn make_output_config() -> GrepOutputConfig { } enum GrepLineRegex { - FilePathWithFileExtensionAndLineNumber, - FilePathWithFileExtension, - FilePathWithoutSeparatorCharacters, + WithFileExtensionAndLineNumber, + WithFileExtension, + WithoutSeparatorCharacters, } lazy_static! { static ref GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION_AND_LINE_NUMBER: Regex = - make_grep_line_regex(GrepLineRegex::FilePathWithFileExtensionAndLineNumber); + make_grep_line_regex(GrepLineRegex::WithFileExtensionAndLineNumber); } lazy_static! { static ref GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION: Regex = - make_grep_line_regex(GrepLineRegex::FilePathWithFileExtension); + make_grep_line_regex(GrepLineRegex::WithFileExtension); } lazy_static! { static ref GREP_LINE_REGEX_ASSUMING_NO_INTERNAL_SEPARATOR_CHARS: Regex = - make_grep_line_regex(GrepLineRegex::FilePathWithoutSeparatorCharacters); + make_grep_line_regex(GrepLineRegex::WithoutSeparatorCharacters); } // See tests for example grep lines @@ -334,8 +334,7 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex { // Make-7-file-7-xxx let file_path = match regex_variant { - GrepLineRegex::FilePathWithFileExtensionAndLineNumber - | GrepLineRegex::FilePathWithFileExtension => { + GrepLineRegex::WithFileExtensionAndLineNumber | GrepLineRegex::WithFileExtension => { r" ( # 1. file name (colons not allowed) [^:|\ ] # try to be strict about what a file path can start with @@ -344,7 +343,7 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex { ) " } - GrepLineRegex::FilePathWithoutSeparatorCharacters => { + GrepLineRegex::WithoutSeparatorCharacters => { r" ( # 1. file name (colons not allowed) [^:|\ =-] # try to be strict about what a file path can start with @@ -356,7 +355,7 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex { }; let separator = match regex_variant { - GrepLineRegex::FilePathWithFileExtensionAndLineNumber => { + GrepLineRegex::WithFileExtensionAndLineNumber => { r#" (?: ( From 05ba9013ec9063d2cde4bfb9f27d01685b01f6f7 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Sun, 2 Jan 2022 23:55:50 +0000 Subject: [PATCH 7/8] Failing grep parse test cases --- src/handlers/grep.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index 27db2bc38..8affdaf0e 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -690,6 +690,28 @@ mod tests { submatches: None, }) ); + + assert_eq!( + parse_grep_line(r#"aaa/bbb.scala- s"xxx.yyy.zzz: $ccc ddd""#), + Some(GrepLine { + path: "aaa/bbb.scala".into(), + line_number: None, + line_type: LineType::Context, + code: r#" s"xxx.yyy.zzz: $ccc ddd""#.into(), + submatches: None, + }) + ); + + assert_eq!( + parse_grep_line(r#"aaa/bbb.scala- val atRegex = Regex.compile("(@.*)|(-shdw@.*)""#), + Some(GrepLine { + path: "aaa/bbb.scala".into(), + line_number: None, + line_type: LineType::Context, + code: r#" val atRegex = Regex.compile("(@.*)|(-shdw@.*)""#.into(), + submatches: None, + }) + ); } #[test] From 399f81e19bb9c6942c47a077c648101aa585f010 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Sun, 2 Jan 2022 23:56:09 +0000 Subject: [PATCH 8/8] Fix grep parse failing cases --- src/handlers/grep.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index 8affdaf0e..3bff9c447 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -290,6 +290,7 @@ fn make_output_config() -> GrepOutputConfig { enum GrepLineRegex { WithFileExtensionAndLineNumber, WithFileExtension, + WithFileExtensionNoSpaces, WithoutSeparatorCharacters, } @@ -298,6 +299,11 @@ lazy_static! { make_grep_line_regex(GrepLineRegex::WithFileExtensionAndLineNumber); } +lazy_static! { + static ref GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION_NO_SPACES: Regex = + make_grep_line_regex(GrepLineRegex::WithFileExtensionNoSpaces); +} + lazy_static! { static ref GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION: Regex = make_grep_line_regex(GrepLineRegex::WithFileExtension); @@ -343,6 +349,14 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex { ) " } + GrepLineRegex::WithFileExtensionNoSpaces => { + r" + ( # 1. file name (colons not allowed) + [^:|\ ]+ # try to be strict about what a file path can start with + [^\ ]\.[^.\ :=-]{1,6} # extension + ) + " + } GrepLineRegex::WithoutSeparatorCharacters => { r" ( # 1. file name (colons not allowed) @@ -418,6 +432,7 @@ pub fn parse_grep_line(line: &str) -> Option { match &*process::calling_process() { process::CallingProcess::GitGrep(_) | process::CallingProcess::OtherGrep => [ &*GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION_AND_LINE_NUMBER, + &*GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION_NO_SPACES, &*GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION, &*GREP_LINE_REGEX_ASSUMING_NO_INTERNAL_SEPARATOR_CHARS, ]