git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
May 21, 2013, 17:27 UTC
Message-ID
<7v8v38p7jx.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1368995232-11042-5-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 31 quoted lines
> 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 <mhagger@alum.mit.edu>
> ---
>  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.
Show 29 quoted lines
> -	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);
Previous: Michael HaggertyNext: Michael Haggerty
Message 6 of 42 in “Remove assumptions about refname lifetimes”
  1. 00/17 Remove assumptions about refname lifetimesMichael Haggerty, May 19, 2013
  2. 01/17 describe: make own copy of refnameMichael Haggerty, May 19, 2013
  3. 02/17 fetch: make own copies of refnamesMichael Haggerty, May 19, 2013
  4. 03/17 add_rev_cmdline(): make a copy of the name argumentMichael Haggerty, May 19, 2013
  5. 04/17 builtin_diff_tree(): make it obvious that function wants two entriesMichael Haggerty, May 19, 2013
  6. Junio C HamanoMay 21, 2013
  7. Michael HaggertyMay 23, 2013
  8. 05/17 cmd_diff(): use an object_array for holding treesMichael Haggerty, May 19, 2013
  9. Junio C HamanoMay 21, 2013
  10. Michael HaggertyMay 23, 2013
  11. 06/17 cmd_diff(): rename local variable "list" -> "entry"Michael Haggerty, May 19, 2013
  12. 07/17 cmd_diff(): make it obvious which cases are exclusive of each otherMichael Haggerty, May 19, 2013
  13. 08/17 revision: split some overly-long linesMichael Haggerty, May 19, 2013
  14. Junio C HamanoMay 21, 2013
  15. Michael HaggertyMay 23, 2013
  16. Junio C HamanoMay 23, 2013
  17. 09/17 gc_boundary(): move the check "alloc <= nr" to callerMichael Haggerty, May 19, 2013
  18. Junio C HamanoMay 21, 2013
  19. Michael HaggertyMay 23, 2013
  20. Junio C HamanoMay 23, 2013
  21. 10/17 get_revision_internal(): make check less mysteriousMichael Haggerty, May 19, 2013
  22. Junio C HamanoMay 21, 2013
  23. Michael HaggertyMay 23, 2013
  24. 11/17 object_array: add function object_array_filter()Michael Haggerty, May 19, 2013
  25. 12/17 object_array_remove_duplicates(): rewrite to reduce copyingMichael Haggerty, May 19, 2013
  26. 13/17 fsck: don't put a void*-shaped peg in a char*-shaped holeMichael Haggerty, May 19, 2013
  27. 14/17 find_first_merges(): initialize merges variable using initializerMichael Haggerty, May 19, 2013
  28. 15/17 find_first_merges(): remove unnecessary codeMichael Haggerty, May 19, 2013
  29. 16/17 object_array_entry: copy name before storing in name fieldMichael Haggerty, May 19, 2013
  30. Johan HerlandMay 20, 2013
  31. Michael HaggertyMay 20, 2013
  32. Jeff KingMay 20, 2013
  33. Michael HaggertyMay 20, 2013
  34. 17/17 refs: document the lifetime of the refname passed to each_ref_fnMichael Haggerty, May 19, 2013
  35. Johan HerlandMay 20, 2013
  36. Michael HaggertyMay 20, 2013
  37. Junio C HamanoMay 20, 2013
  38. Jeff KingMay 20, 2013
  39. Johan HerlandMay 20, 2013
  40. Junio C HamanoMay 20, 2013
  41. Johan HerlandMay 20, 2013
  42. Junio C HamanoMay 21, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.