From: Duy Nguyen Date: Fri, 22 Feb 2019 09:21:16 GMT Subject: Re: [PATCH v2 1/1] worktree add: sanitize worktree names Message-ID: In-Reply-To: <6fe399f0-98ad-37e6-f4b1-3a3f6e4bce03@ramsayjones.plus.com> On Fri, Feb 22, 2019 at 12:42 AM Ramsay Jones 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. 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