Re: [PATCH 1/5] t3416: set $EDITOR in subshell

From: Jonathan Tan <jonathantanmy@google.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: Jonathan Tan <jonathantanmy@google.com>,
	Junio C Hamano <gitster@pobox.com>,
	Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Philippe Blain <levraiphilippeblain@gmail.com>,
	Denton Liu <liu.denton@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH 1/5] t3416: set $EDITOR in subshell
Date: Wed, 24 Aug 2022 15:28:55 -0700	[thread overview]
Message-ID: <20220824222855.1686175-1-jonathantanmy@google.com> (raw)
In-Reply-To: <eab55cef-62bd-af2b-c295-1e76beca032f@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:
> Hi Junio
> 
> On 15/08/2022 17:53, Junio C Hamano wrote:
> > "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> > 
> >> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> >>
> >> As $EDITOR is exported setting it in one test affects all subsequent
> >> tests. Avoid this by always setting it in a subshell and remove a
> >> couple of unnecessary call to set_fake_editor.
> > 
> > Unnecessary because it reuses the one that was established in the
> > previous test [1]?  Or unnecessary because we know "rebase -i" would
> > fail even before it gets to the point of asking an editor to tweak
> > the todo sequence [2]?  Or something else?
> 
> I meant unnecessary as the editor does not change the todo list, but [2] 
> also applies.

Maybe this is moot with the other changes you're planning, but even if
the editor doesn't change the todo list, it's still necessary, right? At
the very least, we need to suppress the default interactive editor and
replace it with one that just reuses the input file without any
modification.