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

Re: [PATCH v4 5/6] get_short_oid: sort ambiguous objects by type, then SHA-1

From
Jeff King <peff@peff.net>
Date
May 10, 2018, 15:22 UTC
Message-ID
<20180510152221.GB6462@sigill.intra.peff.net>
In-Reply-To
<20180510124303.6020-6-avarab@gmail.com>
On Thu, May 10, 2018 at 12:43:02PM +0000, Ævar Arnfjörð Bjarmason wrote:
Show 14 quoted lines
> Now we'll instead show:
> 
>     hint:   e8f2650052 tag v2.17.0
>     hint:   e8f21caf94 commit 2013-06-24 - bash prompt: print unique detached HEAD abbreviated object name
>     hint:   e8f26250fa commit 2017-02-03 - Merge pull request #996 from jeffhostetler/jeffhostetler/register_rename_src
>     hint:   e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for multiple remote.url entries
>     hint:   e8f2093055 tree
>     hint:   e8f25a3a50 tree
>     hint:   e8f28d537c tree
>     hint:   e8f2cf6ec0 tree
>     hint:   e8f21d02f7 blob
>     hint:   e8f21d577c blob
>     hint:   e8f2867228 blob
>     hint:   e8f2a35526 blob

I said already that I like the output, but this time I'll actually read the code. ;)

It all looks good to me, with the exception of a few documentation nits I'll mention below.

Show 11 quoted lines
> A note on the implementation: Derrick rightly pointed out[1] that
> we're bending over backwards here in get_short_oid() to first
> de-duplicate the list, and then emit it, but could simply do it in one
> step.
> 
> The reason for that is that oid_array_for_each_unique() doesn't
> actually require that the array be sorted by oid_array_sort(), it just
> needs to be sorted in some order that guarantees that all objects with
> the same ID are adjacent to one another, which (barring a hash
> collision, which'll be someone else's problem) the sort_ambiguous()
> function does.

If we were to go this route, I think it would make sense to add a sorting function pointer to "struct oid_array". I'm OK with punting on it for now, though.

Show 26 quoted lines
> diff --git a/Documentation/technical/api-oid-array.txt b/Documentation/technical/api-oid-array.txt
> index b0c11f868d..94b529722c 100644
> --- a/Documentation/technical/api-oid-array.txt
> +++ b/Documentation/technical/api-oid-array.txt
> @@ -35,13 +35,18 @@ Functions
>  	Free all memory associated with the array and return it to the
>  	initial, empty state.
>  
> +`oid_array_for_each`::
> +	Iterate over each element of the list, executing the callback
> +	function for each one. Does not sort the list, so any custom
> +	hash order is retained. If the callback returns a non-zero
> +	value, the iteration ends immediately and the callback's
> +	return is propagated; otherwise, 0 is returned.
> +
>  `oid_array_for_each_unique`::
> -	Efficiently iterate over each unique element of the list,
> -	executing the callback function for each one. If the array is
> -	not sorted, this function has the side effect of sorting it. If
> -	the callback returns a non-zero value, the iteration ends
> -	immediately and the callback's return is propagated; otherwise,
> -	0 is returned.
> +	Iterate over each unique element of the list in sort order ,
> +	but otherwise behaves like `oid_array_for_each`. If the array
> +	is not sorted, this function has the side effect of sorting
> +	it.
Extra space in "sort order ,".

I'd probably say "sorted order", but that might be a matter of preference.

Also, your parallel verb tenses don't agree. ;) It should be "Iterate ... but otherwise behave", not "behaves".

Show 12 quoted lines
> +	/*
> +	 * Between object types show tags, then commits, and finally
> +	 * trees and blobs.
> +	 *
> +	 * The object_type enum is commit, tree, blob, tag, but we
> +	 * want tag, commit, tree blob. Cleverly (perhaps too
> +	 * cleverly) do that with modulus, since the enum assigns 1 to
> +	 * commit, so tag becomes 0.
> +	 */
> +	a_type_sort = a_type % 4;
> +	b_type_sort = b_type % 4;
> +	return a_type_sort > b_type_sort ? 1 : -1;

This is amusingly clever, and should be very efficient. I'm glad there's a comment at least, though.

