Re: [RFC/PATCH] Add --exclude-dir option to git grep

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 27, 2010, 04:53 UTC
Message-ID: <7vzkv33hyn.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100925033530.GA21483@nulllenny.dreamhost.com>

David Ripton <dripton@ripton.net> writes:

Show 18 quoted lines

> It works much like the same option in recent versions of GNU grep.
> Any directory name which matches the option will not be searched.
>
> For example, "git grep --exclude-dir Documentation malloc"
>
> Signed-off-by: David Ripton <dripton@ripton.net>
> ---
> Version 2 of this patch, following Junio's comments:
> strdup() -> xstrdup()
> Fixed a declaration after code.
> Removed basenames from subdirs()
> Do not call subdirs() at all if exclude_dir_list is empty.
>
> Unfortunately the other suggested optimization, moving the original test for
> max_depth < 0 in accept_subdir to the top, turned out to be unsafe.  And
> simplifying subdirs() to only deal with the last subdirectory rather than the
> whole path makes it difficult to exclude a multi-part directory like
> "Documentation/technical".

Doesn't the caller have the full path, relative to the root of the working tree, at that point? That is what the "name" parameter given to pathspec_matches() is, and is given to accept_subdir().

If you are in "x/y/doc" and about to visit "tech", "x/y/doc/tech" is given to you in "name" to see if it is worth going into that hierarchy.

Why isn't it enough to check that "tech", "doc/tech", "y/doc/tech", nor "x/y/doc/tech" appear in the list of excluded patterns? At that point, you know none of "x", "x/y" nor "x/y/doc" appear in the exclude list; otherwise you wouldn't be in "x/y/doc" in the first place, no?

And the beauty of not having to check anything but directory components at the tail end is that you do not have to reallocate the strings nor stuff them in a list at all.

You might need to restructure the loop that walks the index which is a flat list needs to be restructured to match hierarchical tree walking code that lets you skip the entries with the same prefix in one go, though.