Re: [PATCH] rebase-interactive: show revert option and add single letter shortcut

[Junio C Hamano <gitster@pobox.com>]

Michael Lohmann <mial.lohmann@gmail.com> writes:

> A `revert` in an interactive rebase can be useful, e.g. if a faulty
> commit was pushed to the main branch already, so you can't just drop it.

But wouldn't that be typically done simply by running "git revert",
totally outside the context of "git rebase -i"?

Interactive rebase is more geared toward rearranging an already
built history *after* such a "git revert" is made and possibly other
commits are made either before or after that commit that was created
by "git revert".

And when "git rebase -i" sees such a series of commits, e.g.,

	git checkout -b side-branch master
	git commit -m "some work"
	git commit -m "some more work"
	git revert -m "revert a bad one for now" master~4
	git commit -m "tentative alternative for what master~4 did"
	git rebase -i master

The "revert a bad one for now" commit looks to the machinery just
like any other commit in the todo list.

> When you are already working in a feature branch you might just want to
> revert said commit right where you branched off from main, so you can
> continue working on the feature you intend while still being up-to-date
> otherwise.

Yes, I can see why sometimes you want to work on a history with
effects from certain commits removed.  But that does not explain why
you want to _insert_ a revert that you do not even have in the
history anywhere before you start your interactive rebase.

> Another reason why you might not want to drop a commit is if it is a
> work in progress one and you want to properly fix it later, but for now
> need to revert the changes. That way it is a lot cleaner to structure
> your branch like this:
>
>     A---B---C       (B is WIP commit you cannot use as is)
> =>
>     A---B---~B---C  (temporarily revert B (called "~B") directly after
>                      it is created, until you find the time to fix it -
>                      at which point in time you will naturally drop the
>                      revert commit)
>
> This way you still have the WIP patch, but "your history is not broken
> the whole time".

A much cleaner way to structure your branch is not to muck with such
tentative changes *on* the branch you eventually want to store the
final result on.  Fork another branch and rebase B away:

    $ git checkout -b topic-ng topic [*]
    $ git rebase -i A

to obtain

	A---B---C    topic
	 \
	  C          topic-ng

and then you'd build on top a better version of B eventually

	A---B---C
	 \
	  C---D---E---...---B*---X

And after that you may "rebase -i" to refine the result, and then
get rid of the tentative work:

    $ work work work (still on topic-ng)
    ...
    $ git commit -m "X"
    $ git rebase -i A
    $ git branch -M topic 

Nowhere in the above two flow, there is no need to manually insert a
new "make a revert here of that other commit" in the todo list.
So I am not sure if I buy the above, especially this part:

> +To revert a commit, add a line starting with "revert" followed by the commit
> +name.

It really smells like a confusing odd man out, among all other
existing instructions that are naturally created by the
rebase/sequencer machinery and all the user needs to do is to
shuffle them, never creating a new thing.



[Footnote]

 * I do this too frequently that I often do without a separate -ng
   branch; once you get used to the flow, you learn to do this kind
   of thing on detached HEAD instead, so this step would look more
   like

   $ git checkout --detach topic

   and the remainder of the above procedure will not change.  The
   last step would become

   $ git checkout -B topic

   to bring me back to the target branch in a completed form.

   One beauty about this "detached HEAD" approach is that output
   from "git reflog topic" will show the refinement of the topic as
   a single atomic event.