Re: [PATCH 11/19] tree_entry_interesting: support depth limit
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 13, 2010, 18:10 UTC
- Message-ID
- <7vwrnd1r2s.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1292233616-27692-12-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 22 quoted lines
> diff --git a/dir.c b/dir.c
> index 0987d0c..bb5076c 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -71,6 +71,21 @@ int fill_directory(struct dir_struct *dir, const char **pathspec)
> return len;
> }
>
> +int within_depth(const char *name, int namelen,
> + int depth, int max_depth)
> +{
> + const char *cp = name, *cpe = name + namelen;
> +
> + while (cp < cpe) {
> + if (*cp++ != '/')
> + continue;
> + depth++;
> + if (depth > max_depth)
> + return 0;
> + }
> + return 1;
> +}Makes me almost suspect that it may make more sense to keep track of the "depth" in a similar way as "base" and "baselen" as "traversal state" on the side of the caller so that you do not have to scan the string for slashes over and over again. But given the codeflow, doing so might make the result look too ugly, so I won't recommend that without thinking, though.
Show 19 quoted lines
> diff --git a/tree-walk.c b/tree-walk.c
> index 40a4657..d28de30 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -558,9 +559,17 @@ int tree_entry_interesting(const struct name_entry *entry,
> int pathlen;
> int never_interesting = -1;
>
> - if (!ps || !ps->nr)
> + if (!ps)
> return 1;
>
> + if (!ps->nr) {
> + if (!ps->recursive || ps->max_depth == -1)
> + return 1;
> + return !!within_depth(base, baselen,
> + !!S_ISDIR(entry->mode),
> + ps->max_depth);
> + }This gives different behaviour to between callers that give you NULL as pathspec and callers that give you a pathspec with zero elements. Is this intended? What is the use case (iow, what does an end user give from the command line to experience this difference)?
Show 13 quoted lines
> @@ -571,7 +580,13 @@ int tree_entry_interesting(const struct name_entry *entry, > if (!match_dir_prefix(base, baselen, match, matchlen)) > /* Just a random prefix match */ > continue; > - return 2; > + > + if (!ps->recursive || ps->max_depth == -1) > + return 2; > + > + return !!within_depth(base+matchlen+1, baselen-matchlen-1, > + !!S_ISDIR(entry->mode), > + ps->max_depth); > }
If two pathspecs that overlap with each other (e.g. "Documentation/" and "Documentation/technical") are given, and if the shorter one comes before the longer one in ps[], wouldn't this give you an unexpected result? When inspecting "Documentation/technical/api/foo.txt" with depth limit of 2, if you didn't have "Documentation/" pathspec, you count "api/foo.txt" relative to "Documentation/technical", declare that the path is within limit, and show it. But if you have "Documentation/" in ps[], you look at it, decide "technical/api/foo.txt" is too deep and return false without even looking at "Documentation/technical" that may appear later in ps[], no?