Re: [PATCH 2/3] Revert and amend "test-lib-functions: assert correct parameter count"

On Sat, Apr 17, 2021 at 8:58 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

[...]
The goal here is to get rid of the verbosity of having e.g. a "test 2
-ne 2" line for every "test_cmp". We use "$@" as an argument to "test"
to intentionally feed the "test" operator too many arguments if the
functions are called with too many arguments, thus piggy-backing on it
to check the number of arguments we get.

My one concern about this change is that it enters the realm of
"huh?". Although it's dead-simple to understand what this code is
doing:

    test "$#" -ne 1 && BUG "1 param"
    if ! test -f "$1"

the replacement code:

    if ! test -f "$@"

can easily lead to head-scratching, wondering why the author chose to
use "$@" rather than the more obvious "$1". This sort of unusual local
idiom might normally deserve an in-code comment explaining why the
more obvious code is not employed. However, adding a comment at each
site would be overkill, so it might be the sort of thing to explain in
documentation somewhere ("In order to make -x output less noisy,
employ "$@" rather than explicitly checking function arguments when
writing new test functions...".) Otherwise, the reader is forced to
consult the commit message.

Not a major objection; just voicing the bit of unease I feel about it.

This does not to the "right" thing in cases like:

Channeling Dscho's inner Eric Sunshine[1]: s/to/do/

[1]: https://lore.kernel.org/git/nycvar.QRO.7.76.6.2103222235150.50@tvgsbejvaqbjf.bet/ (local)