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

--files flag seems to be interpreting regex as a filename (or docs aren't completely clear?) #418

Closed
elirnm opened this issue Mar 23, 2017 · 10 comments
Labels
doc An issue with or an improvement to documentation. question An issue that is lacking clarity on one or more points.

Comments

@elirnm
Copy link
Contributor

elirnm commented Mar 23, 2017

I'm on Windows 7, and running ripgrep with the --files flag seems to be treating my search regex as a filename and then throwing an error.

So rg -w Bob works properly and produces matches, but rg -w Bob --files produces Bob: The system cannot find the file specified. (os error 2) and exits. If I run --files without the actual search term, it works and prints the files to be searched. This happens in both cmd.exe and Powershell.

It occurs to me now that this might be expected behavior, since the search term shouldn't affect the output when rg is run with --files, but if so that should probably be documented in the help page and/or produce a more useful error. This seems implied by the synopsis in https://github.com/BurntSushi/ripgrep/blob/master/doc/rg.1.md, but that doesn't appear in the command line --help output and doesn't appear in the documentation for the --files flag.

Thanks for the great work on ripgrep.

@elirnm elirnm changed the title --files flag seems to be interpreting regex as a filename --files flag seems to be interpreting regex as a filename (or docs aren't completely clear?) Mar 23, 2017
@BurntSushi
Copy link
Owner

@elirnm The behavior you're seeing is correct. --files prints file paths that would be searched, but doesn't actually perform the search, so specifying a pattern doesn't really make any sense. The top of the output of rg -h, rg --help and man rg all include this section:

    rg [OPTIONS] <pattern> [<path> ...]
    rg [OPTIONS] [-e PATTERN | -f FILE ]... [<path> ...]
    rg [OPTIONS] --files [<path> ...]
    rg [OPTIONS] --type-list

Could you suggest something that would make documentation clearer? (Note the third variant above.)

@BurntSushi BurntSushi added doc An issue with or an improvement to documentation. question An issue that is lacking clarity on one or more points. labels Mar 23, 2017
@elirnm
Copy link
Contributor Author

elirnm commented Mar 23, 2017

Hmm, rg --help doesn't look like that for me. All I get that talks about usage is

USAGE:ripgrep [OPTIONS] <pattern> [--] [path]...

which isn't even correct because ripgrep doesn't trigger anything. I'm using 0.5.0.

Exploring a bit, I get the output you gave when I enter rg without any options, but when I include -h or --help I don't get that.

capture

@BurntSushi
Copy link
Owner

That's... interesting. I was using an older version when I responded. It looks like ripgrep's argv parser changed behavior recently? cc @kbknapp

@kbknapp
Copy link
Contributor

kbknapp commented Mar 23, 2017

Interesting. If so it'd be a bug. I'll investigate when I get home. Do you know which version of clap got compiled with it, or if the Cargo.lock was removed before building?

@BurntSushi
Copy link
Owner

@kbknapp ripgrep 0.4.0 was using clap 2.19.3 and ripgrep 0.5.0 is using 2.21.1. (There were some interesting changes I needed to make for the upgrade to work: 95bc678)

@kbknapp
Copy link
Contributor

kbknapp commented Mar 24, 2017

Ok I'll look into it and put out a fix, it would be a major regression to slip by all the tests! Thanks for the ping!

@kbknapp
Copy link
Contributor

kbknapp commented Mar 24, 2017

I've got a PR in to fix this and have added a regression test to ensure it doesn't pop up. Once the PR merges i'll put out v2.22.1

@kbknapp
Copy link
Contributor

kbknapp commented Mar 24, 2017

The new version is out which fixes this regression

@BurntSushi
Copy link
Owner

The latest version of clap sadly does not fix this issue. ripgrep uses a template in addition to a custom usage, which I don't think is accounted for in the regression test in clap-rs/clap#916.

See also #426

I'll see about fixing clap when I get chance.

homu added a commit to clap-rs/clap that referenced this issue Mar 30, 2017
Template fix

Fixes BurntSushi/ripgrep#426 and BurntSushi/ripgrep#418

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/kbknapp/clap-rs/921)
<!-- Reviewable:end -->
@kpp kpp mentioned this issue Mar 30, 2017
kpp added a commit to kpp/ripgrep that referenced this issue Mar 31, 2017
BurntSushi pushed a commit that referenced this issue Mar 31, 2017
@BurntSushi
Copy link
Owner

Closed in #429

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc An issue with or an improvement to documentation. question An issue that is lacking clarity on one or more points.
Projects
None yet
Development

No branches or pull requests

3 participants