v2completion: add new git_complete helper

This simplifies the completions, and makes it easier to define aliases:

 git_complete gf git_fetch

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Since v2:

 * Remove stuff related to aliases fixes; should work on top of master

 contrib/completion/git-completion.bash |   68 +++++++++++++++-----------------
 t/t9902-completion.sh                  |    2 +-
 2 files changed, 32 insertions(+), 38 deletions(-)

Felipe Contreras wrote:

Sounds neat. Unfortunately users could already be using a function with some other purpose named git_complete in their .profile and this would override it. The completion script has so far stuck to a limited namespace:

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

A name like __git_complete should work, presumably.

Hope that helps, Jonathan

On Mon, Apr 16, 2012 at 12:37 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Perhaps it's time to avoid the __ prefix for public interfaces; otherwise how would people know they are public?

Felipe Contreras <felipe.contreras@gmail.com> writes:

We could do the same as __git_ps1, no? People know to use it already; they know it is public.

I am OK with introducing git_ps1 while keeping __git_ps1 as an equivalent and declare that git_$anything will be the surface interface for end users to *use* the machinery we provide, though. Then git_complete could be introduced without __git_complete equivalent. Probably _git_$name has to stay an implementation detail (i.e. the users can use it but it is their responsibility to update their using script when the implementation changes).

Junio C Hamano wrote:

The problem is that completion scripts are often included implicitly in .profile through the bash completion machinery. Users do not have to explicitly ask for them, so there is no reason for users to expect that the function name "more_cowbell" is dangerous to use in .profile because some day the completion script for the "more" command will start using it.

Because of these considerations, the convention is that every identifier provided by a completion script, including public ones, starts with an underscore.

Jonathan Nieder <jrnieder@gmail.com> writes:

OK.

On Mon, Apr 16, 2012 at 7:07 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Sure, that's why we would have _git_cowbell instead, so for *most* of functions the user would have no trouble, it would *only* be git_completion the one without prefix, and I think the name is quite safe. There's quite likely few, probably nobody, actually using that function name in their profiles.

Also, the script would now be loaded only *after*, the user types 'git <TAB>'. And there's already a bunch of functions that are already exported by just having bash completion: have(), quote(), dequote(), quote_readline(). And there's at least one script that uses a function without a prefix: ri_get_methods()

I would like to see a completion script that actually has a function supposed to be exported and that still uses the _ prefix anyway.

Cheers.

Felipe Contreras wrote:

The /etc/bash_completion library itself exports lots and lots of functions with a _ prefix.

On Mon, Apr 16, 2012 at 11:09 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

We are not making a bash_completion library; I mean a bash completion script (other than the library).

Felipe Contreras wrote:

Ok. If you refuse to put two and two together, then I will (as usual when this happens) just be a little passive aggressive and annoyed and let you talk to other people.

Cheers, Jonathan

On Mon, Apr 16, 2012 at 11:33 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Just to be clear; we are making a completion script for the 'git' command, so if we are looking for guidance on what other 'foo' command completion scripts do, the bash completion library itself is not good guidance.

My gut feeling is that there's no completion script that exports functions meant to be used directly by the user, so there's really no guidance for this.

Now, even if you use the bash completion library, it still does export functions without a prefix, so why take the ones with prefix as a rule, and ignore the other ones? It seems like there's no real guideline for what gets a prefix and what doesn't, even in the bash completion library itself, which I don't think should be used a guideline anyway.

The closest relative of 'git_complete' would be 'complete', both of course are meant for public usage. I have not seen any other functions that are similar.

Cheers.

Felipe Contreras wrote:

Are you sure? "complete" is a bash builtin and has nothing to do with the bash completion library except that the latter uses it.

On Mon, Apr 16, 2012 at 11:46 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

I already provided examples: have(), quote(), dequote(), quote_readline()

(cc-ing the bash-completion-devel list) Felipe Contreras wrote:

Ah, that's what you mean. Thanks for the pointers, and sorry to have misunderstood.

There's a little oddity here. "have" is clearly an unwanted backward compatibility feature:

	# @deprecated should no longer be used; generally not needed with dynamically
	#             loaded completions, and _have is suitable for runtime use.
	have()
	[...]
	unset -f have
	unset have

But "quote", "dequote", and "quote_readline" do not get the same treatment. Perhaps they are for backward compatibility, too, but are so widely used that there is no hope of ever getting rid of them.

Hopefully this information helps clarify to what extent the leading underscores in functions exposed by completion scripts are meant or are not meant as a convention.

Jonathan

_______________________________________________ Bash-completion-devel mailing list Bash-completion-devel@lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/bash-completion-devel

On 2012-04-16 23:59, Jonathan Nieder wrote:

We've discussed what a real "API" or "namespace" of bash-completion would look like, but so far nothing concrete has come out of it.

http://thread.gmane.org/gmane.comp.shells.bash.completion.scm/2013/focus=3135

On Sat, Apr 21, 2012 at 10:20 AM, Ville Skyttä <ville.skytta@iki.fi> wrote:

