From: Ben Knoble <ben.knoble@gmail•com>
To: Patrick Steinhardt <ps@pks•im>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 2/4] builtin/history: check for merges before asking for user input
Date: Fri, 13 Feb 2026 08:42:35 -0500 [thread overview]
Message-ID: <3C51B8C4-E594-44ED-97F0-0EB7C36A3BCB@gmail.com> (raw)
In-Reply-To: <aY670nh0ElOZU46j@pks.im>
> Le 13 févr. 2026 à 00:51, Patrick Steinhardt <ps@pks•im> a écrit :
>
> On Thu, Feb 12, 2026 at 05:20:08PM -0500, D. Ben Knoble wrote:
>>> On Thu, Feb 12, 2026 at 7:45 AM Patrick Steinhardt <ps@pks•im> wrote:
>>> diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
>>> index 6775ed62f9..12a9a7d051 100755
>>> --- a/t/t3451-history-reword.sh
>>> +++ b/t/t3451-history-reword.sh
>>> @@ -203,7 +203,7 @@ test_expect_success 'can reword a merge commit' '
>>>
>>> # It is not possible to replay merge commits embedded in the
>>> # history (yet).
>>> - test_must_fail git history reword HEAD~ 2>err &&
>>> + test_must_fail git -c core.editor=false history reword HEAD~ 2>err &&
>>> test_grep "replaying merge commits is not supported yet" err &&
>>>
>>> # But it is possible to reword a merge commit directly.
>>
>> Hm, I don't quite see how "false" tests that we don't invoke the
>> editor at all, though I'm not sure if that behavior should be encoded
>> in the test or not. (Looks like Junio noticed the same in 1/4; I'm
>> getting better at reading my mail before sending, sometimes… :)
>>
>> Otherwise looks good!
>
> As replied in the other thread, this would abort with "Aborting commit
> as launching the editor failed." if Git tried to execute the editor.
>
> I'm not quite sure whether I get the other remark about "should be
> encoded in the test or not". Do you mean to say we should use a separate
> test?
>
> Patrick
No, just musing on whether “did not launch editor” is an important behavior to avoid regressing. I think yes, and I have no real objection to it :)
next prev parent reply other threads:[~2026-02-13 13:42 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-12 12:44 [PATCH 0/4] builtin/history: some smaller UI improvements Patrick Steinhardt
2026-02-12 12:44 ` [PATCH 1/4] builtin/history: perform revwalk checks before asking for user input Patrick Steinhardt
2026-02-12 20:04 ` Junio C Hamano
2026-02-13 5:51 ` Patrick Steinhardt
2026-02-13 17:02 ` Junio C Hamano
2026-02-12 12:44 ` [PATCH 2/4] builtin/history: check for merges " Patrick Steinhardt
2026-02-12 22:20 ` D. Ben Knoble
2026-02-12 22:26 ` Junio C Hamano
2026-02-13 5:51 ` Patrick Steinhardt
2026-02-13 5:51 ` Patrick Steinhardt
2026-02-13 13:42 ` Ben Knoble [this message]
2026-02-12 12:44 ` [PATCH 3/4] builtin/history: replace "--ref-action=print" with "--dry-run" Patrick Steinhardt
2026-02-12 20:19 ` Junio C Hamano
2026-02-13 5:50 ` Patrick Steinhardt
2026-02-12 22:20 ` D. Ben Knoble
2026-02-13 5:50 ` Patrick Steinhardt
2026-02-12 12:44 ` [PATCH 4/4] builtin/history: rename "--ref-action=" to "--update-refs=" Patrick Steinhardt
2026-02-13 9:12 ` [PATCH v2 0/5] builtin/history: some smaller UI improvements Patrick Steinhardt
2026-02-13 9:12 ` [PATCH v2 1/5] builtin/history: perform revwalk checks before asking for user input Patrick Steinhardt
2026-02-13 17:20 ` Junio C Hamano
2026-02-13 9:12 ` [PATCH v2 2/5] builtin/history: check for merges " Patrick Steinhardt
2026-02-13 9:12 ` [PATCH v2 3/5] builtin/history: replace "--ref-action=print" with "--dry-run" Patrick Steinhardt
2026-02-13 17:30 ` Kristoffer Haugsbakk
2026-02-16 6:39 ` Patrick Steinhardt
2026-02-18 16:09 ` Kristoffer Haugsbakk
2026-02-13 9:12 ` [PATCH v2 4/5] builtin/history: rename "--ref-action=" to "--update-refs=" Patrick Steinhardt
2026-02-13 9:12 ` [PATCH v2 5/5] Documentation/git-history: document default for "--update-refs=" Patrick Steinhardt
2026-02-13 17:21 ` Junio C Hamano
2026-02-16 6:45 ` [PATCH v3 0/5] builtin/history: some smaller UI improvements Patrick Steinhardt
2026-02-16 6:45 ` [PATCH v3 1/5] builtin/history: perform revwalk checks before asking for user input Patrick Steinhardt
2026-02-16 6:45 ` [PATCH v3 2/5] builtin/history: check for merges " Patrick Steinhardt
2026-02-16 6:45 ` [PATCH v3 3/5] builtin/history: replace "--ref-action=print" with "--dry-run" Patrick Steinhardt
2026-02-16 6:45 ` [PATCH v3 4/5] builtin/history: rename "--ref-action=" to "--update-refs=" Patrick Steinhardt
2026-02-16 6:45 ` [PATCH v3 5/5] Documentation/git-history: document default for "--update-refs=" Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3C51B8C4-E594-44ED-97F0-0EB7C36A3BCB@gmail.com \
--to=ben.knoble@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox