From: Ramkumar Ramachandra Date: Fri, 07 Jun 2013 17:21:53 GMT Subject: Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion Message-ID: In-Reply-To: <20130603192555.GK2192@goldbirke> SZEDER Gábor wrote: > Well, people out there might have completion scriplets for their > aliases or custom git commands which use __git_complete_file(). > Removing this function would break those scripts. What is the advantage of using __git_complete_file() over __git_complete_revlist_file()? Isn't it just a misleading alias? > Arguably the name of __git_complete_file() could describe better what > the function does, or what it did, i.e. it used to provide completion > for the master:Doc notation. But that's only the name. Since > both git ls-tree and git archive understand this notation, calling the > helper for master:Doc in their completion functions is not > misleading at all. But __git_complete_revlist_file() provides all this and more, no? > Now, __git_complete_revlist_file() provides completion both for this > master:Doc notation and for revision ranges, i.e. for > master..n and master...n. However, since neither git > ls-tree nor git archive accept revision ranges, calling > __git_complete_revlist_file() in their completion function would be > misleading. Yeah, they accept tree-ish'es. Isn't __git_complete_file() still a horrible name? $ git ls-tree HEAD:Documentation What file? There's already a __git_complete_index_file(), which is properly named and used by the ls-files family. If anything, we should write a new __git_complete_treeish() function that does what __git_complete_revlist_file() does, except that it doesn't complete revision ranges, right? Frankly, I don't know if it's worth the additional trouble: we do spurious completions all over the place, and we haven't clamped down on any of that. $ git log HEAD:Doc Note how log doesn't even error out. > git show is special, as it understands both the master:Doc > notation and revision ranges, and even the combination of the two, so > calling __git_complete_revlist_file() there would indeed be better. It just accepts any revspec with pathspec filtering, like many many other commands.