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

Re: [PATCH v2] ls-tree: fix --long implying -r regression in 9c4d58ff2c3

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Apr 7, 2022, 07:14 UTC
Message-ID
<220407.86y20hcpy5.gmgdl@evledraar.gmail.com>
In-Reply-To
<xmqq4k35x4m6.fsf@gitster.g>
On Wed, Apr 06 2022, Junio C Hamano wrote:
Show 20 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>>> +	cat >expect &&
>>>> +	cat <&6 >expect.-d &&
>>>> +	cat <&7 >expect.-r &&
>>>> +	cat <&8 >expect.-t &&
>>>
>>> Let's not go too cute like this.  This forces the caller to remember
>>> which among 6, 7, and 8 corresponds to which option.  It is too ugly
>>> to live.
>>
>> I think it's rather elegant actually, but to be fair it would, per:
>>
>> 	git grep '<&[1-9]| [1-9]<<-'
>>
>> Be the user with the most FD's using this sort of pattern.
>
> Please give a clear explanation why "-d" has to be 6, "-r" 7 and
> "-t" 8, that can be used by developers as a memory aid to help them
> write new tests using the helper.

It's documented when test-lib.sh does the redirection, since Fabian Stelzer's a6714088e0c (test-lib: make BAIL_OUT() work in tests and prereq, 2021-12-01).

I guess a bit of archane knowledge not documented there is that FD #5 is used for the test-lib.sh itself, e.g. BUG(), so if you want that to work properly you can't touch it.

But everything as of #6 is generally fair game, secondary helpers like "test_pause" won't work properly, but as far as the test framork is concerned 6-9 is good.

So I don't really have a better explanation the same on as why pick "int i" rather than "int x", it's just the prevailing pattern.

> Or justify why the developers have to memorize such a meaningless
> correspondence, if there is no any good reason.
Explained above, I think.
> Alternatively, you can stop abusing the word "elegant".  It is not a
> synonym to "what I wrote" ;-).

The "elegant" part is getting away with passing structured data into a shell function, which it's generally resistant to.

As much as I'd like to take credit from it I just picked it up from code Ilya wrote in 0445e6f0a12 (test-lib: '--run' to run only specific tests, 2014-04-30). I didn't know about it before then.

In any case, you merged down Josh's version without these tests for rc1, so I'm assuming that's a "no" to the upthread "if you'd like to pick this up with the version with the tests at all" in the context of the RC period.

So I think we can table this for now, or would you like a version of this without (a)use of these file descriptors to pass in arguments?

