threads / patch / 34051

patchcompletion: add deprecated __git_complete_file ()

Subject: [PATCH] completion: add deprecated __git_complete_file ()

## tl;dr

7 messages between Jun 7, 2013 and Jun 8, 2013. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Ramkumar Ramachandra· Jun 7, 2013, 19:09 UTC · lore

77c130 (completion: clarify ls-tree, archive, show completion, 2013-06-02) removed __git_complete_file () because it had no callers left in the file. However, to avoid breaking user scripts that may depend on this, add it back as a deprecated alias.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Based on pu.

 contrib/completion/git-completion.bash | 5 +++++
 1 file changed, 5 insertions(+)

Show changes to contrib/completion/git-completion.bash +5 −0

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d0a9ba4..0fb81c9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -489,6 +489,11 @@ __git_complete_revlist_file ()
 	esac
 }
 
+# no callers; deprecated alias
+__git_complete_file ()
+{
+	__git_complete_revlist_file
+}
 
 # __git_complete_index_file requires 1 argument:
 # 1: the options to pass to ls-file

-- 
1.8.3.244.g98dd9db.dirty

Felipe Contreras· Jun 7, 2013, 19:25 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] completion: add deprecated __git_complete_file ()

On Fri, Jun 7, 2013 at 2:09 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

> 77c130 (completion: clarify ls-tree, archive, show completion,
> 2013-06-02) removed __git_complete_file () because it had no callers
> left in the file.  However, to avoid breaking user scripts that may
> depend on this, add it back as a deprecated alias.

This is fine by me, but at some point we need to decide how we should prefix the functions that are supposed to be used by external scripts.

Also, maybe we should start adding '# TODO remove in v2.0' so we remember to do that in v2.0.

-- 
Felipe Contreras

Ramkumar Ramachandra· Jun 7, 2013, 19:29 UTC · re: Felipe Contreras · lore

Re: [PATCH] completion: add deprecated __git_complete_file ()

Felipe Contreras wrote:

> This is fine by me, but at some point we need to decide how we should
> prefix the functions that are supposed to be used by external scripts.

Yeah, I thought __ meant "internal" :/

> Also, maybe we should start adding '# TODO remove in v2.0' so we
> remember to do that in v2.0.

While at it, let's also clean up the deprecated zsh nonsense in git-completion.bash.

Felipe Contreras· Jun 7, 2013, 19:31 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] completion: add deprecated __git_complete_file ()

On Fri, Jun 7, 2013 at 2:29 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 11 quoted lines

> Felipe Contreras wrote:
>> This is fine by me, but at some point we need to decide how we should
>> prefix the functions that are supposed to be used by external scripts.
>
> Yeah, I thought __ meant "internal" :/
>
>> Also, maybe we should start adding '# TODO remove in v2.0' so we
>> remember to do that in v2.0.
>
> While at it, let's also clean up the deprecated zsh nonsense in
> git-completion.bash.

And the _git _gitk compatibility wrappers.

-- 
Felipe Contreras

SZEDER Gábor· Jun 8, 2013, 00:35 UTC · re: Felipe Contreras · lore

Re: [PATCH] completion: add deprecated __git_complete_file ()

On Fri, Jun 07, 2013 at 02:25:54PM -0500, Felipe Contreras wrote:

Show 8 quoted lines

> On Fri, Jun 7, 2013 at 2:09 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> > 77c130 (completion: clarify ls-tree, archive, show completion,
> > 2013-06-02) removed __git_complete_file () because it had no callers
> > left in the file.  However, to avoid breaking user scripts that may
> > depend on this, add it back as a deprecated alias.
> 
> This is fine by me, but at some point we need to decide how we should
> prefix the functions that are supposed to be used by external scripts.

Or rather how we should prefix the functions that are _not_ supposed to be used by external scripts. That way all public functions would retain the "__git" prefix and existing scripts calling only common functions like __git_(heads|git_refs|find_on_cmdline|etc.) would work without modification.

Best, Gábor

Junio C Hamano· Jun 7, 2013, 20:38 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] completion: add deprecated __git_complete_file ()

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 8 quoted lines

> 77c130 (completion: clarify ls-tree, archive, show completion,
> 2013-06-02) removed __git_complete_file () because it had no callers
> left in the file.  However, to avoid breaking user scripts that may
> depend on this, add it back as a deprecated alias.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Based on pu.

Will queue; thanks. With this, I think it will be safe to push the series in question to 'master'.

SZEDER Gábor· Jun 8, 2013, 00:24 UTC · re: Junio C Hamano · lore

Re: [PATCH] completion: add deprecated __git_complete_file ()

On Fri, Jun 07, 2013 at 01:38:16PM -0700, Junio C Hamano wrote:

Show 13 quoted lines

> Ramkumar Ramachandra <artagnon@gmail.com> writes:
> 
> > 77c130 (completion: clarify ls-tree, archive, show completion,
> > 2013-06-02) removed __git_complete_file () because it had no callers
> > left in the file.  However, to avoid breaking user scripts that may
> > depend on this, add it back as a deprecated alias.
> >
> > Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> > ---
> >  Based on pu.
> 
> Will queue; thanks.  With this, I think it will be safe to push the
> series in question to 'master'.

Safe? Yes, at least scripts won't break because of the missing function.

However, I still think it would be worth reverting at least the hunks modifying the completion functions of ls-tree and archive. Or better yet, the whole series.