[PATCH] git-completion: Add git help completion for aliases

[PATCH] git-completion: Add git help completion for aliases

[Jakob Pfender]

Enable bash completion for "git help <alias>", analogous to "git
<alias>", which was already implemented.

Signed-off-by: Jakob Pfender <jpfender@elegosoft.com>
---
  contrib/completion/git-completion.bash |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 893b771..f9a74d3 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1506,7 +1506,7 @@ _git_help ()
  		;;
  	esac
  	__git_compute_all_commands
-	__gitcomp "$__git_all_commands
+	__gitcomp "$__git_all_commands $(__git_aliases)
  		attributes cli core-tutorial cvs-migration
  		diffcore gitk glossary hooks ignore modules
  		repository-layout tutorial tutorial-2
-- 
1.7.0.4

Re: [PATCH] git-completion: Add git help completion for aliases

[Junio C Hamano]

Jakob Pfender <jpfender@elegosoft.com> writes:

> Enable bash completion for "git help <alias>", analogous to "git
> <alias>", which was already implemented.
>
> Signed-off-by: Jakob Pfender <jpfender@elegosoft.com>
> ---
>  contrib/completion/git-completion.bash |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 893b771..f9a74d3 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1506,7 +1506,7 @@ _git_help ()
>  		;;
>  	esac
>  	__git_compute_all_commands
> -	__gitcomp "$__git_all_commands
> +	__gitcomp "$__git_all_commands $(__git_aliases)
>  		attributes cli core-tutorial cvs-migration
>  		diffcore gitk glossary hooks ignore modules
>  		repository-layout tutorial tutorial-2

This patch may not be _wrong_ per-se, but it it useful in practice?

I dunno.

Don't people usually use aliases so that they do not have to type long
command names that would need completion?

Re: [PATCH] git-completion: Add git help completion for aliases

[Johannes Schindelin]

Hi,

On Mon, 21 Mar 2011, Junio C Hamano wrote:

> Jakob Pfender <jpfender@elegosoft.com> writes:
> 
> > Enable bash completion for "git help <alias>", analogous to "git
> > <alias>", which was already implemented.
> >
> > Signed-off-by: Jakob Pfender <jpfender@elegosoft.com>
> > ---
> >  contrib/completion/git-completion.bash |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/contrib/completion/git-completion.bash
> > b/contrib/completion/git-completion.bash
> > index 893b771..f9a74d3 100755
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -1506,7 +1506,7 @@ _git_help ()
> >  		;;
> >  	esac
> >  	__git_compute_all_commands
> > -	__gitcomp "$__git_all_commands
> > +	__gitcomp "$__git_all_commands $(__git_aliases)
> >  		attributes cli core-tutorial cvs-migration
> >  		diffcore gitk glossary hooks ignore modules
> >  		repository-layout tutorial tutorial-2
> 
> This patch may not be _wrong_ per-se, but it it useful in practice?
> 
> I dunno.
> 
> Don't people usually use aliases so that they do not have to type long
> command names that would need completion?

Yes. And if you use a lot of aliases, or a lot of machines with evolving 
aliases, the easiest way to inspect the alias is "git help <alias>". Not 
seeing it completed is an unexpected behavior.

Git is moving the right direction in terms of user-friendliness, e.g. by 
DWIMming "git checkout <remote branch>". Adding this completion would be 
a small step in the same direction, not more.

Ciao,
Johannes

Re: [PATCH] git-completion: Add git help completion for aliases

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > diff --git a/contrib/completion/git-completion.bash
>> > b/contrib/completion/git-completion.bash
>> > index 893b771..f9a74d3 100755
>> > --- a/contrib/completion/git-completion.bash
>> > +++ b/contrib/completion/git-completion.bash
>> > @@ -1506,7 +1506,7 @@ _git_help ()
>> >  		;;
>> >  	esac
>> >  	__git_compute_all_commands
>> > -	__gitcomp "$__git_all_commands
>> > +	__gitcomp "$__git_all_commands $(__git_aliases)
>> >  		attributes cli core-tutorial cvs-migration
>> >  		diffcore gitk glossary hooks ignore modules
>> >  		repository-layout tutorial tutorial-2
>> 
>> This patch may not be _wrong_ per-se, but it it useful in practice?
>> 
>> I dunno.
>> 
>> Don't people usually use aliases so that they do not have to type long
>> command names that would need completion?
>
> Yes. And if you use a lot of aliases, or a lot of machines with evolving 
> aliases, the easiest way to inspect the alias is "git help <alias>". Not 
> seeing it completed is an unexpected behavior.

