On Fri, Jun 07, 2013 at 10:51:53PM +0530, Ramkumar Ramachandra wrote: > 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()? That it doesn't imply that the command takes refs in the form of ref1..ref2. > Isn't it just a misleading alias? No. It's an implementation detail that __git_complete_file() became an alias to __git_complete_revlist_file() to avoid unnecessary code duplication. And it was a concious decision to keep __git_complete_file() (and __git_complete_revlist()) around in order not to break completion scriplets for users' alieses and custom git commands which might call it. > > 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<TAB> notation. But that's only the name. Since > > both git ls-tree and git archive understand this notation, calling the > > helper for master:Doc<TAB> in their completion functions is not > > misleading at all. > > But __git_complete_revlist_file() provides all this and more, no? Indeed, and this "more" is exactly why it is misleading to call __git_complete_revlist_file() directly for git ls-tree and git archive. > > Now, __git_complete_revlist_file() provides completion both for this > > master:Doc<TAB> notation and for revision ranges, i.e. for > > master..n<TAB> and master...n<TAB>. 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? We can't go back in time to correct it, unfortunately. > 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 I agree that it isn't worth it, and that is exactly why __git_complete_revlist() and __git_complete_file() were unified in __git_complete_revlist_file(). > $ git log HEAD:Doc<TAB> > > Note how log doesn't even error out. But note how git log master..HEAD:Documentation/ errors out. > > git show is special, as it understands both the master:Doc<TAB> > > 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. Which many many other commands do accept ref1..ref2:file? Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html