From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 13 Feb 2026 17:02:07 GMT
Subject: Re: [PATCH 1/4] builtin/history: perform revwalk checks before asking for user input
Message-ID: <xmqq1pioo9v4.fsf@gitster.g>
In-Reply-To: <aY67ycgnisRjeguw@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Feb 12, 2026 at 12:04:50PM -0800, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> > diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
>> > index 3594421b68..6775ed62f9 100755
>> > --- a/t/t3451-history-reword.sh
>> > +++ b/t/t3451-history-reword.sh
>> > @@ -263,7 +263,7 @@ test_expect_success '--ref-action=head updates only HEAD' '
>> >  
>> >  		# When told to update HEAD, only, the command will refuse to
>> >  		# rewrite commits that are not an ancestor of HEAD.
>> > -		test_must_fail git history reword --ref-action=head theirs 2>err &&
>> > +		test_must_fail git -c core.editor=false history reword --ref-action=head theirs 2>err &&
>> >  		test_grep "rewritten commit must be an ancestor of HEAD" err &&
>> 
>> This ensures that the editor is never consulted?  How?  Running the
>> "false" editor would give us a different error, like "your editor
>> exited with non-zero status, telling us to abort" or something?
>
> Yup, exactly that. We'd see "Aborting commit as launching the editor
> failed." instead of the above error message.

I see at least two people wondered during the review, so perhaps

 		# When told to update HEAD, only, the command will refuse to
 		# rewrite commits that are not an ancestor of HEAD.
-		test_must_fail git history reword --ref-action=head theirs 2>err &&
+		# Use the "false" editor that shows a different error when run
+		test_must_fail git -c core.editor=false history reword --ref-action=head theirs 2>err &&
  		test_grep "rewritten commit must be an ancestor of HEAD" err &&

would help future readers.  Or it might be too much.