Re: [PATCH v3 1/1] worktree add: sanitize worktree names
- From
Jeff King <peff@peff.net>
- Date
- Feb 27, 2019, 16:04 UTC
- Message-ID
- <20190227160457.GA30817@sigill.intra.peff.net>
- In-Reply-To
- <CAPig+cRJZBvwsptPOzx3oPSOnt6+uGLoyOr_JbUnku4kdSwdgA@mail.gmail.com>
On Wed, Feb 27, 2019 at 09:23:33AM -0500, Eric Sunshine wrote:
Show 16 quoted lines
> > 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.
I agree that sanitize_refname_format() would be nice, but I'm pretty sure it's going to end up having to duplicate many of the rules from check_refname_format(). Which is ugly if the two ever get out of sync.
But if we could write it in a way that keeps the actual policy logic in one factored-out portion, I think it would be worth doing.
-Peff