git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder@ira.uka.de>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, "Shawn O. Pearce" <spearce@spearce.org>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Thomas Rast <trast@student.ethz.ch>
Subject: Re: [PATCH 10/12] completion: add new git_complete helper
Date: Thu, 12 Apr 2012 00:50:46 +0200	[thread overview]
Message-ID: <20120411225046.GU2289@goldbirke> (raw)
In-Reply-To: <1333854479-23260-11-git-send-email-felipe.contreras@gmail.com>

Hi,


On Sun, Apr 08, 2012 at 06:07:57AM +0300, Felipe Contreras wrote:
> 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>
> ---
>  contrib/completion/git-completion.bash |   69 +++++++++++++++-----------------
>  t/t9902-completion.sh                  |    2 +-
>  2 files changed, 33 insertions(+), 38 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 60ea224..6cf1d98 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2616,21 +2616,6 @@ _git ()
>  {
>  	local i c=1 cmd cmd_pos __git_dir
>  
> -	if [[ -n ${ZSH_VERSION-} ]]; then
> -		emulate -L bash
> -		setopt KSH_TYPESET
> -
> -		# workaround zsh's bug that leaves 'words' as a special
> -		# variable in versions < 4.3.12
> -		typeset -h words
> -
> -		# workaround zsh's bug that quotes spaces in the COMPREPLY
> -		# array if IFS doesn't contain spaces.
> -		typeset -h IFS
> -	fi
> -
> -	local cur words cword prev
> -	_get_comp_words_by_ref -n =: cur words cword prev
>  	while [ $c -lt $cword ]; do
>  		i="${words[c]}"
>  		case "$i" in
> @@ -2683,22 +2668,6 @@ _git ()
>  
>  _gitk ()
>  {
> -	if [[ -n ${ZSH_VERSION-} ]]; then
> -		emulate -L bash
> -		setopt KSH_TYPESET
> -
> -		# workaround zsh's bug that leaves 'words' as a special
> -		# variable in versions < 4.3.12
> -		typeset -h words
> -
> -		# workaround zsh's bug that quotes spaces in the COMPREPLY
> -		# array if IFS doesn't contain spaces.
> -		typeset -h IFS
> -	fi
> -
> -	local cur words cword prev
> -	_get_comp_words_by_ref -n =: cur words cword prev
> -

Eliminating code duplication, great!

>  	__git_has_doubledash && return
>  
>  	local g="$(__gitdir)"
> @@ -2719,16 +2688,42 @@ _gitk ()
>  	__git_complete_revlist
>  }
>  
> -complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \
> -	|| complete -o default -o nospace -F _git git
> -complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \
> -	|| complete -o default -o nospace -F _gitk gitk

More greatness.

> +foo_wrap ()

This git-completion-specific internal helper function will pollute the
user's environment.  It should get a __git_ prefix.

> +{
> +	local cmd=foo_cmd cmd_pos=1
> +	if [[ -n ${ZSH_VERSION-} ]]; then
> +		emulate -L bash
> +		setopt KSH_TYPESET
> +
> +		# workaround zsh's bug that leaves 'words' as a special
> +		# variable in versions < 4.3.12
> +		typeset -h words
> +
> +		# workaround zsh's bug that quotes spaces in the COMPREPLY
> +		# array if IFS doesn't contain spaces.
> +		typeset -h IFS
> +	fi
> +	local cur words cword prev
> +	_get_comp_words_by_ref -n =: cur words cword prev
> +	foo "$@"
> +}
> +
> +git_complete ()

Maybe this should also get a prefix, but I'm not sure, because this
is, strictly speaking, not a bash completion function.

> +{
> +	local name="${2-$1}"
> +	local cmd="${name#git_}"
> +	eval "$(typeset -f foo_wrap | sed -e "s/foo_cmd/$cmd/" -e "s/foo/_$name/")"
> +	complete -o bashdefault -o default -o nospace -F _${name}_wrap $1 2>/dev/null \
> +		|| complete -o default -o nospace -F _${name}_wrap $1
> +}
> +
> +git_complete git
> +git_complete gitk

Clever ;)