I've queued the patch and I already said that this is not wrong, but I
think your argument cuts it both ways.

I have "co" aliased to "checkout", but with this change "git help co<TAB>"
now will interfere with my expectation that it would complete to commit,
config and the like, but exclude an alias that I myself defined and know
how to spell but forgot what its exact definition was, in order to avoid
cluttering the choices offered.

This is a constructive tangent but if we are going to run $(__git_aliases)
every time we run _git_help, perhaps it would want a hack similar to the
way the value for $__git_all_commands is generated just once?

Re: [PATCH] git-completion: Add git help completion for aliases

[SZEDER Gábor]

On Mon, Mar 21, 2011 at 02:48:10PM -0700, Junio C Hamano wrote:
> Jakob Pfender <jpfender@elegosoft.com> writes:
> 
> > Enable bash completion for "git help <alias>", analogous to "git
> > <alias>", which was already implemented.
> >
> > Signed-off-by: Jakob Pfender <jpfender@elegosoft.com>
> > ---
> >  contrib/completion/git-completion.bash |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/contrib/completion/git-completion.bash
> > b/contrib/completion/git-completion.bash
> > index 893b771..f9a74d3 100755
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -1506,7 +1506,7 @@ _git_help ()
> >  		;;
> >  	esac
> >  	__git_compute_all_commands
> > -	__gitcomp "$__git_all_commands
> > +	__gitcomp "$__git_all_commands $(__git_aliases)
> >  		attributes cli core-tutorial cvs-migration
> >  		diffcore gitk glossary hooks ignore modules
> >  		repository-layout tutorial tutorial-2
> 
> This patch may not be _wrong_ per-se, but it it useful in practice?
> 
> I dunno.

I think most people use 'git help <cmd>' to bring up the man page of
given git command.  Since my aliases don't have a man page (and I
doubt that any alias will have one), I would never run 'git help
<alias>' myself, because it would only lead to a 'man page not
available' error.  Well, at least so I thought, because I never knew
that 'git help <alias>' actually prints the definition of the alias.
It's definitely shorter and easier than 'git config --get
alias.<alias>', but on the other hand it clutters 'git help <TAB>
output, especially if you have a lot of aliases.  If I had aliases
like 'ci' or 'co' instead of 'commit' and 'checkout', then I would
definitely not want to see them after 'git help <TAB>'.

So I don't know either.

