Re: [PATCH 1/1] rebase -r: let `label` generate safer labels

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Tue, 3 Sep 2019, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> I'm sightly concerned that this opens the possibility for unexpected effects
> >> if two different labels get sanitized to the same string. I suspect it's
> >> unlikely to happen in practice but doing something like percent encoding
> >> non-alphanumeric characters would avoid the problem entirely.
> >
> > Oh, but we make sure that the labels are unique, via the `label_oid()`
> > function! Otherwise, we would not be able to label more than one merge
> > parent ;-)
>
> It somewhat feels suboptimal, from code followability's point of
> view, to have this "pre-sanitization" to replace isspace() to a
> dash, which is being extended to "all non-alnums", and the uniquefy
> of labels in label_oid(), in two separate places.  I wonder if the
> resulting code becomes easier to follow and harder to introduce new
> bugs, if this part is made to just yield label.buf it obtained form
> the log message as-is and leave the munging to label_oid()?

While the patch looks somewhat bulky without `-w` (due to the
indentation change), I agree that it is conceptually a good idea,
therefore I made it so.

Ciao,
Dscho