Thank you for pointing this out. This means I was correct; there was/is no convention for public APIs.

According to that thread, the closest there is to a convention would be to name it _GIT_complete. That would certainly avoid conflicts with any current namespace, so I feel it's much better than __git_complete.

Still, I don't see the point in avoiding 'git_complete' and making our lifes more difficult. Bash public functions, like *complete*, don't have any special namespace, they just snatch them, and that's the end of it. In the particular case of git, where would have only a couple (currently 2) public functions, I don't see what's the big deal.

Cheers.

On 2012-04-21 18:41, Felipe Contreras wrote:

I'm not aware of ways it'd make people's lifes more difficult, but if git_complete is a function intended for completion purposes, I'd personally not name it git* because it'd interfere for example with completing git<TAB> by being included in the suggested completions even though it's never meant to be used that way. Prefixing for example with underscore doesn't avoid the problem completely, but makes it less likely to happen.

2012/4/21 Ville Skyttä <ville.skytta@iki.fi>:

That's actually a good point, I forgot that functions are also completed.

Cheers.

Jonathan Nieder <jrnieder@gmail.com> writes:

I have been assuming that bash_completion library is a sort of metaproject whose sole purpose is to supply a framework for completion scripts for other packages like Git to be plugged easily and give niceties like lazy loading to avoid inflicting undue latencies to the end users.

Earlier I said "Ok" to you based on that assumption; somebody may have his own git_complete that rebuilds a test integration branch (i.e. completes it) and having the name git_complete exported by us will indirectly affect her fingers if she installs bash_completion and Git package on her system.

But that assumption may not be correct, or even if it is correct, Felipe may be missing that part, so it may not be "refusing to add two and two", but could be "learned two, but do not know the number of the other side of addition".

On Mon, Apr 16, 2012 at 11:51 PM, Junio C Hamano <gitster@pobox.com> wrote:

She might have had the same problem if she named it _git_complete. That might be unlinkely, but I also find it unlikely that anybody would have a git_complete function. In fact, I find it unlikely that people would write shell functions like that; it's much easier to write scripts and put them in $PATH.

Even more, I just added a foobar() function in my profile, and I also added a foobar() function in /etc/bash_completion.d/git. I don't see my function replaced in any way, even after typing and completing 'git' commands. I don't know how that's possible, but that's why I don't like to take these types of claims as face value.

Cheers.

Felipe Contreras wrote:

What version of the bash_completion library do you use? Lazy-loading was introduced in version 1.90.

If you put

	foobar() {
		echo hi
	}
	. /etc/bash_completion

then is your private foobar unclobbered?

To answer your demand before for a function in the public interface of a completion script, not library, which respects or does not respect the bash-completion convention: see the public _rpm_installed_packages helper from the rpm completion script[1].

If you don't like the convention or think that I have misunderstood it, I can understand that and would recommend that you suggest a different one and get it adopted and documented by the bash_completion project.

Jonathan

[1] http://lists.alioth.debian.org/pipermail/bash-completion-devel/2012-March/004359.html

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

1.99

Yes, in that case it does, but that's not the default behavior, at least not in my system, and I doubt anybody would define their functions before loading library scripts.

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.

The fact that somebody uses it doesn't mean it's public.

Cheers.

Felipe Contreras wrote:

Is your git completion script installed at /etc/bash_completion.d/git, /usr/share/bash-completion/completions/git, or somewhere else?

Felipe Contreras wrote:

Ok, you win. I hadn't realized we were having a debate, but now I do, and you have won.

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

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, though I used to do that for a long time and Debian's default .bashrc loads /etc/bash_completion at the end, too.

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

Jonathan

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

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.

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.

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

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.

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 wrote:

No, honestly it isn't my responsibility to waste time arguing with you. What kind of crazy world would work that way?

And I did not mean to bring up any argument. I only meant to bring up the _datum_ that, at least in the context of the bash_completion project, that is the current convention. And then you started trying to tell me that I had the facts wrong! You might even be right, but you haven't shown any sign of trying to check that, by, say, asking someone from the bash_completion project what convention they use.

I'm sick of this. Call it whatever you want. I don't know why you think this is productive.

Still annoyed, Jonathan

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

I don't understand. I'm proposing the name 'git_complete', I thought you were arguing against it. If you were only providing random facts, then we can just ignore them, and it would be OK, right? But I'm pretty sure you would be angry as well if I just ignored that fact.

Sure, it would be nice to follow bash_completion project's convention for these kinds of functions, if they had any, and might be useful to ask them what they think. But we don't *have* to. And nobody is arguing that we should ask them. Right? Or are you?

We don't have to agree on any name right now. I believe it's worth waiting before deciding on a name for a public function like this, and try to get some consensus.

But throwing arguments on the air, and they get angry when they get counter-argued is not helpful. If we are going to discuss, lets discuss, but that doesn't seem to be what you want. You want me to blindly use the name you propose without saying a word? Blindly accept your argument... follow orders?