Sidenote: the completion for 'git help' already lists custom git
commands (i.e. a script named 'git-<cmd>' somewhere in $PATH'), and
those are likely not documented either.

> Don't people usually use aliases so that they do not have to type long
> command names that would need completion?

I mostly use aliases for a command and a couple of options (e.g. a
custom log format), and give them meaningful names.


Best,
Gábor

Re: [PATCH] git-completion: Add git help completion for aliases

[Johannes Schindelin]

Hi,

On Tue, 22 Mar 2011, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> > diff --git a/contrib/completion/git-completion.bash
> >> > b/contrib/completion/git-completion.bash
> >> > index 893b771..f9a74d3 100755
> >> > --- a/contrib/completion/git-completion.bash
> >> > +++ b/contrib/completion/git-completion.bash
> >> > @@ -1506,7 +1506,7 @@ _git_help ()
> >> >  		;;
> >> >  	esac
> >> >  	__git_compute_all_commands
> >> > -	__gitcomp "$__git_all_commands
> >> > +	__gitcomp "$__git_all_commands $(__git_aliases)
> >> >  		attributes cli core-tutorial cvs-migration
> >> >  		diffcore gitk glossary hooks ignore modules
> >> >  		repository-layout tutorial tutorial-2
> >> 
> >> This patch may not be _wrong_ per-se, but it it useful in practice?
> >> 
> >> I dunno.
> >> 
> >> Don't people usually use aliases so that they do not have to type long
> >> command names that would need completion?
> >
> > Yes. And if you use a lot of aliases, or a lot of machines with evolving 
> > aliases, the easiest way to inspect the alias is "git help <alias>". Not 
> > seeing it completed is an unexpected behavior.
> 
> I've queued the patch and I already said that this is not wrong, but I
> think your argument cuts it both ways.
> 
> I have "co" aliased to "checkout", but with this change "git help 
> co<TAB>" now will interfere with my expectation that it would complete 
> to commit, config and the like, but exclude an alias that I myself 
> defined and know how to spell but forgot what its exact definition was, 
> in order to avoid cluttering the choices offered.

In a corporate environment, one would now do a market research as to what 
is the common expectation.

> This is a constructive tangent but if we are going to run 
> $(__git_aliases) every time we run _git_help, perhaps it would want a 
> hack similar to the way the value for $__git_all_commands is generated 
> just once?

Indeed.

Ciao,
Johannes

Re: [PATCH] git-completion: Add git help completion for aliases

[SZEDER Gábor]

On Tue, Mar 22, 2011 at 12:53:43AM -0700, Junio C Hamano wrote:
> This is a constructive tangent but if we are going to run $(__git_aliases)
> every time we run _git_help, perhaps it would want a hack similar to the
> way the value for $__git_all_commands is generated just once?

I think this is not necessary.  We already run __git_aliases() every
time after 'git <TAB>', and it was not an issue so far.  And indeed, I
just created 50 aliases, and the time required for __git_aliases()
seems to be negligible:

  $ time __git_aliases
  <bunch of aliases>

  real    0m0.028s
  user    0m0.016s
  sys     0m0.004s

Besides, invoking __git_aliases() every time has the benefit that
newly added aliases will appear immediately in completion suggestions.


Best,
Gábor

Re: [PATCH] git-completion: Add git help completion for aliases

[Erik Faye-Lund]

2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
> On Tue, Mar 22, 2011 at 12:53:43AM -0700, Junio C Hamano wrote:
>> This is a constructive tangent but if we are going to run $(__git_aliases)
>> every time we run _git_help, perhaps it would want a hack similar to the
>> way the value for $__git_all_commands is generated just once?
>
> I think this is not necessary.  We already run __git_aliases() every
> time after 'git <TAB>', and it was not an issue so far.  And indeed, I
> just created 50 aliases, and the time required for __git_aliases()
> seems to be negligible:
>
>  $ time __git_aliases
>  <bunch of aliases>
>
>  real    0m0.028s
>  user    0m0.016s
>  sys     0m0.004s
>

Unfortunately, the situation is not quite so good on Windows:
$ time __git_aliases
<bunch of aliases>

real    0m0.112s
user    0m0.030s
sys     0m0.015s

This is with 50 aliases, with 0 aliases I get this:
$ time __git_aliases
test

real    0m0.063s
user    0m0.015s
sys     0m0.015s

Re: [PATCH] git-completion: Add git help completion for aliases

[Erik Faye-Lund]

2011/3/22 Erik Faye-Lund <kusmabite@gmail.com>:
> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
>> On Tue, Mar 22, 2011 at 12:53:43AM -0700, Junio C Hamano wrote:
>>> This is a constructive tangent but if we are going to run $(__git_aliases)
>>> every time we run _git_help, perhaps it would want a hack similar to the
>>> way the value for $__git_all_commands is generated just once?
>>
>> I think this is not necessary.  We already run __git_aliases() every
>> time after 'git <TAB>', and it was not an issue so far.  And indeed, I
>> just created 50 aliases, and the time required for __git_aliases()
>> seems to be negligible:
>>
>>  $ time __git_aliases
>>  <bunch of aliases>
>>
>>  real    0m0.028s
>>  user    0m0.016s
>>  sys     0m0.004s
>>
>
> Unfortunately, the situation is not quite so good on Windows:
> $ time __git_aliases
> <bunch of aliases>
>
> real    0m0.112s
> user    0m0.030s
> sys     0m0.015s
>
> This is with 50 aliases, with 0 aliases I get this:
> $ time __git_aliases
> test
>
> real    0m0.063s
> user    0m0.015s
> sys     0m0.015s
>

OK, apparently I have a dummy alias around from some time earlier, so
this was for 51 and 1 alias. But the point remains, it's actually a
bit of a problem on Windows.

Re: [PATCH] git-completion: Add git help completion for aliases

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 21.03.2011 22:48:
> Jakob Pfender <jpfender@elegosoft.com> writes:
> 
>> Enable bash completion for "git help <alias>", analogous to "git
>> <alias>", which was already implemented.
>>
>> Signed-off-by: Jakob Pfender <jpfender@elegosoft.com>
>> ---
>>  contrib/completion/git-completion.bash |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash
>> b/contrib/completion/git-completion.bash
>> index 893b771..f9a74d3 100755
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -1506,7 +1506,7 @@ _git_help ()
>>  		;;
>>  	esac
>>  	__git_compute_all_commands
>> -	__gitcomp "$__git_all_commands
>> +	__gitcomp "$__git_all_commands $(__git_aliases)
>>  		attributes cli core-tutorial cvs-migration
>>  		diffcore gitk glossary hooks ignore modules
>>  		repository-layout tutorial tutorial-2
> 
> This patch may not be _wrong_ per-se, but it it useful in practice?
> 
> I dunno.
> 
> Don't people usually use aliases so that they do not have to type long
> command names that would need completion?

I'd say this would be a perfect argument for "do not suggest aliases for
'git <cmd>'"; independent of that, we could still suggest aliases for
'git help <cmd>' since, presumably, you use commands more often than you
look up help...

But currently, we do it the other way round, and even worse:

git for<TAB>
for-each-ref-do   format-patch

git help for<TAB>
for-each-ref   format-patch

That is, git command completion lists aliases (for-each-ref-do) and
common commands, and git help completion lists no aliases and all commands.

The patch makes it so that the suggestions for git command are a subset
of those for git help command, which is a good thing. (Not completing
aliases for 'git <cmd>' would be good also, on top of that.)

And yes, I use git help <alias> a lot, because I don't like git config
--get alias.<alias>, and our config files tend to grow in ugliness over
time. (We need git config --broomstick or git config --vacuum.)

Michael

Re: [PATCH] git-completion: Add git help completion for aliases

[SZEDER Gábor]

On Tue, Mar 22, 2011 at 10:16:16AM +0100, Erik Faye-Lund wrote:
> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
> > On Tue, Mar 22, 2011 at 12:53:43AM -0700, Junio C Hamano wrote:
> >> This is a constructive tangent but if we are going to run $(__git_aliases)
> >> every time we run _git_help, perhaps it would want a hack similar to the
> >> way the value for $__git_all_commands is generated just once?
> >
> > I think this is not necessary.  We already run __git_aliases() every
> > time after 'git <TAB>', and it was not an issue so far.  And indeed, I
> > just created 50 aliases, and the time required for __git_aliases()
> > seems to be negligible:
> >
> >  $ time __git_aliases
> >  <bunch of aliases>
> >
> >  real    0m0.028s
> >  user    0m0.016s
> >  sys     0m0.004s
> >
> 
> Unfortunately, the situation is not quite so good on Windows:
> $ time __git_aliases
> <bunch of aliases>
> 
> real    0m0.112s
> user    0m0.030s
> sys     0m0.015s
> 
> This is with 50 aliases, with 0 aliases I get this:
> $ time __git_aliases
> test
> 
> real    0m0.063s
> user    0m0.015s
> sys     0m0.015s

I see.  However, on Windows everything git-related tends to be slow,
so this is nothing new.

The question is whether the slowness of a known slow platform would
justify the regression on all platforms.


Best,
Gábor

Re: [PATCH] git-completion: Add git help completion for aliases

[Erik Faye-Lund]

2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
> On Tue, Mar 22, 2011 at 10:16:16AM +0100, Erik Faye-Lund wrote:
>> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
>> > On Tue, Mar 22, 2011 at 12:53:43AM -0700, Junio C Hamano wrote:
>> >> This is a constructive tangent but if we are going to run $(__git_aliases)
>> >> every time we run _git_help, perhaps it would want a hack similar to the
>> >> way the value for $__git_all_commands is generated just once?
>> >
>> > I think this is not necessary.  We already run __git_aliases() every
>> > time after 'git <TAB>', and it was not an issue so far.  And indeed, I
>> > just created 50 aliases, and the time required for __git_aliases()
>> > seems to be negligible:
>> >
>> >  $ time __git_aliases
>> >  <bunch of aliases>
>> >
>> >  real    0m0.028s
>> >  user    0m0.016s
>> >  sys     0m0.004s
>> >
>>
>> Unfortunately, the situation is not quite so good on Windows:
>> $ time __git_aliases
>> <bunch of aliases>
>>
>> real    0m0.112s
>> user    0m0.030s
>> sys     0m0.015s
>>
>> This is with 50 aliases, with 0 aliases I get this:
>> $ time __git_aliases
>> test
>>
>> real    0m0.063s
>> user    0m0.015s
>> sys     0m0.015s
>
> I see.  However, on Windows everything git-related tends to be slow,
> so this is nothing new.

That's not the case. Every thing Git-related isn't slow on Windows,
but there are some things in Git that is.

> The question is whether the slowness of a known slow platform would
> justify the regression on all platforms.
>

Windows isn't slow. Get over this way of thinking, it's just wrong.
Windows has some different performance characteristics for some
operations than e.g Linux, but saying that it's slow is just wrong.
However, _Bash for Windows_ is quite slow, much due to Windows' lack
of fork(), which means that some very involved emulation needs to be
performed.

But even so, at least 25% of the git user base is on Windows,
according to the latest Git User Survey. That makes this stuff matter.

Re: [PATCH] git-completion: Add git help completion for aliases

[Michael J Gruber]

Erik Faye-Lund venit, vidit, dixit 22.03.2011 11:28:
> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
>> On Tue, Mar 22, 2011 at 10:16:16AM +0100, Erik Faye-Lund wrote:
>>> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
>>>> On Tue, Mar 22, 2011 at 12:53:43AM -0700, Junio C Hamano wrote:
>>>>> This is a constructive tangent but if we are going to run $(__git_aliases)
>>>>> every time we run _git_help, perhaps it would want a hack similar to the
>>>>> way the value for $__git_all_commands is generated just once?
>>>>
>>>> I think this is not necessary.  We already run __git_aliases() every
>>>> time after 'git <TAB>', and it was not an issue so far.  And indeed, I
>>>> just created 50 aliases, and the time required for __git_aliases()
>>>> seems to be negligible:
>>>>
>>>>  $ time __git_aliases
>>>>  <bunch of aliases>
>>>>
>>>>  real    0m0.028s
>>>>  user    0m0.016s
>>>>  sys     0m0.004s
>>>>
>>>
>>> Unfortunately, the situation is not quite so good on Windows:
>>> $ time __git_aliases
>>> <bunch of aliases>
>>>
>>> real    0m0.112s
>>> user    0m0.030s
>>> sys     0m0.015s
>>>
>>> This is with 50 aliases, with 0 aliases I get this:
>>> $ time __git_aliases
>>> test
>>>
>>> real    0m0.063s
>>> user    0m0.015s
>>> sys     0m0.015s
>>
>> I see.  However, on Windows everything git-related tends to be slow,
>> so this is nothing new.
> 
> That's not the case. Every thing Git-related isn't slow on Windows,
> but there are some things in Git that is.
> 
>> The question is whether the slowness of a known slow platform would
>> justify the regression on all platforms.
>>
> 
> Windows isn't slow. Get over this way of thinking, it's just wrong.
> Windows has some different performance characteristics for some
> operations than e.g Linux, but saying that it's slow is just wrong.
> However, _Bash for Windows_ is quite slow, much due to Windows' lack
> of fork(), which means that some very involved emulation needs to be
> performed.
> 
> But even so, at least 25% of the git user base is on Windows,
> according to the latest Git User Survey. That makes this stuff matter.

[Disclaimer: No, I don't like Win nor MS, but:]

Exactly, and the attitude which you criticise is a self-fulling prophecy
if one follows it. We may break (or make things worse for) Git on
Windows accidentally (because the percentage among developers is low),
but should never do so knowingly. And we may even try and watch out for
things which may be troublesome for Git on Win, and consult with our
brave Git on Win developers.

Michael

Re: [PATCH] git-completion: Add git help completion for aliases

[Junio C Hamano]

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

> I think this is not necessary.  We already run __git_aliases() every
> time after 'git <TAB>', and it was not an issue so far.

If we allow that line of reasoning, the cached $__git_all_commands that
can go stale needs to be removed, no?  I think folks running bash on
windows wanted it and it would be helping them.

One valid looking argument against caching aliases but still caching
commands is that the former could change on the end user's whim while the
latter is more or less stable across releases, but people can throw their
own "git-frotz" in their path and run it as "git frotz", so that is not
entirely a valid argument, either.

Re: [PATCH] git-completion: Add git help completion for aliases

[SZEDER Gábor]

On Tue, Mar 22, 2011 at 11:28:01AM +0100, Erik Faye-Lund wrote:
> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
> > On Tue, Mar 22, 2011 at 10:16:16AM +0100, Erik Faye-Lund wrote:
> >> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
> >> > On Tue, Mar 22, 2011 at 12:53:43AM -0700, Junio C Hamano wrote:
> >> >> This is a constructive tangent but if we are going to run $(__git_aliases)
> >> >> every time we run _git_help, perhaps it would want a hack similar to the
> >> >> way the value for $__git_all_commands is generated just once?
> >> >
> >> > I think this is not necessary.  We already run __git_aliases() every
> >> > time after 'git <TAB>', and it was not an issue so far.  And indeed, I
> >> > just created 50 aliases, and the time required for __git_aliases()
> >> > seems to be negligible:
> >> >
> >> >  $ time __git_aliases
> >> >  <bunch of aliases>
> >> >
> >> >  real    0m0.028s
> >> >  user    0m0.016s
> >> >  sys     0m0.004s
> >> >
> >>
> >> Unfortunately, the situation is not quite so good on Windows:
> >> $ time __git_aliases
> >> <bunch of aliases>
> >>
> >> real    0m0.112s
> >> user    0m0.030s
> >> sys     0m0.015s
> >>
> >> This is with 50 aliases, with 0 aliases I get this:
> >> $ time __git_aliases
> >> test
> >>
> >> real    0m0.063s
> >> user    0m0.015s
> >> sys     0m0.015s
> >
> > I see.  However, on Windows everything git-related tends to be slow,
> > so this is nothing new.
> 
> That's not the case. Every thing Git-related isn't slow on Windows,
> but there are some things in Git that is.
> 
> > The question is whether the slowness of a known slow platform would
> > justify the regression on all platforms.
> >
> 
> Windows isn't slow. Get over this way of thinking, it's just wrong.
> Windows has some different performance characteristics for some
> operations than e.g Linux, but saying that it's slow is just wrong.
> However, _Bash for Windows_ is quite slow, much due to Windows' lack
> of fork(), which means that some very involved emulation needs to be
> performed.

I meant the above only in the context of git, based on my last -- it
seems outdated -- experiences with msysgit about (more than?) a year
ago, when I found such builtins like commit and checkout to be quite
noticeably slow.  I'm glad to hear that things are changing for good.

Anyway, with the following silly "benchmarking" on top of the
thread-starter patch[*] ...


diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d2b8746..12aa613 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2742,7 +2742,7 @@ _git ()
 	fi
 
 	local completion_func="_git_${command//-/_}"
-	declare -f $completion_func >/dev/null && $completion_func && return
+	declare -f $completion_func >/dev/null && time $completion_func && return
 
 	local expansion=$(__git_aliased_command "$command")
 	if [ -n "$expansion" ]; then


... the time required for 'git help <TAB>' with msysgit is around
120-140ms.

With the caching of aliases for 'git help' ...


diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 12aa613..466578b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -963,6 +963,12 @@ __git_aliases ()
 	done
 }
 
+__git_aliases=
+__git_compute_aliases ()
+{
+	: ${__git_aliases:=$(__git_aliases)}
+}
+
 # __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
@@ -1508,7 +1514,8 @@ _git_help ()
 		;;
 	esac
 	__git_compute_all_commands
-	__gitcomp "$__git_all_commands $(__git_aliases)
+	__git_compute_aliases
+	__gitcomp "$__git_all_commands $__git_aliases
 		attributes cli core-tutorial cvs-migration
 		diffcore gitk glossary hooks ignore modules
 		repository-layout tutorial tutorial-2

... it goes down to around 50ms.

Looking at the numbers it seems to be a significant improvement, but
in practice I'm not sure I noticed anything; perhaps it's more
noticeable on slower hardware.  Just for the record, the numbers on
linux are ~50ms without and ~40ms with alias caching.

> But even so, at least 25% of the git user base is on Windows,
> according to the latest Git User Survey. That makes this stuff matter.

The other point of view is that it will cause a regression without a
compensating benefit for maybe more than 75% of the user base ;)


