Skip to content

Commit

Permalink
printer: fix \r\n line terminator handling
Browse files Browse the repository at this point in the history
This fixes a bug where it was assumed that 'is_suffix' when CRLF
handling was enabled mean that '\r\n' was present. But that's not the
case, and it is intentional that 'is_suffix' only looks for '\n'. (Which
is why #1803 wasn't taken, which tries to fix this by changing
'is_suffix'.)

Fixes #1765, Closes #1803
  • Loading branch information
BurntSushi committed May 30, 2021
1 parent 922a298 commit d93480e
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ Bug fixes:
Clarify that CLI invocation must always be valid, regardless of config file.
* [BUG #1741](https://github.com/BurntSushi/ripgrep/issues/1741):
Fix stdin detection when using PowerShell in UNIX environments.
* [BUG #1765](https://github.com/BurntSushi/ripgrep/issues/1765):
Fix panic when `--crlf` is used in some cases.
* [BUG #1816](https://github.com/BurntSushi/ripgrep/issues/1816):
Add documentation for glob alternate syntax, e.g., `{a,b,..}`.
* [BUG #1847](https://github.com/BurntSushi/ripgrep/issues/1847):
Expand Down
33 changes: 29 additions & 4 deletions crates/printer/src/standard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1474,7 +1474,10 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
let lineterm = self.searcher.line_terminator();
if lineterm.is_suffix(&buf[*line]) {
let mut end = line.end() - 1;
if lineterm.is_crlf() && buf[end - 1] == b'\r' {
if lineterm.is_crlf()
&& end > 0
&& buf.get(end - 1) == Some(&b'\r')
{
end -= 1;
}
*line = line.with_end(end);
Expand Down Expand Up @@ -1547,11 +1550,12 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {

#[cfg(test)]
mod tests {
use grep_regex::RegexMatcher;
use grep_matcher::LineTerminator;
use grep_regex::{RegexMatcher, RegexMatcherBuilder};
use grep_searcher::SearcherBuilder;
use termcolor::NoColor;
use termcolor::{Ansi, NoColor};

use super::{Standard, StandardBuilder};
use super::{ColorSpecs, Standard, StandardBuilder};

const SHERLOCK: &'static str = "\
For the Doctor Watsons of this world, as opposed to the Sherlock
Expand All @@ -1576,6 +1580,10 @@ and exhibited clearly, with a label attached.\
String::from_utf8(printer.get_mut().get_ref().to_owned()).unwrap()
}

fn printer_contents_ansi(printer: &mut Standard<Ansi<Vec<u8>>>) -> String {
String::from_utf8(printer.get_mut().get_ref().to_owned()).unwrap()
}

#[test]
fn reports_match() {
let matcher = RegexMatcher::new("Sherlock").unwrap();
Expand Down Expand Up @@ -3388,4 +3396,21 @@ and xxx clearly, with a label attached.
";
assert_eq_printed!(expected, got);
}

#[test]
fn regression_search_empty_with_crlf() {
let matcher =
RegexMatcherBuilder::new().crlf(true).build(r"x?").unwrap();
let mut printer = StandardBuilder::new()
.color_specs(ColorSpecs::default_with_color())
.build(Ansi::new(vec![]));
SearcherBuilder::new()
.line_terminator(LineTerminator::crlf())
.build()
.search_reader(&matcher, &b"\n"[..], printer.sink(&matcher))
.unwrap();

let got = printer_contents_ansi(&mut printer);
assert!(!got.is_empty());
}
}
10 changes: 10 additions & 0 deletions tests/regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,16 @@ use B;
eqnice!("2\n", cmd.stdout());
});

// See: https://github.com/BurntSushi/ripgrep/issues/1765
rgtest!(r1765, |dir: Dir, mut cmd: TestCommand| {
dir.create("test", "\n");
// We need to add --color=always here to force the failure, since the bad
// code path is only triggered when colors are enabled.
cmd.args(&[r"x?", "--crlf", "--color", "always"]);

assert!(!cmd.stdout().is_empty());
});

rgtest!(r1866, |dir: Dir, mut cmd: TestCommand| {
dir.create("test", "foobar\nfoobar\nfoo quux");
cmd.args(&[
Expand Down

0 comments on commit d93480e

Please sign in to comment.