Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 12, 2018, 09:30 UTC
Message-ID: <nycvar.QRO.7.76.6.1804121102260.65@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <26c21a44-92b3-80f4-5211-034c64f7568c@talktalk.net>

Hi Phillip,

On Wed, 11 Apr 2018, Phillip Wood wrote:

> On 10/04/18 13:30, Johannes Schindelin wrote:
> 
> Firstly let me say that I think expanding the documentation and having an
> example is an excellent idea.

Thanks! At first, I meant to leave this for others to contribute, but I think it makes sense for me to describe it, as I do have a little bit of experience with rebasing merges.

Show 27 quoted lines

> > +
> > +------------
> > +label onto
> > +
> > +# Branch: refactor-button
> > +reset onto
> > +pick 123456 Extract a generic Button class from the DownloadButton one
> > +pick 654321 Use the Button class for all buttons
> > +label refactor-button
> > +
> > +# Branch: report-a-bug
> > +reset refactor-button # Use the Button class for all buttons
> > +pick abcdef Add the feedback button
> > +label report-a-bug
> > +
> > +reset onto
> > +merge -C a1b2c3 refactor-button # Merge 'refactor-button'
> > +merge -C 6f5e4d report-a-bug # Merge 'report-a-bug'
> > +------------
> > +
> > +In contrast to a regular interactive rebase, there are `label`, `reset` and
> > +`merge` commands in addition to `pick` ones.
> > +
> > +The `label` command puts a label to whatever will be the current
> 
> s/puts a label to/associates a label with/ would be clearer I think. Maybe
> s/whatever will be the current revision/the current HEAD/ an well?

Thanks, I incorporated both changes here.

Show 7 quoted lines

> > +revision when that command is executed. Internally, these labels are
> > +worktree-local refs that will be deleted when the rebase finishes or
> > +when it is aborted.
> 
> I agree they should be deleted when the rebase is aborted but I cannot see any
> changes to git-rebase.sh to make that happen. I think they should also be
> deleted by 'rebase --quit'.

Oh right! For some reason I thought I already hooked up rebase--helper --abort when rebase was called with --abort or quit, but I had not managed yet. I think I will leave this for later, or for GSoC, or something.

In the meantime, I'll just drop the "or when it is aborted.".

Show 7 quoted lines

> > That way, rebase operations in multiple worktrees
> > +linked to the same repository do not interfere with one another.
> > +
> > +The `reset` command is essentially a `git reset --hard` to the specified
> > +revision (typically a previously-labeled one).
> 
> s/labeled/labelled/

As Eric pointed out, I am using 'murricane spelling here (or is it speling? Ya never know these days).

> I think it would be worthwhile to point out that unlike the other commands
> this will not preserve untracked files. Maybe something like
> "Note that unlike the `pick` or `merge` commands or initial checkout when the
> rebase starts the `reset` command will overwrite any untracked files."

You know what? You just pointed out a bug in my thinking. Previously, I thought that this is impossible, that you cannot overwrite untracked files because we labeled this revision previously, so the only new files to write by `reset` were tracked files previous. But that forgets `exec` and `reset` with unlabeled revisions (e.g. for cousins).

So I changed the `reset` command to refuse overwriting untracked files...

Thank you for improving this patch series! Dscho