Best,
Gábor


[*] - off topic: Was it just me, or that simple patch really couldn't
be applied with a simple 'git am' on today's master?  What exactly
caused the conflict?  I looked quite hard, but couldn't find
anything...

Re: [PATCH] git-completion: Add git help completion for aliases

[SZEDER Gábor]

On Tue, Mar 22, 2011 at 10:25:58AM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > I think this is not necessary.  We already run __git_aliases() every
> > time after 'git <TAB>', and it was not an issue so far.
> 
> If we allow that line of reasoning, the cached $__git_all_commands that
> can go stale needs to be removed, no?  I think folks running bash on
> windows wanted it and it would be helping them.

Well, Shawn wrote that it was your request ;)

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

But there was indeed a remark shortly before that about bad fork-exec
performance under Cygwin on an other topic.

> One valid looking argument against caching aliases but still caching
> commands is that the former could change on the end user's whim while the
> latter is more or less stable across releases,

Agreed.

> but people can throw their
> own "git-frotz" in their path and run it as "git frotz"

Also agreed, and been annoyed by it not being recognized
automatically.  In fact I've even tried to make the completion script
detect user's commands on the fly but still without that huge loop
over the 'git help -a' output in __git_list_porcelain_commands(), but
couldn't come up with anything acceptable so far.  And simply
re-sourcing the completion script is not all that difficult,  which
will also take care of refreshing cached aliases, so...

