Skip to content

Commit

Permalink
printer: fix duplicative replacement in multiline mode
Browse files Browse the repository at this point in the history
This furthers our kludge of dealing with PCRE2's look-around in the
printer. Because of our bad abstraction boundaries, we added a kludge to
deal with PCRE2 look-around by extending the bytes we search by a fixed
amount to hopefully permit any look-around to operate. But because of
that kludge, we wind up over extending ourselves in some cases and
dragging along those extra bytes.

We had fixed this for simple searching by simply rejecting any matches
past the end point. But we didn't do the same for replacements. So this
commit extends our kludge to replacements.

Thanks to @sonohgong for diagnosing the problem and proposing a fix. I
mostly went with their solution, but adding the new replacement routine
as an internal helper rather than a new APIn in the 'grep-matcher'
crate.

Fixes #2095, Fixes #2208
  • Loading branch information
BurntSushi committed May 11, 2022
1 parent 4dc6c73 commit 91afd42
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 20 deletions.
70 changes: 50 additions & 20 deletions crates/printer/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,26 +82,26 @@ impl<M: Matcher> Replacer<M> {
dst.clear();
matches.clear();

matcher
.replace_with_captures_at(
subject,
range.start,
caps,
dst,
|caps, dst| {
let start = dst.len();
caps.interpolate(
|name| matcher.capture_index(name),
subject,
replacement,
dst,
);
let end = dst.len();
matches.push(Match::new(start, end));
true
},
)
.map_err(io::Error::error_message)?;
replace_with_captures_in_context(
matcher,
subject,
range.clone(),
caps,
dst,
|caps, dst| {
let start = dst.len();
caps.interpolate(
|name| matcher.capture_index(name),
subject,
replacement,
dst,
);
let end = dst.len();
matches.push(Match::new(start, end));
true
},
)
.map_err(io::Error::error_message)?;
}
Ok(())
}
Expand Down Expand Up @@ -458,3 +458,33 @@ pub fn trim_line_terminator(
*line = line.with_end(end);
}
}

/// Like `Matcher::replace_with_captures_at`, but accepts an end bound.
///
/// See also: `find_iter_at_in_context` for why we need this.
fn replace_with_captures_in_context<M, F>(
matcher: M,
bytes: &[u8],
range: std::ops::Range<usize>,
caps: &mut M::Captures,
dst: &mut Vec<u8>,
mut append: F,
) -> Result<(), M::Error>
where
M: Matcher,
F: FnMut(&M::Captures, &mut Vec<u8>) -> bool,
{
let mut last_match = range.start;
matcher.captures_iter_at(bytes, range.start, caps, |caps| {
let m = caps.get(0).unwrap();
if m.start() >= range.end {
return false;
}
dst.extend(&bytes[last_match..m.start()]);
last_match = m.end();
append(caps, dst)
})?;
let end = std::cmp::min(bytes.len(), range.end);
dst.extend(&bytes[last_match..end]);
Ok(())
}
74 changes: 74 additions & 0 deletions tests/regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1044,3 +1044,77 @@ rgtest!(r1891, |dir: Dir, mut cmd: TestCommand| {
// happen when each match needs to be detected.
eqnice!("1:\n2:\n2:\n", cmd.args(&["-won", "", "test"]).stdout());
});

// See: https://github.com/BurntSushi/ripgrep/issues/2095
rgtest!(r2095, |dir: Dir, mut cmd: TestCommand| {
dir.create(
"test",
"#!/usr/bin/env bash
zero=one
a=one
if true; then
a=(
a
b
c
)
true
fi
a=two
b=one
});
",
);
cmd.args(&[
"--line-number",
"--multiline",
"--only-matching",
"--replace",
"${value}",
r"^(?P<indent>\s*)a=(?P<value>(?ms:[(].*?[)])|.*?)$",
"test",
]);
let expected = "4:one
8:(
9: a
10: b
11: c
12: )
15:two
";
eqnice!(expected, cmd.stdout());
});

// See: https://github.com/BurntSushi/ripgrep/issues/2208
rgtest!(r2208, |dir: Dir, mut cmd: TestCommand| {
dir.create("test", "# Compile requirements.txt files from all found or specified requirements.in files (compile).
# Use -h to include hashes, -u dep1,dep2... to upgrade specific dependencies, and -U to upgrade all.
pipc () { # [-h] [-U|-u <pkgspec>[,<pkgspec>...]] [<reqs-in>...] [-- <pip-compile-arg>...]
emulate -L zsh
unset REPLY
if [[ $1 == --help ]] { zpy $0; return }
[[ $ZPY_PROCS ]] || return
local gen_hashes upgrade upgrade_csv
while [[ $1 == -[hUu] ]] {
if [[ $1 == -h ]] { gen_hashes=--generate-hashes; shift }
if [[ $1 == -U ]] { upgrade=1; shift }
if [[ $1 == -u ]] { upgrade=1; upgrade_csv=$2; shift 2 }
}
}
");
cmd.args(&[
"-N",
"-U",
"-r", "$usage",
r#"^(?P<predoc>\n?(# .*\n)*)(alias (?P<aname>pipc)="[^"]+"|(?P<fname>pipc) \(\) \{)( #(?P<usage> .+))?"#,
"test",
]);
let expected = " [-h] [-U|-u <pkgspec>[,<pkgspec>...]] [<reqs-in>...] [-- <pip-compile-arg>...]\n";
eqnice!(expected, cmd.stdout());
});

0 comments on commit 91afd42

Please sign in to comment.