Re: [PATCH v3 02/19] Improve documentation and comments regarding directory traversal API

[Adam Spiers <git@adamspiers.org>]

On Wed, Jan 02, 2013 at 12:54:19PM +0000, Adam Spiers wrote:
> On Tue, Jan 1, 2013 at 8:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Adam Spiers <git@adamspiers.org> writes:
> >> diff --git a/dir.c b/dir.c
> >> index ee8e711..89e27a6 100644
> >> --- a/dir.c
> >> +++ b/dir.c
> >> @@ -2,6 +2,8 @@
> >>   * This handles recursive filename detection with exclude
> >>   * files, index knowledge etc..
> >>   *
> >> + * See Documentation/technical/api-directory-listing.txt
> >> + *
> >>   * Copyright (C) Linus Torvalds, 2005-2006
> >>   *            Junio Hamano, 2005-2006
> >>   */
> >> @@ -476,6 +478,10 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)
> >>               die("cannot use %s as an exclude file", fname);
> >>  }
> >>
> >> +/*
> >> + * Loads the per-directory exclude list for the substring of base
> >> + * which has a char length of baselen.
> >> + */
> >>  static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
> >>  {
> >>       struct exclude_list *el;
> >> @@ -486,7 +492,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
> >>           (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))
> >>               return; /* too long a path -- ignore */
> >>
> >> -     /* Pop the ones that are not the prefix of the path being checked. */
> >> +     /* Pop the directories that are not the prefix of the path being checked. */
> >
> > The "one" does not refer to a "directory", but to an "exclude-list".
> 
> No, if that was the case, it would mean that multiple exclude lists
> would be popped, but that is not the case here (prior to v4).

Sorry, I meant prior to v3 11/19.

> >         Pop the ones that are not for parent directories of the path
> >         being checked
> 
> Better would be:
> 
>     Pop the entries within the EXCL_DIRS exclude list which originate
>     from directories not in the prefix of the path being checked.
> 
> although as previously stated, the v4 series I have been holding off
> from submitting (in order not to distract you from a maint release)
> actually changes this behaviour so EXCL_DIRS becomes an exclude_group of
> multiple exclude_lists, one per directory.  So in v4, multiple
> exclude_lists *will* be popped.  I'll tweak the comment in v4 to make
> this clear.

Again, I got confused and forgot that I already included the switch to
exclude_list_groups as v3 11/19.  But since the patch being discussed
here is v3 02/19 which precedes it, everything I said still applies.