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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Eric,

On Fri, 19 Jan 2018, Eric Sunshine wrote:

> On Thu, Jan 18, 2018 at 10:35 AM, Johannes Schindelin
> <johannes.schindelin@xxxxxx> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux