Re: [PATCH] grep: error out if --untracked is used with --cached
- From
Matheus Tavares <matheus.bernardino@usp.br>
- Date
- Feb 8, 2021, 23:21 UTC
- Message-ID
- <20210208232159.100543-1-matheus.bernardino@usp.br>
- In-Reply-To
- <YCGxos2vB6mgHOTA@nand.local>
On Mon, Feb 8, 2021 at 6:48 PM Taylor Blau <me@ttaylorr.com> wrote:
Show 28 quoted lines
>
> On Mon, Feb 08, 2021 at 04:43:28PM -0300, Matheus Tavares wrote:
> > The options --untracked and --cached are not compatible, but if they are
> > used together, grep just silently ignores --cached and searches the
> > working tree. Error out, instead, to avoid any potential confusion.
> >
> > Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> > ---
> > builtin/grep.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/builtin/grep.c b/builtin/grep.c
> > index ca259af441..392acf8cab 100644
> > --- a/builtin/grep.c
> > +++ b/builtin/grep.c
> > @@ -1157,6 +1157,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
> > if (!use_index && (untracked || cached))
> > die(_("--cached or --untracked cannot be used with --no-index"));
> >
> > + if (untracked && cached)
> > + die(_("--untracked cannot be used with --cached"));
> > +
>
> Are these really incompatible? --untracked says that untracked files are
> searched in addition to tracked ones in the working tree.
> --cached says that the index is searched instead of tracked files. From
> my reading, that seems to imply that the combination you're proposing
> getting rid of would mean: "search the index,and untracked files".Yeah, I agree that there is nothing conceptually wrong with the use case "search the index, and untracked files". The problem is that git-grep is currently not able to search both these places in the same execution. In fact, I don't think it can combine working tree and index searches in any way (besides with --assume-unchanged paths).
When --cached is used with --untracked, git-grep silently ignores --cached and searches the working tree only:
$ echo 'cached A' >A $ git add A $ git commit -m A $ echo 'modified A' >A $ echo 'untracked' >B $ git grep --cached --untracked . A:modified A B:untracked
Perhaps, instead of erroring out with this currently invalid combination, should we make it valid by teaching git-grep how to search the two places on a single run? I.e. something like:
diff --git a/builtin/grep.c b/builtin/grep.c index ca259af441..b0e99096ff 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -699,7 +699,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec, } static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec, - int exc_std, int use_index) + int exc_std, int use_index, int untracked_only) { struct dir_struct dir; int i, hit = 0; @@ -712,7 +712,13 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec, fill_directory(&dir, opt->repo->index, pathspec); for (i = 0; i < dir.nr; i++) { - hit |= grep_file(opt, dir.entries[i]->name); + struct dir_entry *ent = dir.entries[i]; + + if (untracked_only && + !index_name_is_other(opt->repo->index, ent->name, ent->len)) + continue; + + hit |= grep_file(opt, ent->name); if (hit && opt->status_only) break; } @@ -1157,9 +1163,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (!use_index && (untracked || cached)) die(_("--cached or --untracked cannot be used with --no-index")); - if (!use_index || untracked) { + if (cached && untracked) { + hit = grep_cache(&opt, &pathspec, 1); + hit |= grep_directory(&opt, &pathspec, !!opt_exclude, 1, 1); + + } else if (!use_index || untracked) { int use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude; - hit = grep_directory(&opt, &pathspec, use_exclude, use_index); + hit |= grep_directory(&opt, &pathspec, use_exclude, use_index, 0); + } else if (0 <= opt_exclude) { die(_("--[no-]exclude-standard cannot be used for tracked contents")); } else if (!list.nr) { With this, the `git grep --cached --untracked .` search from the previous example would produce the following output: A:cached A B:untracked In this case, should we also add an 'untracked:' / 'cached:' prefix to the filenames, like we do for revision searches (e.g. 'HEAD:A:cached A') ?