Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix grep parse bugs #865

Merged
merged 8 commits into from
Jan 5, 2022
Merged

Fix grep parse bugs #865

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
946dc36
Failing test of git grep parsing
dandavison Dec 21, 2021
b9fc7f8
Refactor
dandavison Dec 21, 2021
6f52fdc
Demand a non-space before extension
dandavison Dec 21, 2021
374151c
Add another test case
dandavison Dec 21, 2021
7b3cf47
New grep parse heuristic
dandavison Dec 21, 2021
25b93c8
Clippy: rename
dandavison Dec 21, 2021
05ba901
Failing grep parse test cases
dandavison Jan 2, 2022
399f81e
Fix grep parse failing cases
dandavison Jan 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 119 additions & 24 deletions src/handlers/grep.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,18 +288,30 @@ fn make_output_config() -> GrepOutputConfig {
}

enum GrepLineRegex {
FilePathWithFileExtension,
FilePathWithoutSeparatorCharacters,
WithFileExtensionAndLineNumber,
WithFileExtension,
WithFileExtensionNoSpaces,
WithoutSeparatorCharacters,
}

lazy_static! {
static ref GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION_AND_LINE_NUMBER: Regex =
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::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
Expand Down Expand Up @@ -328,16 +340,24 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex {
// Make-7-file-7-xxx

let file_path = match regex_variant {
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
[^:]* # anything
\.[^.\ :=-]{1,6} # extension
[^\ ]\.[^.\ :=-]{1,6} # extension
)
"
}
GrepLineRegex::FilePathWithoutSeparatorCharacters => {
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)
[^:|\ =-] # try to be strict about what a file path can start with
Expand All @@ -348,30 +368,59 @@ fn make_grep_line_regex(regex_variant: GrepLineRegex) -> Regex {
}
};

let separator = match regex_variant {
GrepLineRegex::WithFileExtensionAndLineNumber => {
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
(?:([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()
}
Expand All @@ -382,6 +431,8 @@ pub fn parse_grep_line(line: &str) -> Option<GrepLine> {
} 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_NO_SPACES,
&*GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION,
&*GREP_LINE_REGEX_ASSUMING_NO_INTERNAL_SEPARATOR_CHARS,
]
Expand Down Expand Up @@ -654,6 +705,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]
Expand Down Expand Up @@ -684,6 +757,28 @@ 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,
})
);

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]
Expand Down