From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Elijah Newren <newren@gmail.com>
Cc: "Aleen 徐沛文" <pwxu@coremail.cn>,
"Junio C Hamano" <gitster@pobox.com>,
"Git Mailing List" <git@vger.kernel.org>,
"徐沛文 (Aleen)" <aleen42@vip.qq.com>,
"Aleen via GitGitGadget" <gitgitgadget@gmail.com>
Subject: Re: What's cooking in git.git (Nov 2021, #07; Mon, 29)
Date: Fri, 03 Dec 2021 20:56:43 +0100 [thread overview]
Message-ID: <211203.86wnklqx05.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <CABPp-BGE5Ff=adH3nREMDm38DGLEmtRTcPwuJowHw-ckwpbmqQ@mail.gmail.com>
On Fri, Dec 03 2021, Elijah Newren wrote:
> On Fri, Dec 3, 2021 at 10:37 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> On Wed, Dec 01 2021, Elijah Newren wrote:
>>
>> > On Tue, Nov 30, 2021 at 5:42 PM Aleen 徐沛文 <pwxu@coremail.cn> wrote:
>> >>
>> >> > Please don't, at least not this version. There have been newer
>> >> > submissions with three commits.
>> >> >
>> >> > I also still find the word 'die' confusing, since to me it suggests
>> >> > aborting the whole am operation, and the documentation does not dispel
>> >> > that concern. Even if you don't like 'ask' (for consistency with
>> >> > git-rebase), I think 'stop' or 'interrupt' would be much better
>> >> > options than 'die'. If you really want it to be 'die', I think the
>> >> > behavior needs to be explained in the documentation, rather than just
>> >> > assumed that users will understand it (because I didn't understand it
>> >> > until I read the code).
>> >>
>> >> Dears Newren,
>> >>
>> >> I don't think 'stop' and 'interrupt' words are better to explain more than 'die'
>> >> because they still indicate that it will stop or interrupt the whole am session,
>> >> rather than stop in the middle of it.
>> >
>> > Since you've been through several rounds of revisions already, if this
>> > is the only remaining issue with your series, I wouldn't try to hold
>> > it up for this issue; I just thought it could be fixed while you were
>> > working on the --allow-empty stuff.
>>
>> FWIW I think it's worth getting the UX issue right, tweaking it is
>> relatively easy, and if we can decide on what the thing is called
>> then...
>
> :-)
>
>> > However, while I don't think it's worth holding up your series for
>> > just this issue, I would definitely submit a follow-up RFC patch to
>> > fix the wording, because I do disagree with your assertion here pretty
>> > strongly. Let's look at the meanings of the terms:
>> >
>> > die: connotes something pretty final and irreversible -- people tend
>> > not to revive after death as far as recorded history goes; most such
>> > recorded instances tend to be causes for people to debate the
>> > definition of 'dead'.
>> >
>> > stop: could be final, but is often used in a transitory setting: "stop
>> > and go traffic", "stopped to catch my breath", "the train will stop
>> > at this station", "stop! I want to get out", etc.
>> >
>> > interrupt: seems to nearly always be used as a transitory thing
>> >
>> > Now, in the computer science context, all three terms come up relative
>> > to processes. You can interrupt a process (the kernel does all the
>> > time), but it'll usually continue afterwards. Or you can give it a
>> > SIGINT (interrupt from keyboard signal), which the process can catch
>> > and ignore. You can stop a process (and SIGSTOP cannot be caught),
>> > but you can also continue it later. die essentially means the process
>> > is going to be gone for good (at least short of some kind of black
>> > magic like a reversible debugger such as rr).
>> >
>> > So, I think it's much more likely that 'die' will be misunderstood to
>> > mean abortion of the entire am-process, than that 'stop' or
>> > 'interrupt' would.
>>
>> Why are we exposing an --empty=die at all? It's what we do by default,
>> so why have it? The user can just not provide the "--empty" option, then
>> they'll get the current behavior of die_user_resolve(), which seems to
>> have inpired the name for this "die" (it exits with code 128, just like
>> die()).
>
> That's an interesting angle to take; I hadn't thought of that. It's
> worth considering.
>
> We do often introduce options equivalent to the default as a way to
> either countermand an earlier option (e.g. --do-walk overrides
> --no-walk in git log), or because we want to allow new config options
> that change the default while allowing the user to explicitly request
> something different (e.g. --no-renames was introduced at the same time
> as diff.renames), or because we may want to change the default
> behavior and want users to be able to explicitly request a certain
> type of behavior (e.g. rename detection is the default and
> --no-renames overrides).
>
> It's not clear to me whether that type of flexibility is needed or
> whether we can just leave it unnamed. Three points that may affect
> that decision: (a) the default (and actually, hardcoded) behavior
> before this series for git-am was 'drop', (b) the default behavior
> for git-rebase is 'drop' (though it only affects commits which become
> empty, something we can't determine in the context of am) and (c)
> there was one point during the series that the author asked about just
> removing the 'die' implementation and picking a different default.
>
> The above three points suggest to me that there might reasonably be
> config added to control this or that the default could change, and
> thus that it might be useful to name the interrupt-the-operation
> behavior so that users can explicitly request it. But that's
> somewhere around three levels of chained "might" conditions, so...
> :shrug:
>
>> Once we get rid of "die" the rest of the UI can follow the example of
>> the existing "git rebase" options:
>>
>> --empty={drop,keep,ask}
>>
>> I.e. the "drop" and "keep" will be the same, no "ask" currently, but it
>> can be implemented in the future.
>
> Um, there are minor contextual differences, but what rebase calls
> "ask" (interrupt the operation and tell the users what commands they
> can use to keep or drop the commit and then resume the operation) _is_
> implemented by this series -- it's just being called "die" here.
>
> That's the kind of maddening inconsistency in Git that users complain
> about that I really think we should avoid adding to. If for some
> reason 'ask' from rebase seems like a bad choice, then I think we
> should pick a new name for this interrupt-the-operation behavior that
> makes sense (unlike 'die') for git-am and add it to git-rebase as a
> preferred synonym to 'ask'.
>
>> Maybe I'm missing something, I haven't used "am" in this way (or rebase
>> with --empty=*), but this just seems to me to be needlessly exposing a
>> "die" (or "stop" or whatever) because it's how we implement this.
>>
>> Whereas for the UX we don't need to call it anything except the absence
>> of an --empty option, or perhaps --no-empty.
>
> `--no-empty` would semantically be read by users to mean get rid of
> empty commits, which would be a synonym of 'drop'. I think it'd be as
> confusing as 'die' (and maybe even more so) for naming the
> interrupt-the-operation behavior.
Ah, I didn't look into the finer details. Yes if it does maps to "ask"
in rebase we could just use that, so would this be consistent?:
--empty=die -> --empty=ask
--empty=drop -> (ditto, no change)
--empty=keep -> (ditto, no change)
I think "ask" is a bit of a weird term for this, but I think consistency
trumps a while-we're-at-it fix here.
Whatever new synonym we'd come up with (if that would be justified, that
itself would add to confusion) could be done as a follow-up and
implemented for both "rebase" and "am".
next prev parent reply other threads:[~2021-12-03 20:00 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-30 2:16 What's cooking in git.git (Nov 2021, #07; Mon, 29) Junio C Hamano
2021-11-30 7:33 ` jk/loosen-urlmatch, was " Jeff King
2021-11-30 13:17 ` brian m. carlson
2021-11-30 20:54 ` ab/run-command + em/missing-pager (was: What's cooking in git.git (Nov 2021, #07; Mon, 29)) Ævar Arnfjörð Bjarmason
2021-11-30 21:17 ` Jeff King
2021-11-30 22:09 ` Eric Sunshine
2021-11-30 21:08 ` ab/ci-updates " Ævar Arnfjörð Bjarmason
2021-11-30 21:12 ` ab/config-based-hooks-2 " Ævar Arnfjörð Bjarmason
2021-11-30 21:17 ` fs/test-prereq " Ævar Arnfjörð Bjarmason
2021-12-01 8:53 ` Fabian Stelzer
2021-11-30 21:18 ` jc/c99-var-decl-in-for-loop " Ævar Arnfjörð Bjarmason
2021-11-30 23:07 ` ns/tmp-objdir and ns/remerge-diff Elijah Newren
2021-12-03 19:21 ` Junio C Hamano
2021-12-04 2:58 ` Neeraj Singh
2021-12-04 5:51 ` Elijah Newren
2021-11-30 23:35 ` What's cooking in git.git (Nov 2021, #07; Mon, 29) Elijah Newren
2021-12-01 19:29 ` Victoria Dye
2021-11-30 23:45 ` Elijah Newren
2021-12-01 1:42 ` Aleen 徐沛文
2021-12-01 20:56 ` Elijah Newren
2021-12-03 18:21 ` Ævar Arnfjörð Bjarmason
2021-12-03 19:28 ` Elijah Newren
2021-12-03 19:56 ` Ævar Arnfjörð Bjarmason [this message]
2021-12-06 1:25 ` Aleen 徐沛文
2021-12-06 6:28 ` Junio C Hamano
2021-12-06 6:44 ` Aleen 徐沛文
2021-12-06 6:46 ` Aleen 徐沛文
2021-12-06 17:23 ` Junio C Hamano
2021-12-07 1:06 ` Aleen 徐沛文
2021-12-07 1:29 ` Junio C Hamano
2021-12-07 1:58 ` Aleen 徐沛文
2021-12-06 17:37 ` Elijah Newren
2021-12-06 17:50 ` Junio C Hamano
2021-11-30 23:52 ` en/zdiff3 (was: Re: What's cooking in git.git (Nov 2021, #07; Mon, 29)) Elijah Newren
2021-12-01 22:15 ` en/zdiff3 Junio C Hamano
2021-12-01 8:59 ` What's cooking in git.git (Nov 2021, #07; Mon, 29) Fabian Stelzer
2021-12-03 1:12 ` Junio C Hamano
2021-12-03 5:10 ` [PATCH 0/3] unused-parameter cleanups on top of pw/xdiff-classify-record-in-histogram Jeff King
2021-12-03 5:11 ` [PATCH 1/3] xdiff: drop CMP_ENV macro from xhistogram Jeff King
2021-12-03 5:11 ` [PATCH 2/3] xdiff: drop xpparam_t parameter from histogram cmp_recs() Jeff King
2021-12-03 5:12 ` [PATCH 3/3] xdiff: drop unused flags parameter from recs_match Jeff King
2021-12-06 18:59 ` [PATCH 0/3] unused-parameter cleanups on top of pw/xdiff-classify-record-in-histogram Phillip Wood
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=211203.86wnklqx05.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=aleen42@vip.qq.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=newren@gmail.com \
--cc=pwxu@coremail.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).