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

On Tue, Apr 17, 2012 at 2:16 AM, Jonathan Nieder [off-list ref] wrote:

Felipe Contreras wrote:

quoted

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.

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