On Sat, Apr 17, 2021 at 8:58 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxx/
