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

[Junio C Hamano <gitster@pobox.com>]

Adam Spiers <git@adamspiers.org> writes:

> diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
> index 0356d25..944fc39 100644
> --- a/Documentation/technical/api-directory-listing.txt
> +++ b/Documentation/technical/api-directory-listing.txt
> @@ -9,8 +9,11 @@ Data structure
>  --------------
>  
>  `struct dir_struct` structure is used to pass directory traversal
> -options to the library and to record the paths discovered.  The notable
> -options are:
> +options to the library and to record the paths discovered.  A single
> +`struct dir_struct` is used regardless of whether or not the traversal
> +recursively descends into subdirectories.

I am somewhat lukewarm on this part of the change.

The added "regardless of..." does not seem to add as much value as
the two extra lines the patch spends.  If we say something like:

	A `struct dir_struct` structure is used to pass options to
        traverse directories recursively, and to record all the
        paths discovered by the traversal.

it might be much more direct and informative, I suspect, though.

After all, the word "traversal" pretty much implies that the library
goes in and out of the directories recursively.

> @@ -39,7 +42,7 @@ options are:
>  	If set, recurse into a directory that looks like a git
>  	directory.  Otherwise it is shown as a directory.
>  
> -The result of the enumeration is left in these fields::
> +The result of the enumeration is left in these fields:

Good eyes.

> 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".

	Pop the ones that are not for parent directories of the path
	being checked

perhaps?