[Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Junio C Hamano]

What should happen when I turn "pick" to "edit" in a "rebase -i"
session, check what got checked out in the working tree, find it
satisfactory and decide not to make any change to the files, and say
"rebase --continue"?

The current implementation seems to just move to the next step,
without offering a chance to edit the log message.  I do not know
offhand if this is something we changed recently, or if it has been
that way forever.

I personally found this a bit unintuitive, because in my metal
model, "reword" is a mere subset of "edit": the latter would give me
chances to change both the contents and the log, while the former
only would offer me a chance to change the log.

But the actual behaviour does not match that mental model.  "edit"
is purely about editing the worktree files, and only if files (hence
the tree recorded) are modified, a chance to edit the log is offered
to adjust the message to what the new tree brings on top of the
parent commit.

Of course, we can work it around with "git rebase --edit-todo"
before saying "git rebase --continue".  But the current behaviour
somehow feels optimized for a wrong case.  Admittedly, it is logical
that it does not offer a chance to edit the log message if we did
not make any change to the working tree.  After all, the reason why
it may become necessary to edit the log is because the user made
some changes to the tree in the first place.  And by not opening the
editor, only to close it without making any change, the command is
saving the user some keystrokes.

But given that saying "edit" and not making any changes is a rare
case, it feels wrong thing to optimize for.

Anyway.

Re: [Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Sean Allred]

Junio C Hamano <gitster@pobox.com> writes:
> I personally found this a bit unintuitive, because in my metal
> model, "reword" is a mere subset of "edit": the latter would give me
> chances to change both the contents and the log, while the former
> only would offer me a chance to change the log.

My mental model is not quite the same, interestingly: 'reword' and
'edit' are not-quite-orthogonal, not-quite-parallel in terms of intent.
In 'reword', I know that I just care about the log. In 'edit', I don't
know *what* I'm going to do -- in fact, my mental model is more friendly
to the idea that 'edit' => 'pick then break'. This of course may be a
mental model learned from the observed behavior and not conceived from
the ideal behavior.

My 2c: in the case where you're changing the tree, you will already be
prompted to change the log. It appears the assumption is that if you do
not change tree, you will not need to change the log.

This assumption holds true for me, but my workflow is generally to get
each commit's patches into the desired state and only *then* spend some
quality time with my messages. That's certainly not the only workflow,
though.

> After all, the reason why it may become necessary to edit the log is
> because the user made some changes to the tree in the first place. And
> by not opening the editor, only to close it without making any change,
> the command is saving the user some keystrokes.

...and you seem to be on the same lines of thinking as I am. Playing
devil's advocate a bit: there are certainly other cases in Git where the
editor pops open and I have muscle memory to close it. I wish I could
recall in this moment where those cases were, but I don't know that
avoiding an invocation of the editor is a good reason not to invoke the
editor if that's the Right(tm) thing to do -- that seems to be a
circular argument.

Setting aside the obvious reality that an actual change here could have
pretty serious UX considerations for folks with muscle-memory, what in
your opinion would be the right thing to do? Why? Are rebase commands
'shortcuts' or are they intended to be orthogonal? Do they have designed
purposes?

I'm wondering if you can tease out what the 'ideal' state looks like to
you, then you can identify what if anything there is to be done about
it.

-Sean

-- 
Sean Allred

Re: [Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Junio C Hamano]

Sean Allred <allred.sean@gmail.com> writes:

> Setting aside the obvious reality that an actual change here could have
> pretty serious UX considerations for folks with muscle-memory, what in
> your opinion would be the right thing to do? Why? Are rebase commands
> 'shortcuts' or are they intended to be orthogonal? Do they have designed
> purposes?
>
> I'm wondering if you can tease out what the 'ideal' state looks like to
> you, then you can identify what if anything there is to be done about
> it.

Oh, it would be very simple.

If I say "edit", whether I made a tree change or not, I want to get
an editor when I said "rebase --continue".  If I say "reword", I
want to get an editor _without_ having a chance to muck with the
tree status.  That would be the "ideal" behaviour, iow, the "mental
model" is just "edit" gives the users a chance to edit both trees
(by first giving control back to a shell prompt) and the log message
(by opening the editor upon "--continue"), while "reword" is only
about the message so does not give shell prompt back to the user
(unless absolutely necessary, that is.  If the "reword" were to
conflict due to tree changes in earlier steps, it would need to give
control back to a shell prompt to ask the user's help to resolve the
conflict.  It is just that when there is no need to edit the tree
otherwise, that is skipped).

Re: [Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]

