Re: [PATCH] grep: error out if --untracked is used with --cached
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:
Show changes to builtin/grep.c +15 −4
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') ?
Matheus Tavares