> so that is not
> entirely a valid argument, either.

agreed here, too.


Best,
Gábor

Re: [PATCH] git-completion: Add git help completion for aliases

[Erik Faye-Lund]

2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
> On Tue, Mar 22, 2011 at 11:28:01AM +0100, Erik Faye-Lund wrote:
>> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
>> > The question is whether the slowness of a known slow platform would
>> > justify the regression on all platforms.
>> >
>>
>> Windows isn't slow. Get over this way of thinking, it's just wrong.
>> Windows has some different performance characteristics for some
>> operations than e.g Linux, but saying that it's slow is just wrong.
>> However, _Bash for Windows_ is quite slow, much due to Windows' lack
>> of fork(), which means that some very involved emulation needs to be
>> performed.
>
> I meant the above only in the context of git, based on my last -- it
> seems outdated -- experiences with msysgit about (more than?) a year
> ago, when I found such builtins like commit and checkout to be quite
> noticeably slow.  I'm glad to hear that things are changing for good.

Actually, the situation isn't all that good, but it's mostly
git-completion.bash's fault:

$ time __git_ps1 "%s"
build-vbcc
real    0m0.162s
user    0m0.045s
sys     0m0.000s

150+ ms just to assemble what branch you're on does very much makes
things feel laggy. This affects even when yo just press enter.

