Subject: Re: [PATCH 15/19] tree-diff: no need to call "full" diff_tree_sha1 from show_path()

From: Kirill Smelkov <[email-masked]>
Date: Thu, 27 Mar 2014 18:21:29 +0400

On Mon, Feb 24, 2014 at 08:21:47PM +0400, Kirill Smelkov wrote:
> As described in previous commit, when recursing into sub-trees, we can
> use lower-level tree walker, since its interface is now sha1 based.
> 
> The change is ok, because diff_tree_sha1() only invokes
> __diff_tree_sha1(), and also, if base is empty, try_to_follow_renames().
> But base is not empty here, as we have added a path and '/' before
> recursing.
> 
> Signed-off-by: Kirill Smelkov <[email-masked]>
> Signed-off-by: Junio C Hamano <[email-masked]>
> ---
> 
> ( re-posting without change )
> 
>  tree-diff.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tree-diff.c b/tree-diff.c
> index f90acf5..aea0297 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -114,8 +114,8 @@ static void show_path(struct strbuf *base, struct diff_options *opt,
>  
>  	if (recurse) {
>  		strbuf_addch(base, '/');
> -		diff_tree_sha1(t1 ? t1->entry.sha1 : NULL,
> -			       t2 ? t2->entry.sha1 : NULL, base->buf, opt);
> +		__diff_tree_sha1(t1 ? t1->entry.sha1 : NULL,
> +				 t2 ? t2->entry.sha1 : NULL, base->buf, opt);
>  	}
>  
>  	strbuf_setlen(base, old_baselen);

I've found this does not compile as I've forgot to add __diff_tree_sha1
prototype, and also we are changing naming for __diff_tree_sha1() to
ll_diff_tree_sha1() to follow Git coding style for consistency and
corrections to previous patch, so here goes v2:

(please keep author email)
---- 8< ----
From: Kirill Smelkov <[email-masked]>
Date: Mon, 24 Feb 2014 20:21:47 +0400
Subject: [PATCH v2] tree-diff: no need to call "full" diff_tree_sha1 from show_path()

As described in previous commit, when recursing into sub-trees, we can
use lower-level tree walker, since its interface is now sha1 based.

The change is ok, because diff_tree_sha1() only invokes
ll_diff_tree_sha1(), and also, if base is empty, try_to_follow_renames().
But base is not empty here, as we have added a path and '/' before
recursing.

Signed-off-by: Kirill Smelkov <[email-masked]>
---

Changes since v1:

 - adjust to renaming __diff_tree_sha1 -> ll_diff_tree_sha1;
 - added ll_diff_tree_sha1 prototype as the function is defined below
   here-introduced call-site.

 tree-diff.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tree-diff.c b/tree-diff.c
index 1d02e43..7fbb022 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -6,6 +6,10 @@
 #include "diffcore.h"
 #include "tree.h"
 
+
+static int ll_diff_tree_sha1(const unsigned char *old, const unsigned char *new,
+			     const char *base_str, struct diff_options *opt);
+
 /*
  * Compare two tree entries, taking into account only path/S_ISDIR(mode),
  * but not their sha1's.
@@ -118,8 +122,8 @@ static void show_path(struct strbuf *base, struct diff_options *opt,
 
 	if (recurse) {
 		strbuf_addch(base, '/');
-		diff_tree_sha1(t1 ? t1->entry.sha1 : NULL,
-			       t2 ? t2->entry.sha1 : NULL, base->buf, opt);
+		ll_diff_tree_sha1(t1 ? t1->entry.sha1 : NULL,
+				  t2 ? t2->entry.sha1 : NULL, base->buf, opt);
 	}
 
 	strbuf_setlen(base, old_baselen);
-- 
1.9.rc0.143.g6fd479e