Previous: Junio C HamanoNext: Junio C Hamano
Message 89 of 107 in “ls-tree: "--object-only" and "--format" opts”
  1. 00/13 ls-tree: "--object-only" and "--format" optsTeng Long, Feb 8, 2022
  2. 01/13 ls-tree: remove commented-out codeTeng Long, Feb 8, 2022
  3. 03/13 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Feb 8, 2022
  4. 12/13 ls-tree: introduce function "fast_path()"Teng Long, Feb 8, 2022
  5. Ævar Arnfjörð BjarmasonFeb 19, 2022
  6. 13/13 ls-tree.c: support --object-only option for "git-ls-tree"Teng Long, Feb 8, 2022
  7. Ævar Arnfjörð BjarmasonFeb 19, 2022
  8. 04/13 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Feb 8, 2022
  9. 02/13 ls-tree: add missing braces to "else" armsTeng Long, Feb 8, 2022
  10. 06/13 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Feb 8, 2022
  11. Ævar Arnfjörð BjarmasonFeb 19, 2022
  12. 05/13 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Feb 8, 2022
  13. 07/13 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Feb 8, 2022
  14. Ævar Arnfjörð BjarmasonFeb 19, 2022
  15. 08/13 ls-tree: slightly refactor `show_tree()`Teng Long, Feb 8, 2022
  16. Ævar Arnfjörð BjarmasonFeb 19, 2022
  17. 10/13 cocci: allow padding with `strbuf_addf()`Teng Long, Feb 8, 2022
  18. 09/13 ls-tree: introduce struct "show_tree_data"Teng Long, Feb 8, 2022
  19. 11/13 ls-tree.c: introduce "--format" optionTeng Long, Feb 8, 2022
  20. Ævar Arnfjörð BjarmasonFeb 19, 2022
  21. 00/12 ls-tree: "--object-only" and "--format" optsTeng Long, Mar 4, 2022
  22. 01/12 ls-tree: remove commented-out codeTeng Long, Mar 4, 2022
  23. 02/12 ls-tree: add missing braces to "else" armsTeng Long, Mar 4, 2022
  24. 03/12 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Mar 4, 2022
  25. 04/12 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Mar 4, 2022
  26. 05/12 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Mar 4, 2022
  27. 06/12 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Mar 4, 2022
  28. 07/12 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Mar 4, 2022
  29. 08/12 ls-tree: slightly refactor `show_tree()`Teng Long, Mar 4, 2022
  30. 09/12 ls-tree: introduce struct "show_tree_data"Teng Long, Mar 4, 2022
  31. 10/12 cocci: allow padding with `strbuf_addf()`Teng Long, Mar 4, 2022
  32. 11/12 ls-tree: introduce "--format" optionTeng Long, Mar 4, 2022
  33. 12/12 ls-tree: support --object-only option for "git-ls-tree"Teng Long, Mar 4, 2022
  34. 0/7 fixups/suggestions/musings for tl/ls-tree-oid-onlyÆvar Arnfjörð Bjarmason, Mar 10, 2022
  35. 1/7 ls-tree tests: add tests for --name-statusÆvar Arnfjörð Bjarmason, Mar 10, 2022
  36. 2/7 ls-tree tests: exhaustively test fast & slow path for --formatÆvar Arnfjörð Bjarmason, Mar 10, 2022
  37. 5/7 ls-tree: detect and error on --name-only --name-statusÆvar Arnfjörð Bjarmason, Mar 10, 2022
  38. 4/7 ls-tree: remove unused "MODE_UNSPECIFIED"Ævar Arnfjörð Bjarmason, Mar 10, 2022
  39. 3/7 ls-tree: remove dead labelsÆvar Arnfjörð Bjarmason, Mar 10, 2022
  40. 6/7 ls-tree: remove FIELD_*, just use MODE_*Ævar Arnfjörð Bjarmason, Mar 10, 2022
  41. 7/7 ls-tree: split up "fast path" callbacksÆvar Arnfjörð Bjarmason, Mar 10, 2022
  42. Teng LongMar 17, 2022
  43. Ævar Arnfjörð BjarmasonMar 17, 2022
  44. 00/16 ls-tree: "--object-only" and "--format" optsTeng Long, Mar 21, 2022
  45. 01/16 ls-tree: remove commented-out codeTeng Long, Mar 21, 2022
  46. 02/16 ls-tree: add missing braces to "else" armsTeng Long, Mar 21, 2022
  47. 03/16 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Mar 21, 2022
  48. 04/16 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Mar 21, 2022
  49. 05/16 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Mar 21, 2022
  50. 06/16 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Mar 21, 2022
  51. 07/16 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Mar 21, 2022
  52. 08/16 ls-tree: slightly refactor `show_tree()`Teng Long, Mar 21, 2022
  53. 09/16 ls-tree: introduce struct "show_tree_data"Teng Long, Mar 21, 2022
  54. 10/16 cocci: allow padding with `strbuf_addf()`Teng Long, Mar 21, 2022
  55. 11/16 ls-tree: introduce "--format" optionTeng Long, Mar 21, 2022
  56. Ævar Arnfjörð BjarmasonMar 21, 2022
  57. 12/16 ls-tree: support --object-only option for "git-ls-tree"Teng Long, Mar 21, 2022
  58. 13/16 ls-tree tests: add tests for --name-statusTeng Long, Mar 21, 2022
  59. Ævar Arnfjörð BjarmasonMar 21, 2022
  60. 14/16 ls-tree: detect and error on --name-only --name-statusTeng Long, Mar 21, 2022
  61. 15/16 ls-tree: remove FIELD_*, just use MODE_*Teng Long, Mar 21, 2022
  62. Ævar Arnfjörð BjarmasonMar 21, 2022
  63. 16/16 ls-tree: split up "fast path" callbacksTeng Long, Mar 21, 2022
  64. Ævar Arnfjörð BjarmasonMar 21, 2022
  65. Teng LongMar 23, 2022
  66. Junio C HamanoMar 21, 2022
  67. 00/15 ls-tree: "--object-only" and "--format" optsTeng Long, Mar 23, 2022
  68. 01/15 ls-tree tests: add tests for --name-statusTeng Long, Mar 23, 2022
  69. 02/15 ls-tree: remove commented-out codeTeng Long, Mar 23, 2022
  70. 03/15 ls-tree: add missing braces to "else" armsTeng Long, Mar 23, 2022
  71. 04/15 ls-tree: use "enum object_type", not {blob,tree,commit}_typeTeng Long, Mar 23, 2022
  72. 05/15 ls-tree: use "size_t", not "int" for "struct strbuf"'s "len"Teng Long, Mar 23, 2022
  73. 06/15 ls-tree: rename "retval" to "recurse" in "show_tree()"Teng Long, Mar 23, 2022
  74. 07/15 ls-tree: simplify nesting if/else logic in "show_tree()"Teng Long, Mar 23, 2022
  75. 08/15 ls-tree: fix "--name-only" and "--long" combined use bugTeng Long, Mar 23, 2022
  76. 09/15 ls-tree: slightly refactor `show_tree()`Teng Long, Mar 23, 2022
  77. 10/15 ls-tree: introduce struct "show_tree_data"Teng Long, Mar 23, 2022
  78. 11/15 cocci: allow padding with `strbuf_addf()`Teng Long, Mar 23, 2022
  79. 12/15 ls-tree: introduce "--format" optionTeng Long, Mar 23, 2022
  80. 13/15 ls-tree: support --object-only option for "git-ls-tree"Teng Long, Mar 23, 2022
  81. 14/15 ls-tree: detect and error on --name-only --name-statusTeng Long, Mar 23, 2022
  82. 15/15 ls-tree: split up "fast path" callbacksTeng Long, Mar 23, 2022
  83. Josh SteadmonApr 4, 2022
  84. ls-tree: `-l` should not imply recursive listingJosh Steadmon, Apr 4, 2022
  85. ls-tree: fix --long implying -r regression in 9c4d58ff2c3Ævar Arnfjörð Bjarmason, Apr 4, 2022
  86. Junio C HamanoApr 6, 2022
  87. Ævar Arnfjörð BjarmasonApr 6, 2022
  88. Junio C HamanoApr 6, 2022
  89. Ævar Arnfjörð BjarmasonApr 7, 2022
  90. Junio C HamanoApr 7, 2022
  91. ls-tree: test for the regression in 9c4d58ff2c3Ævar Arnfjörð Bjarmason, May 31, 2022
  92. Johannes SchindelinJun 2, 2022
  93. Junio C HamanoJun 2, 2022
  94. js/ci-github-workflow-markup output regression (was: [PATCH] ls-tree: test for the regression in 9c4d58ff2c3)Ævar Arnfjörð Bjarmason, Jun 3, 2022
  95. Junio C HamanoJun 3, 2022
  96. Ævar Arnfjörð BjarmasonJun 3, 2022
  97. Junio C HamanoJun 7, 2022
  98. Ævar Arnfjörð BjarmasonJun 7, 2022
  99. Johannes SchindelinJun 8, 2022
  100. Ævar Arnfjörð BjarmasonJun 9, 2022
  101. ls-tree: test for the regression in 9c4d58ff2c3Ævar Arnfjörð Bjarmason, Jun 3, 2022
  102. Johannes SchindelinJun 8, 2022
  103. Teng LongApr 7, 2022
  104. Junio C HamanoApr 6, 2022
  105. Junio C HamanoMar 23, 2022
  106. Teng LongMar 24, 2022
  107. Ævar Arnfjörð BjarmasonFeb 28, 2022

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.