Re: [PATCH v2 1/1] worktree add: sanitize worktree names

[Duy Nguyen <pclouds@gmail.com>]

On Fri, Feb 22, 2019 at 12:42 AM Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
> > +static void sanitize_worktree_name(struct strbuf *name)
> > +{
> > +     char *orig_name = xstrdup(name->buf);
> > +     int i;
> > +
> > +     /*
> > +      * All special chars replaced with dashes. See
> > +      * check_refname_component() for reference.
> > +      * Note that .lock is also turned to -lock, removing its
> > +      * special status.
> > +      */
> > +     for (i = 0; i < name->len; i++) {
> > +             if (strchr(":?[]\\~ \t@{}*/.", name->buf[i]))
> > +                     name->buf[i] = '-';
> > +     }
> > +
> > +     /* remove consecutive dashes, leading or trailing dashes */
>
> Why? So, '[fred]' will be 'sanitized' to 'fred' (rather than '-fred-'),
> which would increase the chance of a 'collision' with the 'fred'
> worktree (not very likely, but still). Is that useful? How about
> 'x86_64-*-gnu' which now becomes 'x86_64-gnu'?

It is useful when you want to specify HEAD of [fred] for example.
Writing worktrees/fred/HEAD is a bit better than
worktrees/-fred-/HEAD. I haven't done it yet, but these names will be
shown in "git worktree list" too and lots of dashes does not improve
readability. Collision is not a problem because if fred is taken, the
final name would be fred1 or fred<some other number>.

If you're really bothered with this, you will be able to specify the
name you want (you can't, yet). You still have to pass the valid
refname check, but you have a lot more flexibility.

So this code only needs to work mostly ok for the common case and I
could go either way, clean up consecutive dashes or not. I suppose
simpler code would be the tie breaker.
-- 
Duy