But it needs a subshell and a sed invocation, so Windows people might
not be all too happy about this.  Perhaps it's not a big deal in the
default case, because it will be done only twice at startup time.
However, if someone wants several aliases on Windows, it can cause
noticeable delay.  It's a startup speed vs. convenience trade-off.
Anyway, I don't have any ideas how those fork()s and exec() could be
avoided.


Best,
Gábor

  reply	other threads:[~2012-04-11 22:51 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-08  3:07 [PATCH 00/12] Bash completion rework Felipe Contreras
2012-04-08  3:07 ` [PATCH 01/12] tests: add initial bash completion tests Felipe Contreras
2012-04-08  4:25   ` Junio C Hamano
2012-04-08  4:48     ` Jeff King
2012-04-08  5:41       ` Junio C Hamano
2012-04-08  5:42         ` Jeff King
2012-04-08  8:12           ` Jeff King
2012-04-08  9:07             ` Andreas Schwab
2012-04-08 11:04               ` Jeff King
2012-04-11 21:59     ` Felipe Contreras
2012-04-11 23:49       ` Junio C Hamano
2012-04-08  5:01   ` Jeff King
2012-04-08 10:30     ` Jonathan Nieder
2012-04-08 11:06       ` Jeff King
2012-04-09 19:58         ` Junio C Hamano
2012-04-08 10:28   ` John Keeping
2012-04-08  3:07 ` [PATCH 02/12] completion: simplify __gitcomp Felipe Contreras
2012-04-08 12:46   ` SZEDER Gábor
2012-04-08 13:07     ` Felipe Contreras
2012-04-08 13:27       ` Jonathan Nieder
2012-04-08 13:47         ` Felipe Contreras
2012-04-08 14:36           ` Jonathan Nieder
2012-04-08 14:58             ` Felipe Contreras
2012-04-09 18:57               ` Johannes Sixt
2012-04-09 19:12                 ` Felipe Contreras
2012-04-08 14:11         ` Felipe Contreras
2012-04-08 14:39           ` Jonathan Nieder
2012-04-09 18:22             ` Junio C Hamano
2012-04-08  3:07 ` [PATCH 03/12] completion: simplify __gitcomp_1 Felipe Contreras
2012-04-08  3:07 ` [PATCH 04/12] completion: trivial simplification Felipe Contreras
2012-04-08  3:07 ` [PATCH 05/12] completion: add missing global options Felipe Contreras
2012-04-08 10:22   ` John Keeping
2012-04-08 12:36   ` SZEDER Gábor
2012-04-08  3:07 ` [PATCH 06/12] tests: add more bash completion tests Felipe Contreras
2012-04-08  3:07 ` [PATCH 07/12] completion: simplify command stuff Felipe Contreras
2012-04-11 22:14   ` SZEDER Gábor
2012-04-11 22:21     ` Felipe Contreras
2012-04-11 23:01       ` SZEDER Gábor
2012-04-11 23:45         ` Felipe Contreras
2012-04-12 23:08           ` Felipe Contreras
2012-04-08  3:07 ` [PATCH 08/12] completion: simplify _git_bundle Felipe Contreras
2012-04-08  3:07 ` [PATCH 09/12] completion: calculate argument position properly Felipe Contreras
2012-04-08  3:07 ` [PATCH 10/12] completion: add new git_complete helper Felipe Contreras
2012-04-11 22:50   ` SZEDER Gábor [this message]
2012-04-11 23:44     ` Felipe Contreras
2012-04-08  3:07 ` [PATCH 11/12] test: add tests for aliases in bash completion Felipe Contreras
2012-04-08  3:20   ` Felipe Contreras
2012-04-08  3:07 ` [PATCH 12/12] completion: rename _git and _gitk Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120411225046.GU2289@goldbirke \
    --to=szeder@ira.uka.de \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=spearce@spearce.org \
    --cc=trast@student.ethz.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).