Skip to content

Commit

Permalink
Don't try to parse as grep output unless parent process appears to be…
Browse files Browse the repository at this point in the history
… a grep
  • Loading branch information
dandavison committed Nov 19, 2021
1 parent eafe12f commit fc16f88
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 42 deletions.
38 changes: 23 additions & 15 deletions src/handlers/grep.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
// TODO
// Bad parsing: "etc/examples/119-within-line-edits:4:repo=$(mktemp -d)"
// Parsing "Makefile"
// Inspect process tree once
use std::borrow::Cow;

use lazy_static::lazy_static;
Expand All @@ -14,7 +10,7 @@ use crate::delta::{State, StateMachine};
use crate::handlers::{self, ripgrep_json};
use crate::paint::{self, BgShouldFill, StyleSectionSpecifier};
use crate::style::Style;
use crate::utils;
use crate::utils::process;

#[derive(Debug, PartialEq)]
pub struct GrepLine<'b> {
Expand All @@ -40,6 +36,10 @@ struct GrepOutputConfig {
pad_line_number: bool,
}

lazy_static! {
static ref PARENT_GREP_COMMAND: Option<process::GrepCommand> = process::parent_grep_command();
}

lazy_static! {
static ref OUTPUT_CONFIG: GrepOutputConfig = make_output_config();
}
Expand All @@ -51,6 +51,7 @@ impl<'a> StateMachine<'a> {
let mut handled_line = false;

let try_parse = matches!(&self.state, State::Grep | State::Unknown);

if try_parse {
if let Some(mut grep_line) = parse_grep_line(&self.line) {
if matches!(grep_line.line_type, LineType::Ignore) {
Expand Down Expand Up @@ -246,8 +247,10 @@ fn get_code_style_sections<'b>(
}

fn make_output_config() -> GrepOutputConfig {
match utils::process::git_grep_command_options() {
Some((longs, shorts)) if shorts.contains("-W") || longs.contains("--function-context") => {
match &*PARENT_GREP_COMMAND {
Some(process::GrepCommand::GitGrep((longs, shorts)))
if shorts.contains("-W") || longs.contains("--function-context") =>
{
// --function-context is in effect: i.e. the entire function is
// being displayed. In that case we don't render the first line as a
// header, since the second line is the true next line, and it will
Expand All @@ -262,7 +265,9 @@ fn make_output_config() -> GrepOutputConfig {
pad_line_number: true,
}
}
Some((longs, shorts)) if shorts.contains("-p") || longs.contains("--show-function") => {
Some(process::GrepCommand::GitGrep((longs, shorts)))
if shorts.contains("-p") || longs.contains("--show-function") =>
{
// --show-function is in effect, i.e. the function header is being
// displayed, along with matches within the function. Therefore we
// render the first line as a header, but we do not add the navigate
Expand Down Expand Up @@ -372,14 +377,17 @@ $

pub fn parse_grep_line(line: &str) -> Option<GrepLine> {
if line.starts_with('{') {
return ripgrep_json::parse_line(line);
ripgrep_json::parse_line(line)
} else if PARENT_GREP_COMMAND.is_none() {
None
} else {
[
&*GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION,
&*GREP_LINE_REGEX_ASSUMING_NO_INTERNAL_SEPARATOR_CHARS,
]
.iter()
.find_map(|regex| _parse_grep_line(*regex, line))
}
[
&*GREP_LINE_REGEX_ASSUMING_FILE_EXTENSION,
&*GREP_LINE_REGEX_ASSUMING_NO_INTERNAL_SEPARATOR_CHARS,
]
.iter()
.find_map(|regex| _parse_grep_line(*regex, line))
}

pub fn _parse_grep_line<'b>(regex: &Regex, line: &'b str) -> Option<GrepLine<'b>> {
Expand Down
118 changes: 91 additions & 27 deletions src/utils/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@ pub fn git_blame_filename_extension() -> Option<String> {
calling_process_cmdline(ProcInfo::new(), blame::guess_git_blame_filename_extension)
}

pub fn git_grep_command_options() -> Option<(HashSet<String>, HashSet<String>)> {
calling_process_cmdline(ProcInfo::new(), grep::get_grep_options)
#[derive(Debug, PartialEq)]
pub enum GrepCommand {
GitGrep((HashSet<String>, HashSet<String>)),
OtherGrep, // rg, grep, ag, ack, etc
}

pub fn parent_grep_command() -> Option<GrepCommand> {
calling_process_cmdline(ProcInfo::new(), grep::get_grep_command)
}

mod blame {
Expand Down Expand Up @@ -80,33 +86,71 @@ mod blame {
} // mod blame

mod grep {
use std::path::Path;

use super::*;

// Given `... grep --aa val -bc -d val e f -- ...` return
// ({"--aa"}, {"-b", "-c", "-d"})
pub fn get_grep_options(args: &[String]) -> ProcessArgs<(HashSet<String>, HashSet<String>)> {
let mut args = args.iter().map(|s| s.as_str()).skip_while(|s| *s != "grep");
pub fn get_grep_command(args: &[String]) -> ProcessArgs<GrepCommand> {
let mut args = args.iter().map(|s| s.as_str());

match args.next() {
None => ProcessArgs::OtherProcess,
_ => {
let mut longs = HashSet::new();
let mut shorts = HashSet::new();

for s in args {
if s == "--" {
break;
} else if s.starts_with("--") {
longs.insert(s.split('=').next().unwrap().to_owned());
} else if let Some(suffix) = s.strip_prefix('-') {
shorts.extend(suffix.chars().map(|c| format!("-{}", c)));
Some(command) => match Path::new(command).file_stem() {
Some(s) if s.to_str() == Some("git") => {
let mut args = args.skip_while(|s| *s != "grep");
match args.next() {
Some(_) => {
ProcessArgs::Args(GrepCommand::GitGrep(parse_command_option_keys(args)))
}
None => {
// It's git, but not git grep. Don't look at any
// more processes and return not-a-grep-command.
ProcessArgs::ArgError
}
}
}
ProcessArgs::Args((longs, shorts))
Some(s) => match s.to_str() {
Some("rg") | Some("ack") | Some("ag") | Some("pt") | Some("sift")
| Some("ucg") => ProcessArgs::Args(GrepCommand::OtherGrep),
_ => {
// It's not git, and it's not another grep tool. Keep
// looking at other processes.
ProcessArgs::OtherProcess
}
},
_ => {
// Could not parse file stem (not expected); keep looking at
// other processes.
ProcessArgs::OtherProcess
}
},
_ => {
// Empty arguments (not expected); keep looking.
ProcessArgs::OtherProcess
}
}
}
}

// Given `--aa val -bc -d val e f -- ...` return
// ({"--aa"}, {"-b", "-c", "-d"})
fn parse_command_option_keys<'a>(
args: impl Iterator<Item = &'a str>,
) -> (HashSet<String>, HashSet<String>) {
let mut longs = HashSet::new();
let mut shorts = HashSet::new();

for s in args {
if s == "--" {
break;
} else if s.starts_with("--") {
longs.insert(s.split('=').next().unwrap().to_owned());
} else if let Some(suffix) = s.strip_prefix('-') {
shorts.extend(suffix.chars().map(|c| format!("-{}", c)));
}
}
(longs, shorts)
}

struct ProcInfo {
info: sysinfo::System,
}
Expand Down Expand Up @@ -497,10 +541,10 @@ mod tests {
}

#[test]
fn test_get_grep_options() {
fn test_get_grep_command() {
let no_processes = MockProcInfo::with(&[]);
assert_eq!(
calling_process_cmdline(no_processes, grep::get_grep_options),
calling_process_cmdline(no_processes, grep::get_grep_command),
None
);

Expand All @@ -510,8 +554,28 @@ mod tests {
(4, 100, "delta", Some(3)),
]);
assert_eq!(
calling_process_cmdline(parent, grep::get_grep_options),
Some(([].into(), [].into()))
calling_process_cmdline(parent, grep::get_grep_command),
Some(GrepCommand::GitGrep(([].into(), [].into())))
);

let parent = MockProcInfo::with(&[
(2, 100, "-shell", None),
(3, 100, "/usr/local/bin/rg pattern hello.txt", Some(2)),
(4, 100, "delta", Some(3)),
]);
assert_eq!(
calling_process_cmdline(parent, grep::get_grep_command),
Some(GrepCommand::OtherGrep)
);

let parent = MockProcInfo::with(&[
(2, 100, "-shell", None),
(3, 100, "ack.exe pattern hello.txt", Some(2)),
(4, 100, "delta", Some(3)),
]);
assert_eq!(
calling_process_cmdline(parent, grep::get_grep_command),
Some(GrepCommand::OtherGrep)
);

fn set(arg1: &[&str]) -> HashSet<String> {
Expand All @@ -521,18 +585,18 @@ mod tests {
let git_grep_command =
"git grep -ab --function-context -n --show-function -W --foo=val pattern hello.txt";

let expected_result = Some((
let expected_result = Some(GrepCommand::GitGrep((
set(&["--function-context", "--show-function", "--foo"]),
set(&["-a", "-b", "-n", "-W"]),
));
)));

let parent = MockProcInfo::with(&[
(2, 100, "-shell", None),
(3, 100, git_grep_command, Some(2)),
(4, 100, "delta", Some(3)),
]);
assert_eq!(
calling_process_cmdline(parent, grep::get_grep_options),
calling_process_cmdline(parent, grep::get_grep_command),
expected_result
);

Expand All @@ -543,7 +607,7 @@ mod tests {
(5, 100, "delta", Some(4)),
]);
assert_eq!(
calling_process_cmdline(grandparent, grep::get_grep_options),
calling_process_cmdline(grandparent, grep::get_grep_command),
expected_result
);
}
Expand Down

0 comments on commit fc16f88

Please sign in to comment.