Re: [PATCH 2/2] rebase -i: silence stash apply

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 18, 2017, 10:49 UTC
Message-ID: <alpine.DEB.2.21.1.1705181246430.3610@virtualbox>
In-Reply-To: <20170518100233.30559-3-phillip.wood@talktalk.net>

Hi Phillip,

On Thu, 18 May 2017, Phillip Wood wrote:

Show 13 quoted lines

> diff --git a/sequencer.c b/sequencer.c
> index f8bc18badf1a3fb1b39656501c5a316e229968d2..311728a145dfc66e230334221a2610468239932d 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1914,6 +1914,8 @@ static int apply_autostash(struct replay_opts *opts)
>  	strbuf_trim(&stash_sha1);
>  
>  	child.git_cmd = 1;
> +	child.no_stdout = 1;
> +	child.no_stderr = 1;
>  	argv_array_push(&child.args, "stash");
>  	argv_array_push(&child.args, "apply");
>  	argv_array_push(&child.args, stash_sha1.buf);

If I remember correctly, then the shell version prints the output in case of an error.

We already imitated that behavior in `git commit` (https://github.com/git-for-windows/git/blob/v2.13.0.windows.1/sequencer.c#L674-L684):

		/* hide stderr on success */
		struct strbuf buf = STRBUF_INIT;
		int rc = pipe_command(&cmd,
				      NULL, 0,
				      /* stdout is already redirected */
				      NULL, 0,
				      &buf, 0);
		if (rc)
			fputs(buf.buf, stderr);
		strbuf_release(&buf);

I think that would be the appropriate approach here, too.

Ciao, Johannes

P.S.: it may be a very good idea to accompany this patch (as well as the previous one) by a patch to the test suite to verify that the fixed code does not regress.