Re: [PATCH 6/6] completion: clarify ls-tree, archive, show completion

[Ramkumar Ramachandra <artagnon@gmail.com>]

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<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?

> 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?

  $ 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<TAB>

Note how log doesn't even error 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.