Re: [PATCH 1/8] sequencer: introduce new commands to reset the revision

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 29, 2018, 20:50 UTC
Message-ID: <nycvar.QRO.7.76.6.1801292146270.35@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <CAPig+cQbG2s-LrAo9+7C7=dXifbWFJ3SzuNa-QePHDk7egK=jg@mail.gmail.com>

Hi Eric,

On Fri, 19 Jan 2018, Eric Sunshine wrote:

Show 18 quoted lines

> On Thu, Jan 18, 2018 at 10:35 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> > [...]
> > +static int do_reset(const char *name, int len)
> > +{
> > +       [...]
> > +       if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
> > +               return -1;
> > +
> > +       for (i = 0; i < len; i++)
> > +               if (isspace(name[i]))
> > +                       len = i;
> 
> What is the purpose of this loop? I could imagine that it's trying to
> strip all whitespace from the end of 'name', however, to do that it
> would iterate backward, not forward. (Or perhaps it's trying to
> truncate at the first space, but then it would need to invert the
> condition or use 'break'.) Am I missing something obvious?

Yes, you are missing something obvious. The idea of the `reset` command is that it not only has a label, but also the oneline of the original commit:

	reset branch-point sequencer: prepare for cleanup

In this instance, `branch-point` is the label. And for convenience of the person editing, it also has the oneline. This came in *extremely* handy when editing the commit topology in Git for Windows, i.e. when introducing topic branches or flattening them.

In the Git garden shears, I separated the two arguments via `#`:

	reset branch-point # sequencer: prepare for cleanup

I guess that is actually more readable, so I will introduce that into this patch series, too.

Ciao, Dscho