Re: [PATCH v3] completion: add new _GIT_complete helper

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 5, 2012, 16:38 UTC
Message-ID: <CAMP44s1dhAjKt3mxVmg2+0qp-QTyjYb1knhxf+m177Cg2ZBC1Q@mail.gmail.com>
In-Reply-To: <20120505155423.GA14684@burratino>

On Sat, May 5, 2012 at 5:54 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 17 quoted lines

> Felipe Contreras wrote:
>
>> Since v3:
>>
>>  * Rename to _GIT_complete to follow bash completion "guidelines"
>>  * Get rid of foo_wrap name
>
> Thanks.  Gábor, does the "all caps _GIT_ prefix for public API
> functions" convention look like one we should adopt?  If I understand
> correctly, previously contrib/completion/git-completion.bash used
> leading double underscores for everything except completion functions,
> so this is a change.
>
> Following a convention similar to the bash-completion project's
> proposed future convention doesn't really help compatibility.  If we
> want to be able to include this function in that project without
> change some day, we'd have to call it _BC_git_complete. :)

No, that's for bash-completion's functions, this is a git bash completion function.

And in any case, if they want something different they can change it themselves, and they could tell us.

But wasn't you the one that suggested we follow the bash-completion's guidelines, or that was only when the guidelines happened to match your preference?

There are basically four arguments that have been brought forward.

1) Namespace

You said there were two namespaces:

> _git_*  (completion functions)
>__git_* (everything else, including public interfaces like __git_ps1)

But that's not actually true, we have these:

  __gitfoo (__gitdir, __gitcomp_1, __gitcomp, __gitcomp_nl)
  __git_foo
  _git_foo
  _git, _gitk

And what is used for what is not exactly clear.

Currently the only function meant to be public is __git_ps1, but there's other __git_foo functions that are not meant to be public, so clearly there's no namespace for public functions. Everything is ad-hoc.

It could be assumed that anything that doesn't start with a _ is reserved for the user. Of course, there's no such guideline anywhere, so this would be a self-imposed limitation.

2) Guideline

You brought this argument forward, but it turns out the bash-completion guys have no actual guideline; they are still trying to decide what would be the naming for public functions.

If there's anything close to a guideline for bash-completion functions, it would be a _BC_ prefix. Our script thus would be using _GIT_ for its public functions. This would mean __git_ps1 should be renamed to _GIT_ps1.

But now it seems you want to separate from this guideline.

3) Conflicts

Another problem is that a user might already have a function named like that, which means 'git_complete' has higher chances of collision. But I wrote checks that would ensure this doesn't happen, still, nobody was interested in those checks.

It seems people are not interested in *real* conflicts, but rather theoretical namespace collisions.

4) Convenience

git_complete is nicer than _GIT_complete.

It seems to me the push-back away from 'git_complete' is mostly due to imaginary reasons, and now apparently specious reasons as well. So I guess there's no point in discussing; no amount of evidence is going to convince anybody to anything.

Cheers.

-- 
Felipe Contreras