Closed acheronfail closed 10 months ago
Getting this right and in a maintainable way is actually rather difficult with what we have. Basically, the requirement is that rgr
needs to do some parsing to get the information it needs, but also allow passing other options to rg
.
These are the flags that rgr
needs to know:
pattern
(positional argument)-e
or --regexp
(mulitple allowed)-f
of --file
(point to a file with regular expressions per line)-E
or --encoding
so we know how to handle file encodings-F
or --fixed-strings
so we know if we should be doing substring searches or regex searchesThese are the flags that are unsupported by rgr
:
--binary
or -uuu
--no-binary
-a
or --text
--no-text
Then, rgr
itself will spawn rg
with the --json
flag, since we parse its JSON output to power its UI. The --json
flag itself disallows some of rg
's other options, but rg
itself handles that so we don't have to. We can also disable the binary-related flags when spawning too, to override any if they were set.
Thus, the requirements for our argument parsing are to "sniff" our cmdline for the flags we need to know, bail out if we see a flag we don't support, and pass everything else through. We must also take into consideration the --config
flag, since if it or RIPGREP_CONFIG_FILE
is set, then we also need to sniff through the config file for any of the options we need.
I'm leaning towards re-writing rgr
's argument parsing logic, since clap
isn't exactly the right tool for this kind of thing, and it's only really being used as a whitelist anyway.
A fair amount of changes, especially some to CI, but they've been tested and things are looking good.
I'm going to create a release with the changes so far and see how it goes.
Fixes #98
To Do:
handle the case of cli arguments overriding one another (i.e.,--sort=path
in config file, and--sort=modified
on command line)unsure about how to solve this nicely...clap
's API isn't flexible enough for provide context about args, or to be able to merge arguments (as far as I know).Investigate looking intoclap
'sArgMatches::ids
method - we may be able to use that to merge