Re: [PATCH] bash: support user-supplied completion scripts for user's git commands

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 26, 2010, 20:04 UTC
Message-ID: <7vmxyvsqzn.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100226152710.GA17460@neumann>

SZEDER Gábor <szeder@ira.uka.de> writes:

>> so in that sense, the patch is not introducing a new problem,
>> but making it a documented interface and casting it in stone will make the
>> namespace contamination issue harder to rectify later.

I won't quote the first paragraph where you are repeating what I said, while sounding as if you were disagreeing with me.

Show 11 quoted lines

>> So if we were to ...
>> ... It is my understanding that the only published interface so
>> far is __git_ps1.
>
> I would say that __git_ps1() is the only interface that is advertised
> as being public.  ...
> ...  If we were to rename completion functions, these
> people's setup will break (although they will likely get merge
> conflicts caused by this patch anyway).  On the other hand: should we
> really care that much about such users, who use non-pulic interfaces
> from contrib/ ?

I see we are in agreement in the first half of your paragraph; my answer to the question in the latter half is:

 - we shouldn't care about people who already used unpublished interface
   in contrib/ so far; _but_

 - because we will be advertising it as a way to override and enhance
   completion to define your own shell functions, the naming _will_ become
   part of published interface---what we decide _now_ will matter.

That is why I wanted people to at least think about renaming _git_frotz to something less generic. The name tells us that it is a helper shell function about the "git frotz" command, but it does not say what aspect of "git frotz" it is meant to help, i.e. completion. _git_complete_frotz or a variant of such would not have that problem, and will keep the door open for future shell helpers that are about different aspect "xxx" that is unrelated to completion---they can then name theirs _git_xxx_frotz.

> ...  BTW, Mercurial's
> completion script uses _hg_cmd_foo() for hg commands and
> _hg_ext_bar() for extensions, so we might as well be a bit consistent,
> and call our completion functions _git_cmd_foo().

In Hg's context it might make sense to name a function _hg_cmd_foo vs _hg_ext_bar iff the end users need to be very aware of the distinction between commands and extensions, but for us I think "git_cmd_foo" is probably the most meaningless rename, as it doesn't add any extra information (we know 'git foo' is a command already without 'cmd').