I took your argument seriously and looked for evidence myself, and in the absence of evidence I'm still not throwing it away, I'm assuming it has merit and added code to make sure we don't override some user function that has the same name. That would solve the problem you raised, and it would help us find out if this is indeed a real issue, or a theoretical one, and at the same time making sure people don't rely too much on this function for the moment.

I believe this is very scientific-like; you make a hypothesis, we make an experiment, and then we can find out the results, and only then make a decision.

I don't know what else I can do to move this forward.

Cheers.

Felipe Contreras wrote:

No, I really am not arguing that we should ask them. Because we (or at least I) already know at least what their convention was a few years ago. However, I do encourage anyone curious to ask them instead of making up arguments about why the answer is obvious.

[...]

I do not think it is fair to call your position blind. I really would be happier if you were able to listen, and, instead of throwing out protests and debating points, to talk about your real concerns.

I am guessing (but you never said!) that you find the name __git_complete ugly. That's fine. I even agree. I have mentioned that there is a namespace that your proposed alternative violates. Your response is... to refuse to believe that what I said is true or relevant?

How am I supposed to have a reasonable discussion after that?

This is not a debating floor. When I give what I think is relevant information, I am not requesting "please misinterpret me and shoot me down".

Ciao, Jonathan

Felipe Contreras wrote:

Ah.  All that talking, but I was not addressing your actual point.

I though you didn't believe that the namespace adopted by the bash_completion project was "identifiers starting with underscore, plus a few old identifiers that have been grandfathered in".

But in fact, you just don't like namespace conventions.

Now I'm exhausted so I have no time to explain why a project that involves implicitly including scripts from a wide variety of authors into a flat namespace shared with others might end up using a convention to manage that namespace. But at least the world seems a little saner. Sorry for the wasted time.

Please, next time just say what you mean instead of leaving me talking about something irrelevant.

Felipe Contreras wrote:

I'm happy as long as all the identifiers the script exports start with an underscore, as that is the usual convention for completion scripts. Within that namespace, if people want to use ___git or _git__ for private things or something, I don't mind.

If __git_ps1 is moved to a separate script library, it seems fine for it to provide a git_ps1 identifier with no leading underscores when it is not being implicitly included by the completion script.

Jonathan

Hi,

On Sun, Apr 15, 2012 at 04:37:18PM -0500, Jonathan Nieder wrote:

Don't forget the oddball __gitdir() function ;)

And foo_wrap() should also fit into those namespaces.

Best, Gábor

2012/4/17 SZEDER Gábor <szeder@ira.uka.de>:

Yeah, I don't have a problem with that, just forgot about it.

But git_complete I think is different.

Felipe Contreras <felipe.contreras@gmail.com> writes:

Is git_complete something the user types interactively, or is it meant to be used in their .bashrc to help them complete arguments to their custom scripts that take arguments similar to Git Porcelains?

On Tue, Apr 17, 2012 at 6:50 PM, Junio C Hamano <gitster@pobox.com> wrote:

It's meant for their .bashrc, but can be used interactively, exactly like 'complete'. You can type 'complete -o bashdefault -o default -o nospace -F _git git' in the command like, but that would rarely happen.

Felipe Contreras <felipe.contreras@gmail.com> writes:

OK, then I do not think "as a public interface it looks somewhat ugly" (which I happen to think, and I am guessing that you agree with) matters. It looks to me that it would be sane to follow the convention to avoid accidental name clashes with userspace names by naming it "__git_complete" in that case.

On Tue, Apr 17, 2012 at 9:15 PM, Junio C Hamano <gitster@pobox.com> wrote:

But that convention is for *private* functions.

What if there are no clashes?

Are you saying that even if there are no real clashes, only hypothetical ones, you would still prefer __git_complete?

How are people going to distinguish between public and private functions?

Felipe Contreras wrote:

There are only two public functions.

They will be the ones provided without an __ (git_ps1 and git_complete) when you source the git-prompt-and-completion-helpers library in your .bashrc. Neither will be exposed automatically from the completion script, though synonyms with leading __ to avoid namespace clashes will be.

Sensible? Jonathan

On Tue, Apr 17, 2012 at 11:53:08PM +0300, Felipe Contreras wrote:

Can you ascertain that there are no real clashes?

By reading the documentation, perhaps? It was not a problem for __git_ps1(), so I guess it won't be a problem for __git_complete() either. Assuming that it will be documented eventually, of course.

Best, Gábor

2012/4/18 SZEDER Gábor <szeder@ira.uka.de>:

After having this code for a certain amount of time, yeah:

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

I guess that would do it. I would still prefer to have a different convention for public functions. I feel we are being paranoid here.

Cheers.

On Tue, Apr 17, 2012 at 08:50:01AM -0700, Junio C Hamano wrote:

Primarily for .bashrc, however, not for their custom scripts but to be able to use completion for their one-word git aliases, e.g. after an

  alias gf='git fetch'

they want to do

  gf o<TAB>

and get 'origin' completed.

Here's a relevant thread:

  http://thread.gmane.org/gmane.comp.version-control.git/185184/

Best, Gábor