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.

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

Search does not stop with --max-count and --context with matches in the context lines #1380

Closed
dsvf opened this issue Sep 16, 2019 · 3 comments
Labels
bug A bug. rollup A PR that has been merged with many others in a rollup.

Comments

@dsvf
Copy link

dsvf commented Sep 16, 2019

What version of ripgrep are you using?

ripgrep 11.0.2
-SIMD -AVX (compiled)
+SIMD -AVX (runtime)

How did you install ripgrep?

Github binary release

What operating system are you using ripgrep on?

Linux, RHEL 7.7

Describe your question, feature request, or bug.

Unexpected behavior when combining --max-count with --context, if there are additional matches in the context. This is not really a bug, more like inconsistent behavior.

I am searching for the first match in one file and want to print the next n lines.
If there are no additional matches in the context, then the search output stops after the first result and prints the next n lines.
If there is an additional match within the next n lines, then (it seems like) rg treats this as a match and restarts its internal context line counter, printing an additional n lines.

I am aware that --max-count is a limit on per-line matches. But it works for some input files and it does not work for some other files, which is unexpected. Also, it appears somewhat similar to #402.

If this is a bug, what are the steps to reproduce the behavior?

> cat rgtest.txt
a
b
c
d
e
d
e
d
e
d
e

If this is a bug, what is the actual behavior?

Working as expected: Print first result and next line:

> rg -m 1 -A 1 d rgtest.txt
4:d
5-e

Working as expected: Do "print first result and next line" twice:

> rg -m 2 -A 1 d rgtest.txt
4:d
5-e
6:d
7-e

Unexpected: Print first result and next two lines

> rg -m 1 -A 2 d rgtest.txt
4:d
5-e
6:d
7-e
8:d
9-e
10:d
11-e
12-

Expected result:

4:d
5-e
6:d

If this is a bug, what is the expected behavior?

Not a bug, but a wish for consistency (i.e. please fix it for my specific use case)

@BurntSushi
Copy link
Owner

This does actually look like a bug to me. Thanks!

@BurntSushi BurntSushi added the bug A bug. label Sep 16, 2019
@jakubadamw
Copy link
Contributor

@BurntSushi, I was looking at what it would take to sensibly fix this and have come up with a rather invasive change that would involve making the matched family of methods in grep-searcher's Sink and Core return an enum of {Quit, MatchAndContext, ContextOnly} (naming to be bikeshedded as needed) instead of a bool.

fn matched(
&mut self,
_searcher: &Searcher,
_mat: &SinkMatch,
) -> Result<bool, Self::Error>;

Does that sound reasonable?

@BurntSushi
Copy link
Owner

I haven't investigated this yet, so I'm not sure if it's reasonable or not. I'd certainly prefer to avoid breaking changes, and in particular, avoid complicating the API like this. But as I said, I haven't dug into this so I'm not sure.

@BurntSushi BurntSushi added the rollup A PR that has been merged with many others in a rollup. label May 31, 2021
BurntSushi pushed a commit that referenced this issue May 31, 2021
In the case where after-context is requested with a match count limit,
we need to be careful not to reset the state tracking the remaining
context lines.

Fixes #1380, Closes #1642
BurntSushi pushed a commit that referenced this issue Jun 1, 2021
In the case where after-context is requested with a match count limit,
we need to be careful not to reset the state tracking the remaining
context lines.

Fixes #1380, Closes #1642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug. rollup A PR that has been merged with many others in a rollup.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants