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

Allow -o, --only-matching work together with -r, --replace #593

Closed

Conversation

beyondcompute
Copy link
Contributor

Hello!

Thank you for your amazing work on much-needed and super-fast fully-featured regex search utility which is ripgrep.

However I’ve been missing a bit a feature similar to ack’s helpful --output which allows printing only certain capture groups. That feature is discussed in #320 and the discussion offers a solution. However it requires matching an entire line, which still might seem a bit less straightforward for some users.

Now, there is probably a reason why -o is made to conflict with -r so please excuse me if this PR is really out-of-place. However after implementing it, upon superficial gaze, things seem to be working fine:

Examples outputs

ripgrep

$ ./target/debug/rg 'of (\w+)' -Nor '$1' ./tests/hay.rs 
this
detective
luck
straw
cigar

ack

$ ack 'of (\w+)' --output '$1' ./tests/hay.rs 
this
detective
luck
straw
cigar

So I hope this has a chance of being considered. Thank you again!

@BurntSushi
Copy link
Owner

I can't quite remember why we set the flags to conflict. There might be some subtle corner cases. I'd have to go back to that PR and read.

With that said, I'm not in principle against this, and I feel like this combination of flags makes sense.

@ayosec
Copy link

ayosec commented Oct 17, 2017

I can't quite remember why we set the flags to conflict.

I guess that the conflict was set because the original implementation didn't add the if self.only_matching condition in the if self.replace.is_some() block.

The combination of -r and -o seems pretty natural for me. Actually, I tried to write rg -o 'Enqueued (\S+)' -r '$1' a few minutes ago, and then I saw that it was impossible :-( .

@BurntSushi
Copy link
Owner

I rebased this on top of master and merged it in f887bc1. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants