Re: [PATCH 2/7] Fix ignoring of pathspecs with rev-list --objects

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

On Wed, Aug 25, 2010 at 12:53 PM, Elijah Newren <newren@gmail.com> wrote:
> @@ -84,7 +85,24 @@ static void process_tree(struct rev_info *revs,
>
>        init_tree_desc(&desc, tree->buffer, tree->size);
>
> -       while (tree_entry(&desc, &entry)) {
> +       for (; desc.size; update_tree_entry(&desc)) {
> +               entry = desc.entry;
> +
> +               if (!all_interesting) {
> +                       char *full_path = path_name(path, name);
> +                       int full_path_len = strlen(full_path);
> +                       int showit = tree_entry_interesting(&desc,
> full_path, full_path_len,
> +                                                           &revs->diffopt);
> +                       free(full_path);

I wonder if we can avoid xmalloc/free so many times here. If full_path
is unchanged, how about moving it outside the loop?

> diff --git a/tree-diff.c b/tree-diff.c
> index cd659c6..2fb670b 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -91,7 +91,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct
> tree_desc *t2, const
>  *  - zero for no
>  *  - negative for "no, and no subsequent entries will be either"
>  */
> -static int tree_entry_interesting(struct tree_desc *desc, const char *base,
> int baselen, struct diff_options *opt)
> +int tree_entry_interesting(struct tree_desc *desc, const char *base, int
> baselen, struct diff_options *opt)
>  {
>        const char *path;
>        const unsigned char *sha1;

While at it, can you please also fix its comments? The comments say
pathspec while what it uses is actually path prefix. Maybe something
like this

diff --git a/tree-diff.c b/tree-diff.c
index 3c34c77..514dbca 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -83,7 +83,7 @@ static int compare_tree_entry(struct tree_desc *t1,
struct tree_desc *t2, const
 }

 /*
- * Is a tree entry interesting given the pathspec we have?
+ * Is a tree entry interesting given the path prefix we have?
  *
  * Return:
  *  - 2 for "yes, and all subsequent entries will be"


This patch is good stuff. Please be informed I will steal this patch
for my narrow use.
-- 
Duy