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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 07 Apr 2022 09:14:40 +0200

On Wed, Apr 06 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <[email-masked]> 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?