This is especially annoying when just doing the meat of it directly is
way cheaper:
$ time git branch --no-color
* master

real    0m0.036s
user    0m0.016s
sys     0m0.000s

git commit in itself isn't that bad either:
$ time git commit --allow-empty -m.
[master 8d8d75e] .

real    0m0.051s
user    0m0.000s
sys     0m0.015s

50 ms for something that actually does something isn't that bad. And
if it adds a non-empty commit, it's still OK:
$ echo "foo" > test.txt
$ git add test.txt
warning: LF will be replaced by CRLF in test.txt.
The file will have its original line endings in your working directory.
$ time git commit -m "bar"
[master 641c50e] bar
warning: LF will be replaced by CRLF in test.txt.
The file will have its original line endings in your working directory.
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 test.txt

real    0m0.065s
user    0m0.015s
sys     0m0.015s

>> But even so, at least 25% of the git user base is on Windows,
>> according to the latest Git User Survey. That makes this stuff matter.
>
> The other point of view is that it will cause a regression without a
> compensating benefit for maybe more than 75% of the user base ;)
>

I don't think a minor regression like not getting bash-completion
automatically updated (something that only affects interactive use
after adding a new alias, and probably also could be fixed by adding a
/tmp/git-completion.dirty.shell-pid file or something) should justify
degrading performance for an as large portion of users as the Windows
users are.

