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

From: Duy Nguyen <pclouds@gmail.com>
Date: Feb 22, 2019, 09:21 UTC
Message-ID: <CACsJy8Dw0y7qX7jgNH4j_e4YOg-vv0OVDQv99AzT-koZ=Fq-TQ@mail.gmail.com>
In-Reply-To: <6fe399f0-98ad-37e6-f4b1-3a3f6e4bce03@ramsayjones.plus.com>

On Fri, Feb 22, 2019 at 12:42 AM Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:

Show 22 quoted lines

> > +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