On Wed, Mar 18, 2020 at 11:26:00AM -0700, Junio C Hamano wrote: > > One thing that's a little ugly here is the embedded dot in the > > subsection (i.e., "filter.<filter>"). It makes it look like a four-level > > key, but really there is no such thing in Git. But everything else we > > tried was even uglier. > > I think this gives us the best arrangement by upfront forcing all > the configuration handers for "<subcommand>.*.<token>" namespace, > current and future, to use "<group-prefix>" before the unbounded set > of user-specifiable values that affects the <subcommand> (which is > "uploadpack"). > > So far, the configuration variables that needs to be grouped by > unbounded set of user-specifiable values we supported happened to > have only one sensible such set for each <subcommand>, so we could > get away without such <group-prefix> and it was perfectly OK to > have, say "guitool.<name>.cmd". Yeah. We have often just split those out into a separate hierarchy from <subcommand> E.g., tar.<format>.command, which is really feeding the git-archive command. We could do that here, too, but I wasn't sure of a good name (this really is upload-pack specific, though I guess in theory other commands could grow a need to look at or restrict "remote object filters"). > Syntactically, the convention to always end such <group-prefix> with > a dot "." may look unusual, or once readers' eyes get used to them, > may look natural. One tiny sad thing about it is that it cannot be > mechanically enforced, but that is minor. The biggest downside to implying a 4-level key is that the case-sensitivity rules may be different. I.e., you can say: UploadPack.filter.blob:none.Allow but not: UploadPack.Filter.blob:none.Allow Since "filter" is part of the subsection, it's case sensitive. We could match it case-insensitively in upload_pack_config(), but it would crop up in other laces (e.g., "git config --unset" would still care). > > We could do "uploadpackfilter.allow" and "uploadpackfilter.<filter>.allow", > > but that's both ugly _and_ separates these options from the rest of > > uploadpack.*. > > There is an existing instance of a configuration that affects > <subcommand> that uses a different word after <subcommand>, which is > credentialCache.ignoreSIGHUP, and I tend to agree that it is ugly. I don't think that's what's going on here. It affects only the credential-cache subcommand, but we avoid hyphens in our key names. So it really is the subcommand; it's just that the name is a superset of another command name. :) > By the way, I noticed the following while I was studying the current > practice, so before I forget... > > -- >8 -- > Subject: [PATCH] separate tar.* config to its own source file > > Even though there is only one configuration variable in the > namespace, it is not quite right to have tar.umask described > among the variables for tag.* namespace. Yeah, this is definitely an improvement. But I was surprised that tar.<format>.* wasn't covered here. It is documented in git-archive. Probably worth moving or duplicating it in git-config. -Peff
