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

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jun 7, 2013, 17:21 UTC
Message-ID: <CALkWK0mbVgJHRoNXNkc6n7==-H+caNqvzrzUwspS_Eq2sMfJbg@mail.gmail.com>
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?

Show 6 quoted lines

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

Show 6 quoted lines

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