Skip to content

Commit

Permalink
printer: trim before applying max column windowing
Browse files Browse the repository at this point in the history 
Previously, we were applying the -M/--max-columns flag *before* triming
prefix ASCII whitespace. But this doesn't make a whole lot of sense. We
should be trimming first, but the result of trimming is ultimately what
we'll be printing and that's what -M/--max-columns should be applied to.

Fixes #2458
  • Loading branch information
@BurntSushi
BurntSushi committed Nov 22, 2023
1 parent 89d4962 commit 204a79f
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 15 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ Bug fixes:
Make `-p/--pretty` override flags like `--no-line-number`.
* [BUG #2392](https://github.com/BurntSushi/ripgrep/issues/2392):
Improve global git config parsing of the `excludesFile` field.
* [BUG #2458](https://github.com/BurntSushi/ripgrep/pull/2458):
Make `--trim` run before `-M/--max-columns` takes effect.
* [BUG #2479](https://github.com/BurntSushi/ripgrep/issues/2479):
Add documentation about `.ignore`/`.rgignore` files in parent directories.
* [BUG #2480](https://github.com/BurntSushi/ripgrep/issues/2480):
Expand Down
31 changes: 16 additions & 15 deletions crates/printer/src/standard.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1100,13 +1100,14 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
let mut count = 0;
let mut stepper = LineStep::new(line_term, 0, bytes.len());
while let Some((start, end)) = stepper.next(bytes) {
let line = Match::new(start, end);
let mut line = Match::new(start, end);
self.write_prelude(
self.sunk.absolute_byte_offset() + line.start() as u64,
self.sunk.line_number().map(|n| n + count),
Some(matches[0].start() as u64 + 1),
)?;
count += 1;
self.trim_ascii_prefix(bytes, &mut line);
if self.exceeds_max_columns(&bytes[line]) {
self.write_exceeded_line(bytes, line, matches, &mut midx)?;
} else {
Expand Down Expand Up @@ -1189,12 +1190,12 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
Some(m.start().saturating_sub(line.start()) as u64 + 1),
)?;
count += 1;
self.trim_line_terminator(bytes, &mut line);
self.trim_ascii_prefix(bytes, &mut line);
if self.exceeds_max_columns(&bytes[line]) {
self.write_exceeded_line(bytes, line, &[m], &mut 0)?;
continue;
}
self.trim_line_terminator(bytes, &mut line);
self.trim_ascii_prefix(bytes, &mut line);

while !line.is_empty() {
if m.end() <= line.start() {
Expand Down Expand Up @@ -1246,6 +1247,14 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {

#[inline(always)]
fn write_line(&self, line: &[u8]) -> io::Result<()> {
let line = if !self.config().trim_ascii {
line
} else {
let lineterm = self.searcher.line_terminator();
let full_range = Match::new(0, line.len());
let range = trim_ascii_prefix(lineterm, line, full_range);
&line[range]
};
if self.exceeds_max_columns(line) {
let range = Match::new(0, line.len());
self.write_exceeded_line(
Expand All @@ -1255,7 +1264,8 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
&mut 0,
)?;
} else {
self.write_trim(line)?;
// self.write_trim(line)?;
self.write(line)?;
if !self.has_line_terminator(line) {
self.write_line_term()?;
}
Expand All @@ -1274,7 +1284,8 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
return self.write_line(bytes);
}

let line = Match::new(0, bytes.len());
let mut line = Match::new(0, bytes.len());
self.trim_ascii_prefix(bytes, &mut line);
if self.exceeds_max_columns(bytes) {
self.write_exceeded_line(bytes, line, matches, &mut 0)
} else {
Expand All @@ -1298,7 +1309,6 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
match_index: &mut usize,
) -> io::Result<()> {
self.trim_line_terminator(bytes, &mut line);
self.trim_ascii_prefix(bytes, &mut line);
if matches.is_empty() {
self.write(&bytes[line])?;
return Ok(());
Expand Down Expand Up @@ -1535,15 +1545,6 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
Ok(())
}

fn write_trim(&self, buf: &[u8]) -> io::Result<()> {
if !self.config().trim_ascii {
return self.write(buf);
}
let mut range = Match::new(0, buf.len());
self.trim_ascii_prefix(buf, &mut range);
self.write(&buf[range])
}

fn write(&self, buf: &[u8]) -> io::Result<()> {
self.wtr().borrow_mut().write_all(buf)
}
Expand Down
104 changes: 104 additions & 0 deletions tests/feature.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,110 @@ but Doctor Watson has to have it taken out for him and dusted,
eqnice!(expected, cmd.stdout());
});

rgtest!(f917_trim_multi_standard, |dir: Dir, mut cmd: TestCommand| {
const HAYSTACK: &str = " 0123456789abcdefghijklmnopqrstuvwxyz";
dir.create("haystack", HAYSTACK);
cmd.args(&["--multiline", "--trim", "-r$0", "--no-filename", r"a\n?bc"]);

let expected = "0123456789abcdefghijklmnopqrstuvwxyz\n";
eqnice!(expected, cmd.stdout());
});

rgtest!(f917_trim_max_columns_normal, |dir: Dir, mut cmd: TestCommand| {
const HAYSTACK: &str = " 0123456789abcdefghijklmnopqrstuvwxyz";
dir.create("haystack", HAYSTACK);
cmd.args(&[
"--trim",
"--max-columns-preview",
"-M8",
"--no-filename",
"abc",
]);

let expected = "01234567 [... omitted end of long line]\n";
eqnice!(expected, cmd.stdout());
});

rgtest!(f917_trim_max_columns_matches, |dir: Dir, mut cmd: TestCommand| {
const HAYSTACK: &str = " 0123456789abcdefghijklmnopqrstuvwxyz";
dir.create("haystack", HAYSTACK);
cmd.args(&[
"--trim",
"--max-columns-preview",
"-M8",
"--color=always",
"--colors=path:none",
"--no-filename",
"abc",
]);

let expected = "01234567 [... 1 more match]\n";
eqnice!(expected, cmd.stdout());
});

rgtest!(
f917_trim_max_columns_multi_standard,
|dir: Dir, mut cmd: TestCommand| {
const HAYSTACK: &str = " 0123456789abcdefghijklmnopqrstuvwxyz";
dir.create("haystack", HAYSTACK);
cmd.args(&[
"--multiline",
"--trim",
"--max-columns-preview",
"-M8",
// Force the "slow" printing path without actually
// putting colors in the output.
"--color=always",
"--colors=path:none",
"--no-filename",
r"a\n?bc",
]);

let expected = "01234567 [... 1 more match]\n";
eqnice!(expected, cmd.stdout());
}
);

rgtest!(
f917_trim_max_columns_multi_only_matching,
|dir: Dir, mut cmd: TestCommand| {
const HAYSTACK: &str = " 0123456789abcdefghijklmnopqrstuvwxyz";
dir.create("haystack", HAYSTACK);
cmd.args(&[
"--multiline",
"--trim",
"--max-columns-preview",
"-M8",
"--only-matching",
"--no-filename",
r".*a\n?bc.*",
]);

let expected = "01234567 [... 0 more matches]\n";
eqnice!(expected, cmd.stdout());
}
);

rgtest!(
f917_trim_max_columns_multi_per_match,
|dir: Dir, mut cmd: TestCommand| {
const HAYSTACK: &str = " 0123456789abcdefghijklmnopqrstuvwxyz";
dir.create("haystack", HAYSTACK);
cmd.args(&[
"--multiline",
"--trim",
"--max-columns-preview",
"-M8",
"--vimgrep",
"--no-filename",
r".*a\n?bc.*",
]);

let expected = "1:1:01234567 [... 0 more matches]\n";
eqnice!(expected, cmd.stdout());
}
);

// See: https://github.com/BurntSushi/ripgrep/issues/993
rgtest!(f993_null_data, |dir: Dir, mut cmd: TestCommand| {
dir.create("test", "foo\x00bar\x00\x00\x00baz\x00");
Expand Down

0 comments on commit 204a79f

Please sign in to comment.