Re: [PATCH 1/4] stash: pass --no-color to diff-tree child processes

From: Jeff King <peff@peff.net>
Date: Sep 8, 2025, 16:06 UTC
Message-ID: <20250908160628.GB1308482@coredump.intra.peff.net>
In-Reply-To: <aLfs5EDk-krJHnmQ@pks.im>

On Wed, Sep 03, 2025 at 09:23:16AM +0200, Patrick Steinhardt wrote:

Show 15 quoted lines

> > @@ -1345,6 +1346,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps,
> >  
> >  	cp_diff_tree.git_cmd = 1;
> >  	strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "-U1", "HEAD",
> > +		     "--no-color",
> >  		     oid_to_hex(&info->w_tree), "--", NULL);
> >  	if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) {
> >  		ret = -1;
> 
> All of these make sense. It feels a bit like whack-a-mole, and fixing
> the root cause would address that. But I also understand that you shy
> away from addressing it due to the high chance for regressions.
> 
> There's also a call to "diff-index" in the same file. Do we also need to
> adjust that instance?

I was focused on the "-p" code path, but yes, I think it fails when we stash an index change with GIT_PAGER_IN_USE=1. I've fixed it and added a new test in my re-roll.

-Peff