(Warning: speculation ahead)
Also, I suspect the numbers of Windows developers are
underrepresented, for two reasons;
- I suspect that Windows developers are less likely to be subscribed
to the mailing lists where the Git User Survey are promoted.
- I suspect that most Windows users use EITHER Cygwin OR MinGW, but I
intentionally picked the largest of the Windows categories instead of
adding them up because the Git User Survey allowed people to answer
both MinGW and Cygwin.

User Survey people, if your read this: please have a separate
operating system question next year. It's very valuable information!

Re: [PATCH] git-completion: Add git help completion for aliases

[Jakub Narebski]

Erik Faye-Lund <kusmabite@gmail.com> writes:
> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
> > On Tue, Mar 22, 2011 at 11:28:01AM +0100, Erik Faye-Lund wrote:

> > > But even so, at least 25% of the git user base is on Windows,
> > > according to the latest Git User Survey. That makes this stuff matter.
> >
> > The other point of view is that it will cause a regression without a
> > compensating benefit for maybe more than 75% of the user base ;)

[...]
> (Warning: speculation ahead)
> Also, I suspect the numbers of Windows developers are
> underrepresented, for two reasons;
> - I suspect that Windows developers are less likely to be subscribed
> to the mailing lists where the Git User Survey are promoted.
> - I suspect that most Windows users use EITHER Cygwin OR MinGW, but I
> intentionally picked the largest of the Windows categories instead of
> adding them up because the Git User Survey allowed people to answer
> both MinGW and Cygwin.
> 
> User Survey people, if your read this: please have a separate
> operating system question next year. It's very valuable information!

