Re: [PATCH v4] branch: support for shortcuts like @{-1}, completed

[Eric Sunshine <sunshine@sunshineco.com>]

On Sat, Oct 8, 2022 at 7:28 PM Rubén Justo <rjusto@gmail.com> wrote:
> On 8/10/22 19:10, Eric Sunshine wrote:
> > However, `test_cmp` gives you that behavior for free, and it emits a
> > helpful "diff" upon failure, so these days we usually go with
> > `test_cmp`.
>
> I understand the possibility of losing exit codes or segfaults in
> subcommands or pipes, but I'm more thinking in the element to compare to.
> Having something like:
>
>         test_cmp_str () {
>                 test -f "$1" || BUG "first argument must be a file"
>                 if test "$#" -gt 1
>                 then
>                         local F=$1
>                         shift
>                         printf "$@" | eval "$GIT_TEST_CMP" '"$F"' -
>                 else
>                         eval "$GIT_TEST_CMP" '"$1"' -
>                 fi
>         }
>
> to allow writing simpler tests like:
>
> +       test_cmp_str actual "Branch description\n\n"
>
> +       test_cmp_str actual <<-\EOF
> +       Branch description
> +
> +       EOF
>
> My doubts are that maybe this can induce to some bad usages, is
> unusable in some systems, it has already been explored and discarded,
> using files gives the diff with nice names not "-",...

I wouldn't be at all surprised if this sort of thing has been
discussed already (it sounds familiar enough), or that such an
improvement has already been submitted in the form of a patch
(possibly by Ævar, Cc:'d).

My knee-jerk reaction is that the form which takes a string as
argument would hardly be used, thus an unnecessary complication. The
form which accepts stdin could even be retrofitted onto the existing
test_cmp, thus you don't even need to invent a new function name. A
different approach would be to introduce a function `test_stdout`
which both accepts a command to run, as well as the "expected" text on
stdin with which to compare the output of the command; i.e. a
combination of the existing `test_stdout_line_count` in
t/test-lib-functions.sh and your proposed function above.

That said, I'm not yet seeing all that much added value in such a
function; at most, it seems to save only a single line of code, and
it's not as if the code it's replacing was doing anything complicated
or hard to grok. So, I'm pretty ambivalent about it. But, of course,
I'm only one person expressing a knee-jerk reaction. Submitting the
idea as an RFC patch might generate more feedback.