Re: [PATCH 04/17] builtin_diff_tree(): make it obvious that function wants two entries

[Michael Haggerty]

On 05/21/2013 07:27 PM, Junio C Hamano wrote:
> Michael Haggerty  writes:
> 
>> Instead of accepting an array and using exactly two elements from the
>> array, take two single (struct object_array_entry *) arguments.
>>
>> Signed-off-by: Michael Haggerty 
>> ---
>>  builtin/diff.c | 18 +-
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/builtin/diff.c b/builtin/diff.c
>> index 8c2af6c..ba68c6c 100644
>> --- a/builtin/diff.c
>> +++ b/builtin/diff.c
>> @@ -153,7 +153,8 @@ static int builtin_diff_index(struct rev_info *revs,
>>  
>>  static int builtin_diff_tree(struct rev_info *revs,
>>   int argc, const char **argv,
>> - struct object_array_entry *ent)
>> + struct object_array_entry *ent0,
>> + struct object_array_entry *ent1)
>>  {
>>  const unsigned char *(sha1[2]);
>>  int swap = 0;
>> @@ -161,13 +162,13 @@ static int builtin_diff_tree(struct rev_info *revs,
>>  if (argc > 1)
>>  usage(builtin_diff_usage);
>>  
>> -/* We saw two trees, ent[0] and ent[1].
>> - * if ent[1] is uninteresting, they are swapped
>> +/* We saw two trees, ent0 and ent1.
>> + * if ent1 is uninteresting, they are swapped
>>   */
> 
> "While you are touching this comment" is a perfect time to fix the
> existing style violation.

Yes.  Will fix in next version.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/17] builtin_diff_tree(): make it obvious that function wants two entries

[Junio C Hamano]

Michael Haggerty  writes:

> Instead of accepting an array and using exactly two elements from the
> array, take two single (struct object_array_entry *) arguments.
>
> Signed-off-by: Michael Haggerty 
> ---
>  builtin/diff.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index 8c2af6c..ba68c6c 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -153,7 +153,8 @@ static int builtin_diff_index(struct rev_info *revs,
>  
>  static int builtin_diff_tree(struct rev_info *revs,
>int argc, const char **argv,
> -  struct object_array_entry *ent)
> +  struct object_array_entry *ent0,
> +  struct object_array_entry *ent1)
>  {
>   const unsigned char *(sha1[2]);
>   int swap = 0;
> @@ -161,13 +162,13 @@ static int builtin_diff_tree(struct rev_info *revs,
>   if (argc > 1)
>   usage(builtin_diff_usage);
>  
> - /* We saw two trees, ent[0] and ent[1].
> -  * if ent[1] is uninteresting, they are swapped
> + /* We saw two trees, ent0 and ent1.
> +  * if ent1 is uninteresting, they are swapped
>*/

"While you are touching this comment" is a perfect time to fix the
existing style violation.

Other than that, looks good to me.

> - if (ent[1].item->flags & UNINTERESTING)
> + if (ent1->item->flags & UNINTERESTING)
>   swap = 1;
> - sha1[swap] = ent[0].item->sha1;
> - sha1[1-swap] = ent[1].item->sha1;
> + sha1[swap] = ent0->item->sha1;
> + sha1[1-swap] = ent1->item->sha1;
>   diff_tree_sha1(sha1[0], sha1[1], "", &revs->diffopt);
>   log_tree_diff_flush(revs);
>   return 0;
> @@ -403,7 +404,7 @@ int cmd_diff(int argc, const char **argv, const char 
> *prefix)
>   else if (ents == 1)
>   result = builtin_diff_index(&rev, argc, argv);
>   else if (ents == 2)
> - result = builtin_diff_tree(&rev, argc, argv, ent);
> + result = builtin_diff_tree(&rev, argc, argv, &ent[0], &ent[1]);
>   else if (ent[0].item->flags & UNINTERESTING) {
>   /*
>* diff A...B where there is at least one merge base
> @@ -412,8 +413,7 @@ int cmd_diff(int argc, const char **argv, const char 
> *prefix)
>* between the base and B.  Note that we pick one
>* merge base at random if there are more than one.
>*/
> - ent[1] = ent[ents-1];
> - result = builtin_diff_tree(&rev, argc, argv, ent);
> + result = builtin_diff_tree(&rev, argc, argv, &ent[0], 
> &ent[ents-1]);
>   } else
>   result = builtin_diff_combined(&rev, argc, argv,
>  ent, ents);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/17] builtin_diff_tree(): make it obvious that function wants two entries

[Michael Haggerty]

Instead of accepting an array and using exactly two elements from the
array, take two single (struct object_array_entry *) arguments.

Signed-off-by: Michael Haggerty 
---
 builtin/diff.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 8c2af6c..ba68c6c 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -153,7 +153,8 @@ static int builtin_diff_index(struct rev_info *revs,
 
 static int builtin_diff_tree(struct rev_info *revs,
 int argc, const char **argv,
-struct object_array_entry *ent)
+struct object_array_entry *ent0,
+struct object_array_entry *ent1)
 {
const unsigned char *(sha1[2]);
int swap = 0;
@@ -161,13 +162,13 @@ static int builtin_diff_tree(struct rev_info *revs,
if (argc > 1)
usage(builtin_diff_usage);
 
-   /* We saw two trees, ent[0] and ent[1].
-* if ent[1] is uninteresting, they are swapped
+   /* We saw two trees, ent0 and ent1.
+* if ent1 is uninteresting, they are swapped
 */
-   if (ent[1].item->flags & UNINTERESTING)
+   if (ent1->item->flags & UNINTERESTING)
swap = 1;
-   sha1[swap] = ent[0].item->sha1;
-   sha1[1-swap] = ent[1].item->sha1;
+   sha1[swap] = ent0->item->sha1;
+   sha1[1-swap] = ent1->item->sha1;
diff_tree_sha1(sha1[0], sha1[1], "", &revs->diffopt);
log_tree_diff_flush(revs);
return 0;
@@ -403,7 +404,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
else if (ents == 1)
result = builtin_diff_index(&rev, argc, argv);
else if (ents == 2)
-   result = builtin_diff_tree(&rev, argc, argv, ent);
+   result = builtin_diff_tree(&rev, argc, argv, &ent[0], &ent[1]);
else if (ent[0].item->flags & UNINTERESTING) {
/*
 * diff A...B where there is at least one merge base
@@ -412,8 +413,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
 * between the base and B.  Note that we pick one
 * merge base at random if there are more than one.
 */
-   ent[1] = ent[ents-1];
-   result = builtin_diff_tree(&rev, argc, argv, ent);
+   result = builtin_diff_tree(&rev, argc, argv, &ent[0], 
&ent[ents-1]);
} else
result = builtin_diff_combined(&rev, argc, argv,
   ent, ents);
-- 
1.8.2.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html