This information can be extracted from survey data, so there is no
need for separate question.

Prior to adding this information to GitSurvey2010 page on git wiki,

  https://git.wiki.kernel.org/index.php/GitSurvey2010#09._On_which_operating_system.28s.29_do_you_use_Git.3F
  https://git.wiki.kernel.org/index.php?title=GitSurvey2010&diff=14112&oldid=12212

you could get this information by going to 'Analysis' page

  http://tinyurl.com/GitSurvey2010Analysis

selecting last filter: 'F13 - OS: MS Windows (any)', writing down
number of responses: 2671, and calculating percentage relative to
number of all responses to Q9, i.e. 8541, and you get 31.27% of
people using Git on MS Windows (not counting 'other, please specify'
responses, but they are very few).

Or you could have extracted this information from responses data,
which is available on GitSurvey2010 page...

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] git-completion: Add git help completion for aliases

[Erik Faye-Lund]

On Wed, Mar 23, 2011 at 1:11 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>> 2011/3/22 SZEDER Gábor <szeder@ira.uka.de>:
>> > On Tue, Mar 22, 2011 at 11:28:01AM +0100, Erik Faye-Lund wrote:
>
>> > > But even so, at least 25% of the git user base is on Windows,
>> > > according to the latest Git User Survey. That makes this stuff matter.
>> >
>> > The other point of view is that it will cause a regression without a
>> > compensating benefit for maybe more than 75% of the user base ;)
>
> [...]
>> (Warning: speculation ahead)
>> Also, I suspect the numbers of Windows developers are
>> underrepresented, for two reasons;
>> - I suspect that Windows developers are less likely to be subscribed
>> to the mailing lists where the Git User Survey are promoted.
>> - I suspect that most Windows users use EITHER Cygwin OR MinGW, but I
>> intentionally picked the largest of the Windows categories instead of
>> adding them up because the Git User Survey allowed people to answer
>> both MinGW and Cygwin.
>>
>> User Survey people, if your read this: please have a separate
>> operating system question next year. It's very valuable information!
>
> This information can be extracted from survey data, so there is no
> need for separate question.
>
> Prior to adding this information to GitSurvey2010 page on git wiki,
>
>  https://git.wiki.kernel.org/index.php/GitSurvey2010#09._On_which_operating_system.28s.29_do_you_use_Git.3F
>  https://git.wiki.kernel.org/index.php?title=GitSurvey2010&diff=14112&oldid=12212
>
> you could get this information by going to 'Analysis' page
>
>  http://tinyurl.com/GitSurvey2010Analysis
>
> selecting last filter: 'F13 - OS: MS Windows (any)', writing down
> number of responses: 2671, and calculating percentage relative to
> number of all responses to Q9, i.e. 8541, and you get 31.27% of
> people using Git on MS Windows (not counting 'other, please specify'
> responses, but they are very few).
>
> Or you could have extracted this information from responses data,
> which is available on GitSurvey2010 page...
>

OK. Thanks for explaining this :)