-Peff
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 81 of 99 in “get_short_oid UI improvements”
  1. 0/9 get_short_oid UI improvementsÆvar Arnfjörð Bjarmason, Apr 30, 2018
  2. 1/9 sha1-name.c: remove stray newlineÆvar Arnfjörð Bjarmason, Apr 30, 2018
  3. 2/9 sha1-array.h: align function argumentsÆvar Arnfjörð Bjarmason, Apr 30, 2018
  4. 3/9 sha1-name.c: move around the collect_ambiguous() functionÆvar Arnfjörð Bjarmason, Apr 30, 2018
  5. 4/9 get_short_oid: sort ambiguous objects by type, then SHA-1Ævar Arnfjörð Bjarmason, Apr 30, 2018
  6. Derrick StoleeMay 1, 2018
  7. Ævar Arnfjörð BjarmasonMay 1, 2018
  8. Derrick StoleeMay 1, 2018
  9. Ævar Arnfjörð BjarmasonMay 1, 2018
  10. Derrick StoleeMay 1, 2018
  11. 6/9 get_short_oid: learn to disambiguate by ^{blob}Ævar Arnfjörð Bjarmason, Apr 30, 2018
  12. 7/9 get_short_oid / peel_onion: ^{tree} should mean tree, not treeishÆvar Arnfjörð Bjarmason, Apr 30, 2018
  13. brian m. carlsonMay 1, 2018
  14. 5/9 get_short_oid: learn to disambiguate by ^{tag}Ævar Arnfjörð Bjarmason, Apr 30, 2018
  15. 9/9 config doc: document core.disambiguateÆvar Arnfjörð Bjarmason, Apr 30, 2018
  16. 8/9 get_short_oid / peel_onion: ^{tree} should mean commit, not commitishÆvar Arnfjörð Bjarmason, Apr 30, 2018
  17. Eric SunshineApr 30, 2018
  18. Stefan BellerApr 30, 2018
  19. brian m. carlsonMay 1, 2018
  20. Derrick StoleeMay 1, 2018
  21. 00/12 get_short_oid UI improvementsÆvar Arnfjörð Bjarmason, May 1, 2018
  22. 01/12 sha1-name.c: remove stray newlineÆvar Arnfjörð Bjarmason, May 1, 2018
  23. 02/12 sha1-array.h: align function argumentsÆvar Arnfjörð Bjarmason, May 1, 2018
  24. 03/12 git-p4: change "commitish" typo to "committish"Ævar Arnfjörð Bjarmason, May 1, 2018
  25. 04/12 cache.h: add comment explaining the order in object_typeÆvar Arnfjörð Bjarmason, May 1, 2018
  26. 05/12 sha1-name.c: move around the collect_ambiguous() functionÆvar Arnfjörð Bjarmason, May 1, 2018
  27. 07/12 get_short_oid: learn to disambiguate by ^{tag}Ævar Arnfjörð Bjarmason, May 1, 2018
  28. 06/12 get_short_oid: sort ambiguous objects by type, then SHA-1Ævar Arnfjörð Bjarmason, May 1, 2018
  29. 09/12 get_short_oid / peel_onion: ^{tree} should be tree, not treeishÆvar Arnfjörð Bjarmason, May 1, 2018
  30. 10/12 get_short_oid / peel_onion: ^{commit} should be commit, not committishÆvar Arnfjörð Bjarmason, May 1, 2018
  31. 12/12 get_short_oid: document & warn if we ignore the type selectorÆvar Arnfjörð Bjarmason, May 1, 2018
  32. 11/12 config doc: document core.disambiguateÆvar Arnfjörð Bjarmason, May 1, 2018
  33. 08/12 get_short_oid: learn to disambiguate by ^{blob}Ævar Arnfjörð Bjarmason, May 1, 2018
  34. 06/11 get_short_oid: sort ambiguous objects by type, then SHA-1Derrick Stolee, May 1, 2018
  35. Ævar Arnfjörð BjarmasonMay 1, 2018
  36. Derrick StoleeMay 1, 2018
  37. Ævar Arnfjörð BjarmasonMay 1, 2018
  38. Derrick StoleeMay 1, 2018
  39. 00/12 get_short_oid UI improvementsÆvar Arnfjörð Bjarmason, May 1, 2018
  40. 01/12 sha1-name.c: remove stray newlineÆvar Arnfjörð Bjarmason, May 1, 2018
  41. 02/12 sha1-array.h: align function argumentsÆvar Arnfjörð Bjarmason, May 1, 2018
  42. 04/12 cache.h: add comment explaining the order in object_typeÆvar Arnfjörð Bjarmason, May 1, 2018
  43. Junio C HamanoMay 3, 2018
  44. Duy NguyenMay 8, 2018
  45. pack-format.txt: more details on pack file formatNguyễn Thái Ngọc Duy, May 8, 2018
  46. Stefan BellerMay 8, 2018
  47. Duy NguyenMay 8, 2018
  48. Stefan BellerMay 8, 2018
  49. Ævar Arnfjörð BjarmasonMay 8, 2018
  50. Duy NguyenMay 8, 2018
  51. pack-format.txt: more details on pack file formatNguyễn Thái Ngọc Duy, May 10, 2018
  52. Stefan BellerMay 10, 2018
  53. Duy NguyenMay 11, 2018
  54. Junio C HamanoMay 11, 2018
  55. pack-format.txt: more details on pack file formatNguyễn Thái Ngọc Duy, May 11, 2018
  56. 03/12 git-p4: change "commitish" typo to "committish"Ævar Arnfjörð Bjarmason, May 1, 2018
  57. 05/12 sha1-name.c: move around the collect_ambiguous() functionÆvar Arnfjörð Bjarmason, May 1, 2018
  58. 08/12 get_short_oid: learn to disambiguate by ^{blob}Ævar Arnfjörð Bjarmason, May 1, 2018
  59. 09/12 get_short_oid / peel_onion: ^{tree} should be tree, not treeishÆvar Arnfjörð Bjarmason, May 1, 2018
  60. Junio C HamanoMay 3, 2018
  61. Ævar Arnfjörð BjarmasonMay 3, 2018
  62. Junio C HamanoMay 4, 2018
  63. Ævar Arnfjörð BjarmasonMay 4, 2018
  64. Junio C HamanoMay 7, 2018
  65. Jeff KingMay 8, 2018
  66. Ævar Arnfjörð BjarmasonMay 8, 2018
  67. Jeff KingMay 9, 2018
  68. Ævar Arnfjörð BjarmasonMay 9, 2018
  69. Junio C HamanoMay 10, 2018
  70. Jeff KingMay 10, 2018
  71. 0/6 get_short_oid UI improvementsÆvar Arnfjörð Bjarmason, May 10, 2018
  72. 1/6 sha1-name.c: remove stray newlineÆvar Arnfjörð Bjarmason, May 10, 2018
  73. 2/6 sha1-array.h: align function argumentsÆvar Arnfjörð Bjarmason, May 10, 2018
  74. Jeff KingMay 10, 2018
  75. Junio C HamanoMay 11, 2018
  76. Junio C HamanoMay 11, 2018
  77. 3/6 git-p4: change "commitish" typo to "committish"Ævar Arnfjörð Bjarmason, May 10, 2018
  78. Luke DiamandMay 10, 2018
  79. 4/6 sha1-name.c: move around the collect_ambiguous() functionÆvar Arnfjörð Bjarmason, May 10, 2018
  80. 5/6 get_short_oid: sort ambiguous objects by type, then SHA-1Ævar Arnfjörð Bjarmason, May 10, 2018
  81. Jeff KingMay 10, 2018
  82. Junio C HamanoMay 11, 2018
  83. 6/6 get_short_oid: document & warn if we ignore the type selectorÆvar Arnfjörð Bjarmason, May 10, 2018
  84. Martin ÅgrenMay 10, 2018
  85. Jeff KingMay 10, 2018
  86. Jeff KingMay 10, 2018
  87. Jeff KingMay 10, 2018
  88. Jeff KingMay 10, 2018
  89. 10/12 get_short_oid / peel_onion: ^{commit} should be commit, not committishÆvar Arnfjörð Bjarmason, May 1, 2018
  90. 11/12 config doc: document core.disambiguateÆvar Arnfjörð Bjarmason, May 1, 2018
  91. Jeff KingMay 8, 2018
  92. 12/12 get_short_oid: document & warn if we ignore the type selectorÆvar Arnfjörð Bjarmason, May 1, 2018
  93. 06/12 get_short_oid: sort ambiguous objects by type, then SHA-1Ævar Arnfjörð Bjarmason, May 1, 2018
  94. Junio C HamanoMay 3, 2018
  95. Jeff KingMay 8, 2018
  96. 07/12 get_short_oid: learn to disambiguate by ^{tag}Ævar Arnfjörð Bjarmason, May 1, 2018
  97. Derrick StoleeMay 2, 2018
  98. Derrick StoleeMay 2, 2018
  99. Jacob KellerMay 3, 2018

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.