Thread (287 messages) 287 messages, 11 authors, 2018-10-08

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

From: Johannes Schindelin <hidden>
Date: 2018-01-29 20:50:17

Hi Eric,

On Fri, 19 Jan 2018, Eric Sunshine wrote:
On Thu, Jan 18, 2018 at 10:35 AM, Johannes Schindelin
[off-list ref] wrote:
quoted
[...]
+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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help