Re: [PATCH v2] completion: add new git_complete helper

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Apr 17, 2012, 07:10 UTC
Message-ID: <CAMP44s3OSepH_XpCcTWLN=bF0+P9fmHtyPnWdQnEocW25jHTpA@mail.gmail.com>
In-Reply-To: <20120416231626.GW12613@burratino>

On Tue, Apr 17, 2012 at 2:16 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 8 quoted lines

> Felipe Contreras wrote:
>
>> What makes you think this is public? It's under the section '# helper
>> functions', which doesn't seem to be public. Plus, it's repeated in
>> rpm, rpmbuild, and rpmbuild-md5.
>
> Ok, you win.  I hadn't realized we were having a debate, but now I do,
> and you have won.

I thought we were trying to agree on a good name for possibly the first (or second, depending how you count) public function. It's important to choose a good name because we can't just change it again in a month.

You are the one who brought the argument that even public functions have the '_' prefix, so it's *your* responsibility to substantiate that argument. I don't think it would be easy to find a precedent like this, but you can prove me wrong by providing the evidence; so far, I don't think that has happened.

> Can we get back to making the completion script nicer for human
> beings that have been using it, please?

Sure, but we need some sort of git_completion function if we want to make it easy for people to define aliases for git commands. And this is a known issue that has been brought in the past.

Show 12 quoted lines

> The following summary may sound annoyed, because I am.  On the other
> hand I know you mean well and am grateful for your work.
>
> I have said that the convention for bash completion scripts is to
> precede all exposed identifier names with an underscore.  You
> mentioned some old counterexamples that have been grandfathered in.
> You mentioned that you do not trust me.  Your bash completion script
> gets loaded immediately instead of being lazy-loaded, probably because
> it is not in /usr/share/bash-completion/completions/.  You claimed
> that nobody would _ever_ ask for bash completion support at the end of
> their .profile and after their custom functions in .profile that do
> something unrelated,

I did not claim such thing. I said that I *doubted* it, not that it was most definitely the case.

> though I used to do that for a long time and
> Debian's default .bashrc loads /etc/bash_completion at the end, too.

I see. That would be troublesome indeed (if somebody chose to add the function before that line, and not at the end), *if* somehow they used git_completion in their scripts.

> I still maintain that namespaces are useful and we should follow the
> conventional ones when they exist.  What is the next step?

What about this?

if ! type git_complete >/dev/null 2>&1; then
git_complete ()
{
	echo "WARNING: This function is not meant for public use; the name" \
		"might change. Use __git_complete for now."
	__git_complete "$@"
}
else
type git_complete | grep -q "WARNING" ||
echo "git_complete is already being used, please notify
git@vger.kernel.org, to" \
	"avoid overriding this function in the future."
fi

We can use that for 1.7.11, and if nobody complains remove the warnings for 1.7.12.

-- 
Felipe Contreras