Re: [PATCH v3 1/2] stash push: avoid printing errors

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 16, 2018, 21:31 UTC
Message-ID: <xmqqtvtfzp0g.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180316204306.862-2-t.gummerer@gmail.com>

Thomas Gummerer <t.gummerer@gmail.com> writes:

Show 6 quoted lines

> @@ -322,9 +322,12 @@ push_stash () {
>  
>  		if test $# != 0
>  		then
> -			git add -u -- "$@" |
> -			git checkout-index -z --force --stdin

This obviously is not something this patch breaks, but I am finding this pipeline that was already here quite puzzling.

The "add -u" is about adding the changes in paths that match the pathspec to the index; the output from it is meant for human consumption and certainly is not something "--stdin" should expect to be understandable, let alone with "-z".

	... goes and digs ...

I think you mis-copied the suggestion in

    https://public-inbox.org/git/xmqqpo7byjwb.fsf@gitster.mtv.corp.google.com/

when you made bba067d2 ("stash: don't delete untracked files that match pathspec", 2018-01-06), and nobody caught that breakage during the review.

Show 5 quoted lines

> -			git diff-index -p --cached --binary HEAD -- "$@" | git apply --index -R
> +			if git ls-files --error-unmatch -- "$@" >/dev/null 2>/dev/null
> +			then
> +				git add -u -- "$@" |
> +				git checkout-index -z --force --stdin

And the same breakage is inherited here; just drop "|" and downstream "checkout-index" and you should be OK.

> +				git diff-index -p --cached --binary HEAD -- "$@" | git apply --index -R

And while at it, let's split this to two lines after "|".

> +			fi