From: Jeff King <peff@peff.net>
Date: Mon, 08 Sep 2025 16:06:28 GMT
Subject: Re: [PATCH 1/4] stash: pass --no-color to diff-tree child processes
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:

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