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@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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.
