Re: grep --no-index and pathspec

Junio C Hamano [off-list ref] writes:

Michael J Gruber [off-list ref] writes:

quoted

"grep --no-index" and "grep" have different codepaths for looking up the
files/blobs. If I read that correctly then "grep --no-index -- pathspec"
only does a literal match at the left boundary, whereas for the normal
mode glob patterns are allowed.

CC'ing Junio who created "--no-index".

Anything with --no-index is a quick hack, so I wouldn't be surprised if it
ignored the normal pathspec logic.  As I do not recall the details of the
particular codepath and offhand do not know how involved a change to pay
proper attention to the pathspecs would be, but I suspect that it would be
more appropriate to fix it on top of nd/struct-pathspec topic than writing
the current behaviour down in the documentation outside of BUGS section as
if it were a feature ;-).

This is a band-aid modelled after what builtin/clean.c does to the
returned list from fill_directory(), and it seems to do its job, but I am
quite unhappy about it.

The function fill_directory() already takes a pathspec, albeit in the
degenerate "const char **" form.  Why does its output need further
filtering?

 builtin/grep.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index c3af876..5afee2f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c

@@ -626,6 +626,10 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec)
 
 	fill_directory(&dir, pathspec->raw);
 	for (i = 0; i < dir.nr; i++) {
+		const char *name = dir.entries[i]->name;
+		int namelen = strlen(name);
+		if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL))
+			continue;
 		hit |= grep_file(opt, dir.entries[i]->name);
 		if (hit && opt->status_only)
 			break;

2011/2/12 Junio C Hamano [off-list ref]:

This is a band-aid modelled after what builtin/clean.c does to the
returned list from fill_directory(), and it seems to do its job, but I am
quite unhappy about it.

The function fill_directory() already takes a pathspec, albeit in the
degenerate "const char **" form.  Why does its output need further
filtering?

Because it was designed so? Quotes from 9fc42d6 (Optimize directory
listing with pathspec limiter. - 2007-03-30), which added
simplify_away(), the function that does pathspec filtering for
fill_directory():

    NOTE! This does *not* obviate the need for the caller to do the *exact*
    pathspec match later. It's a first-level filter on "read_directory()", but
    it does not do the full pathspec thing. Maybe it should. But in the
    meantime, builtin-add.c really does need to do first

        read_directory(dir, .., pathspec);
        if (pathspec)
                prune_directory(dir, pathspec, baselen);

    ie the "prune_directory()" part will do the *exact* pathspec pruning,
    while the "read_directory()" will use the pathspec just to do some quick
    high-level pruning of the directories it will recurse into.

Looks good. We could move prune_directory() from builtin/add.c to
dir.c and use it here, but the gain is nothing (except noticing people
some pathspecs do not match any).
-- 
Duy