On Sat, Jan 4, 2025 at 6:35 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > * Added a second patch for another bug discovered by the same reporter, > > where branch:path/to/file/named/major-gaffed is interpreted as a request > > for a commit (namely affed) rather than a blob. (At least, assuming > > commit affed exists) > > > > The second patch has some backward compatibility concerns. People used to be > > able to do e.g. git show ${garbage}-g${hash}. I tightened it to > > ${valid_refname}-${number}-g${hash}, but do we want to allow e.g. > > ${valid_refname}-g${hash} (allowing the count to be omitted) or maybe even > > allow a subset of invalid refnames? > > My take on it is that it is an absolute no-no if we require that > ${valid_refname} exists locally, and it is still iffy if we checked > ${valid_refname} with check_format() (because the definition of > validity can change over time, and we would not know the rules that > were valid back when the reference to the commit was written). Fair enough. However... > Otherwise a tightened rule would make "${garbage}-g${hash}" less > useful to copy-and-paste from a text file to command line. > > In general what would we do if a string can be interpreted in > multiple ways in _different_ parts of the object-name codepaths. We > all know that "affed" would trigger the "ambiguous object name" > error if there are more than one object whose object name begins > with "affed", but if "${garbage}-gaffed" can be interpreted as the > name of an object whose object name begins with "affed" and also can > be interpreted as the name of another object that sits at a path > that ends with "-gaffed" in some tree object, regardless of how the > leading part "${garbage}" looks like, it would be desirable if we > declared such a string as "ambiguous" the same way. How would that be desirable? There's no possible way to disambiguate. While abbreviated revisions can just be modified to be less abbreviated, paths cannot be spelled any other way. How would you spell master:path/to/who-gabbed in a "less ambiguous" way to differentiate it from commit abbed? As far as I can tell, this proposal just leaves the user stuck with an error with no way to get the path they want. If you don't like check_format() being called on the leading part of the string, can we at least enforce that there is no ':', so that we can successfully request explicit paths of given revisions and know that we'll get them? (That'd disallow e.g. next^{/doc:}-12-gabbed, but that clearly was never a valid describe output anyway.)