On Fri, May 17, 2024 at 12:09:54AM -0700, Junio C Hamano wrote:
> Sean Allred <allred.sean@gmail.com> writes:
> 
> > Setting aside the obvious reality that an actual change here could have
> > pretty serious UX considerations for folks with muscle-memory, what in
> > your opinion would be the right thing to do? Why? Are rebase commands
> > 'shortcuts' or are they intended to be orthogonal? Do they have designed
> > purposes?
> >
> > I'm wondering if you can tease out what the 'ideal' state looks like to
> > you, then you can identify what if anything there is to be done about
> > it.
> 
> Oh, it would be very simple.
> 
> If I say "edit", whether I made a tree change or not, I want to get
> an editor when I said "rebase --continue".  If I say "reword", I
> want to get an editor _without_ having a chance to muck with the
> tree status.  That would be the "ideal" behaviour, iow, the "mental
> model" is just "edit" gives the users a chance to edit both trees
> (by first giving control back to a shell prompt) and the log message
> (by opening the editor upon "--continue"), while "reword" is only
> about the message so does not give shell prompt back to the user
> (unless absolutely necessary, that is.  If the "reword" were to
> conflict due to tree changes in earlier steps, it would need to give
> control back to a shell prompt to ask the user's help to resolve the
> conflict.  It is just that when there is no need to edit the tree
> otherwise, that is skipped).

I quite frequently use "edit" just to inspect commits, stop at random
points in the history, run tests and whatnot. So this would be a UX
regression for me because I do not want to change commit messages and
don't want to be bothered.

With the introduction of the "break" command you can certainly argue
that "edit" is the wrong command to use in my case. Muscle memory is
hard to retrain though :)

One could potentially make the behaviour configurable so that you get to
choose how "edit" behaves.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Dragan Simic]

On 2024-05-17 09:32, Patrick Steinhardt wrote:
> On Fri, May 17, 2024 at 12:09:54AM -0700, Junio C Hamano wrote:
>> Sean Allred <allred.sean@gmail.com> writes:
>> 
>> > Setting aside the obvious reality that an actual change here could have
>> > pretty serious UX considerations for folks with muscle-memory, what in
>> > your opinion would be the right thing to do? Why? Are rebase commands
>> > 'shortcuts' or are they intended to be orthogonal? Do they have designed
>> > purposes?
>> >
>> > I'm wondering if you can tease out what the 'ideal' state looks like to
>> > you, then you can identify what if anything there is to be done about
>> > it.
>> 
>> Oh, it would be very simple.
>> 
>> If I say "edit", whether I made a tree change or not, I want to get
>> an editor when I said "rebase --continue".  If I say "reword", I
>> want to get an editor _without_ having a chance to muck with the
>> tree status.  That would be the "ideal" behaviour, iow, the "mental
>> model" is just "edit" gives the users a chance to edit both trees
>> (by first giving control back to a shell prompt) and the log message
>> (by opening the editor upon "--continue"), while "reword" is only
>> about the message so does not give shell prompt back to the user
>> (unless absolutely necessary, that is.  If the "reword" were to
>> conflict due to tree changes in earlier steps, it would need to give
>> control back to a shell prompt to ask the user's help to resolve the
>> conflict.  It is just that when there is no need to edit the tree
>> otherwise, that is skipped).
> 
> I quite frequently use "edit" just to inspect commits, stop at random
> points in the history, run tests and whatnot. So this would be a UX
> regression for me because I do not want to change commit messages and
> don't want to be bothered.
> 
> With the introduction of the "break" command you can certainly argue
> that "edit" is the wrong command to use in my case. Muscle memory is
> hard to retrain though :)
> 
> One could potentially make the behaviour configurable so that you get 
> to
> choose how "edit" behaves.

I agree that it would be best to introduce a new configuration option
for this purpose.  Making such a change in the behavior of interactive
rebase permanently would probably result in more than a few raised
eyebrows, while a new configuration option would be a safe choice.

Re: [Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Marc Branchaud]

On 2024-05-16 15:21, Junio C Hamano wrote:
> What should happen when I turn "pick" to "edit" in a "rebase -i"
> session, check what got checked out in the working tree, find it
> satisfactory and decide not to make any change to the files, and say
> "rebase --continue"?
> 
> The current implementation seems to just move to the next step,
> without offering a chance to edit the log message.  I do not know
> offhand if this is something we changed recently, or if it has been
> that way forever.

It's been this way forever (or close enough; I've been using Git since 
~2010).

> I personally found this a bit unintuitive, because in my metal
> model, "reword" is a mere subset of "edit": the latter would give me
> chances to change both the contents and the log, while the former
> only would offer me a chance to change the log.
> 
> But the actual behaviour does not match that mental model.  "edit"
> is purely about editing the worktree files, and only if files (hence
> the tree recorded) are modified, a chance to edit the log is offered
> to adjust the message to what the new tree brings on top of the
> parent commit.

My mental model has always been that "edit" means "amend" -- when I tell 
"git rebase" I want to "edit" a commit it (usually) means I intend to 
"git commit --amend" it in some fashion, whether that's to update the 
tree the commit changes, or to tweak the message, or both.  (Or 
something else entirely -- since I'm at the shell prompt, I can do 
anything!  The power!)

I like that "edit" does not open the editor if I decide not to change 
any files, but does open it if I do change any files (and that "git 
rebase" does the amending for me if I just stage the changes and say 
"git rebase --continue").  Seeing the editor after I say "git rebase 
--continue" is also a good reminder to me that I've changed something in 
the tree being committed.  I've found this helpful if I've been 
distracted during a rebase session.

