Re: [PATCH 2/5] t4108: remove git command upstream of pipe

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 23, 2019, 13:32 UTC
Message-ID: <CAPig+cQ6RCMOaf4ZtOKNtiDdKu1JED=zMKZ8afwSr0cvE2zMgw@mail.gmail.com>
In-Reply-To: <9d915748c1953cc2683fa3189c3c98b1e9a1e299.1571832176.git.liu.denton@gmail.com>

On Wed, Oct 23, 2019 at 8:04 AM Denton Liu <liu.denton@gmail.com> wrote:

Show 23 quoted lines

> Before, the output of `git diff HEAD` would always be piped to
> sanitize_conflicted_diff(). However, since the Git command was upstream
> of the pipe, in case the Git command fails, the return code would be
> lost. Rewrite into separate statements so that the return code is no
> longer lost.
>
> Since only the command `git diff HEAD` was being piped to
> sanitize_conflicted_diff(), move the command into the function and rename
> it to print_sanitized_diff().
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh
> @@ -4,11 +4,12 @@ test_description='git apply --3way'
> -sanitize_conflicted_diff () {
> +print_sanitized_diff () {
> +       git diff HEAD >diff.raw &&
>         sed -e '
>                 /^index /d
>                 s/^\(+[<>][<>][<>][<>]*\) .*/\1/
> -       '
> +       ' diff.raw
>  }

Nit: By hard-coding "HEAD" in this function, you lose the flexibility
of the original. An alternative would have been to accept the ref
against which to diff as an argument to this function:

    print_sanitized_diff () {
        git diff "$@" >diff.raw &&
        ...

Or, better yet, keep the original design and pass the diff in as the shell function's input, so a caller would say:

    git diff HEAD >diff.raw &&
    sanitize_conflicted_diff <diff.raw >expect.diff &&

However, not necessarily worth a re-roll if we never expect anyone to pass anything other than "HEAD".