On Wed, Feb 27, 2019 at 7:09 AM Jeff King <peff@xxxxxxxx> wrote: > On Tue, Feb 26, 2019 at 05:58:51PM +0700, Nguyễn Thái Ngọc Duy wrote: > > Update 'worktree add' code to remove special characters to follow > > these rules. The code could replace chars with '-' more than > > necessary, but it keeps the code simple. In the future the user will > > be able to specify the worktree name by themselves if they're not > > happy with this dumb character substitution. > > So notably this gets around ".." and ".lock" by just disallowing "." > entirely. I think I'm OK with that for worktrees. It does make me a > little nervous to see this new public function, though: > > > +int char_allowed_in_refname(int ch) [...] > > because it's not entirely accurate, as you noted above. I wonder if we > could name this differently to warn people that the refname rules are > not so simple. > > If we just cared about saying "is this worktree name valid", I'd suggest > actually constructing a sample refname with the worktree name embedded > in it and feeding that to check_refname_format(). But because you want > to actually sanitize, I don't think there's an easy way to reuse it. > > So this approach is probably the best we can do, though I do still think > it's worth renaming that function (and/or putting a big warning comment > in front of it). The above arguments seem to suggest the introduction of a companion to check_refname_format() for sanitizing, perhaps named sanitize_refname_format(), in ref.[hc]. The potential difficulty with that is defining exactly what "sanitize" means. Will it be contextual? (That is, will git-worktree have differently sanitation needs than some other facility?) If so, perhaps a 'flags' argument could control how sanitization is done.