If I *know* that I only want to edit the message, then I say "reword".

> Of course, we can work it around with "git rebase --edit-todo"
> before saying "git rebase --continue".

Or, more directly, just "git commit --amend".

> But the current behaviour
> somehow feels optimized for a wrong case.  Admittedly, it is logical
> that it does not offer a chance to edit the log message if we did
> not make any change to the working tree.  After all, the reason why
> it may become necessary to edit the log is because the user made
> some changes to the tree in the first place.  And by not opening the
> editor, only to close it without making any change, the command is
> saving the user some keystrokes.

Yes, and I've always appreciated this behaviour.

		M.

> But given that saying "edit" and not making any changes is a rare
> case, it feels wrong thing to optimize for.
> 
> Anyway.
> 

Re: [Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Sean Allred]

Dragan Simic <dsimic@manjaro.org> writes:
> On 2024-05-17 09:32, Patrick Steinhardt wrote:
>> I quite frequently use "edit" just to inspect commits, stop at
>> random
>> points in the history, run tests and whatnot. So this would be a UX
>> regression for me because I do not want to change commit messages and
>> don't want to be bothered.
>> With the introduction of the "break" command you can certainly argue
>> that "edit" is the wrong command to use in my case. Muscle memory is
>> hard to retrain though :)
>> One could potentially make the behaviour configurable so that you
>> get to
>> choose how "edit" behaves.
>
> I agree that it would be best to introduce a new configuration option
> for this purpose.  Making such a change in the behavior of interactive
> rebase permanently would probably result in more than a few raised
> eyebrows, while a new configuration option would be a safe choice.

I would strongly disagree that new configuration would be best here.
git-config isn't a silver bullet for finding a 'one size fits all'
solution -- on the contrary, it usually only serves to confuse the
community in situations where a decision should just have been made.

I've thought on this on and off and, were I asked the question 'what
would you do if there were no precedent', I'd agree that 'edit'
shouldn't simply be a shortcut for 'pick' then 'break'. Sequence editors
are where such shortcuts should be implemented, IMO.

I want to be clear that I'm not saying the behavior *should* change at
this point (at least not without the set of other breaking changes
discussed elsewhere in the so-called Git 3.0 thread), but I can see why
'edit' having the behavior of invoking the editor every time would be
desirable -- *because* each command should ideally have one clear job.

git-rebase(1) does briefly describe these commands, but perhaps not in a
way that makes the relationship between them as clear as it could be:

       By replacing the command "pick" with the command "edit", you can
       tell git rebase to stop after applying that commit, so that you
       can edit the files and/or the commit message, amend the commit,
       and continue rebasing.

       To interrupt the rebase (just like an "edit" command would do,
       but without cherry-picking any commit first), use the "break"
       command.

       If you just want to edit the commit message for a commit, replace
       the command "pick" with the command "reword".

       To drop a commit, replace the command "pick" with "drop", or just
       delete the matching line.

..

I don't disagree that the behavior of 'edit' could change in a Git 3.0
and that such would be a positive change, but:

- such would be a breaking change -- I know of several tools that use
  interactive rebase in a scripted/non-interactive fashion
- introducing configuration to avoid the breaking change would, IMO,
  cause more harm than good. In my mind, it would be no different than
  having opt-in configuration to automatically staged everything when
  running git-commit.

-- 
Sean Allred

Re: [Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> I quite frequently use "edit" just to inspect commits, stop at random
> points in the history, run tests and whatnot. So this would be a UX
> regression for me because I do not want to change commit messages and
> don't want to be bothered.

I sometimes (ab)use the "edit" exactly like that myself, so a
simple-minded unconditional change of behaviour would be a UX
regression to me as well.

> With the introduction of the "break" command you can certainly argue
> that "edit" is the wrong command to use in my case. Muscle memory is
> hard to retrain though :)

Yes, with a vim macro or its Emacs equivalent, it should be just as
easy as doing "s/^pick /edit /" to insert "break" after every line
that begins with "pick ", but "just as easy" is still an unwanted
forced change to the end-users.  If I really wanted to, the right
and only way out is to introduce a new insn similar to "edit" but
behaves more like what I said earlier.

Thanks.

Re: [Q] rebase -i: turn "pick" to "edit", make no change, what should happen?

[Junio C Hamano]

Marc Branchaud <marcnarc@xiplink.com> writes:

> On 2024-05-16 15:21, Junio C Hamano wrote:
>> What should happen when I turn "pick" to "edit" in a "rebase -i"
>> session, check what got checked out in the working tree, find it
>> satisfactory and decide not to make any change to the files, and say
>> "rebase --continue"?
>> The current implementation seems to just move to the next step,
>> without offering a chance to edit the log message.  I do not know
>> offhand if this is something we changed recently, or if it has been
>> that way forever.
>
> It's been this way forever (or close enough; I've been using Git since
> ~2010).

Thanks.