Show 147 quoted lines
> 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".