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

[Denton Liu <liu.denton@gmail.com>]

On Wed, Oct 23, 2019 at 09:32:26AM -0400, Eric Sunshine wrote:
> On Wed, Oct 23, 2019 at 8:04 AM Denton Liu <liu.denton@gmail.com> wrote:
> > 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".

Since it doesn't really make sense to commmit conflicts, I decided to
hardcode it to be a diff against HEAD and the worktree since that's the
only sensible place where the conflict should live.

Speaking of conflicts, I dropped the "conflicted" part of the old
function name. I think that removes a lot of clarity so I'll reroll
renaming the function to print_sanitized_conflicted_diff() or something
like that.