git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder@ira.uka.de>
To: Peter van der Does <peter@avirtualhome.com>
Cc: "Shawn O. Pearce" <spearce@spearce.org>,
	git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
	Marc Branchaud <marcnarc@xiplink.com>,
	Brian Gernhardt <brian@gernhardtsoftware.com>,
	Kevin Ballard <kevin@sb.org>,
	Mathias Lafeldt <misfire@debugon.org>
Subject: Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
Date: Thu, 28 Oct 2010 00:39:18 +0200	[thread overview]
Message-ID: <20101027223918.GA1877@neumann> (raw)
In-Reply-To: <20101027131506.4da06c6d@MonteCarlo>

Hi,

On Wed, Oct 27, 2010 at 01:15:06PM -0400, Peter van der Does wrote:
> The completion script does not work as expected under Bash 4.

Thank you.  I applied your patch to play around a bit, and it seems to
work (but, of course, I have not tried all the affected commands).

I agree with the comments and suggestiong of Brian and Jonathan.  The
current commit message covers only the symptoms of a bug this patch
attempts to fix, but it should also explain its cause and how the
patch is supposed to fix it.  Unfortunately, I can only comment on the
latter: if these new functions are good enough for the folks over at
bash-completion project, then they should be good enough for us (;

I'm still puzzled that the only relevant entry I could find in the
bash NEWS file is:

i.  The programmable completion code now uses the same set of characters as
    readline when breaking the command line into a list of words.

Yet, as I mentioned in one of the previous threads, I have two
machines with different bash versions (3.2 and 4.1) but with the exact
same set of characters in COMP_WORDBREAKS, and they show different
behavior.

> Bash: 3
> output:
> $ git log --pretty=<tab><tab>
> email     full      medium    raw
> format:   fuller    oneline   short
> 
> Bash: 4
> output:
> $ git log --pretty=<tab><tab>
> .bash_logout         .local/
> .bash_profile        Music/
> --More--
> 
> Signed-off-by: Peter van der Does <peter@avirtualhome.com>


> @@ -556,11 +799,14 @@ __git_complete_revlist ()
>  
>  __git_complete_remote_or_refspec ()
>  {
> -	local cmd="${COMP_WORDS[1]}"
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	local git_comp_words git_comp_cword
> +	__reassemble_comp_words_by_ref : git_comp_words git_comp_cword

I suggest using '_get_comp_words_by_ref -n ":" words cword' here.
First, for consistency's sake, because you use
_get_comp_words_by_ref() everywhere else, too (almost).  Second, I'm
worried about the double underscore prefix in the function name,
because it usually indicates something that is not supposed to be used
directly from outside.  Even the bash-completion project's scripts
prefer _get_comp_words_by_ref() to __reassemble_comp_words_by_ref().
And third, ...

> +	local cmd="${git_comp_words[1]}"
> +	local cur
>  	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
> -	while [ $c -lt $COMP_CWORD ]; do
> -		i="${COMP_WORDS[c]}"
> +	_get_comp_words_by_ref -n ":" cur

... you could then join this _get_comp_words_by_ref() invocation with
the one above.

> +	while [ $c -lt $git_comp_cword ]; do
> +		i="${git_comp_words[c]}"
>  		case "$i" in
>  		--mirror) [ "$cmd" = "push" ] && no_complete_refspec=1 ;;
>  		--all)


> @@ -824,7 +1072,10 @@ __git_whitespacelist="nowarn warn error error-all fix"
>  
>  _git_am ()
>  {
> -	local cur="${COMP_WORDS[COMP_CWORD]}" dir="$(__gitdir)"
> +	local cur
> +	local dir="$(__gitdir)"

You could keep the two declarations in a single line.

> +
> +	_get_comp_words_by_ref -n "=" cur
>  	if [ -d "$dir"/rebase-apply ]; then
>  		__gitcomp "--skip --continue --resolved --abort"
>  		return


> @@ -929,7 +1183,8 @@ _git_bisect ()
>  
>  _git_branch ()
>  {
> -	local i c=1 only_local_ref="n" has_r="n"
> +	local i c=1 only_local_ref="n" has_r="n", cur

The comma before "cur" shouldn't be there.

> +	_get_comp_words_by_ref cur
>  
>  	while [ $c -lt $COMP_CWORD ]; do
>  		i="${COMP_WORDS[c]}"

Hmph.

I don't think there is anything that could go wrong here with respect
to word breaking, but at least for consistency's sake you could query
for words and cword, too, and use ${words[cword]} here.


> @@ -1006,7 +1262,8 @@ _git_cherry ()
>  
>  _git_cherry_pick ()
>  {
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	local cur
> +_get_comp_words_by_ref cur

Missing indentation.

>  	case "$cur" in
>  	--*)
>  		__gitcomp "--edit --no-commit"


> @@ -1606,9 +1886,11 @@ _git_stage ()
>  
>  __git_config_get_set_variables ()
>  {
> -	local prevword word config_file= c=$COMP_CWORD
> +	local prevword word config_file= c
> +	local git_comp_words
> +	__reassemble_comp_words_by_ref = git_comp_words c

Use _get_comp_words_by_ref() instead.

>  	while [ $c -gt 1 ]; do
> -		word="${COMP_WORDS[c]}"
> +		word="${git_comp_words[c]}"
>  		case "$word" in
>  		--global|--system|--file=*)
>  			config_file="$word"


> @@ -2045,7 +2327,8 @@ _git_reset ()
>  {
>  	__git_has_doubledash && return
>  
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	local cur
> +_get_comp_words_by_ref cur

Missing indentation.

>  	case "$cur" in
>  	--*)
>  		__gitcomp "--merge --mixed --hard --soft --patch"


> @@ -2343,15 +2634,16 @@ _git_whatchanged ()
>  
>  _git ()
>  {
> -	local i c=1 command __git_dir
> +	local i c=1 command __git_dir git_comp_words git_comp_cword
>  
>  	if [[ -n $ZSH_VERSION ]]; then
>  		emulate -L bash
>  		setopt KSH_TYPESET
>  	fi
>  
> -	while [ $c -lt $COMP_CWORD ]; do
> -		i="${COMP_WORDS[c]}"
> +	__reassemble_comp_words_by_ref = git_comp_words git_comp_cword

Use _get_comp_words_by_ref() instead.

> +	while [ $c -lt $git_comp_cword ]; do
> +		i="${git_comp_words[c]}"
>  		case "$i" in
>  		--git-dir=*) __git_dir="${i#--git-dir=}" ;;
>  		--bare)      __git_dir="." ;;

  parent reply	other threads:[~2010-10-27 22:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-27 17:15 [PATCH v2/RFC] Make git-completion Bash 4 compatible Peter van der Does
2010-10-27 17:23 ` Brian Gernhardt
2010-10-27 17:31 ` Jonathan Nieder
2010-10-27 22:53   ` SZEDER Gábor
2010-10-28  0:52     ` Peter van der Does
2010-10-28  0:54       ` Jonathan Nieder
2010-10-28 12:14         ` Peter van der Does
2010-10-28 16:15           ` Jakub Narebski
2010-10-28 18:46             ` Peter van der Does
2010-10-27 22:39 ` SZEDER Gábor [this message]
2010-10-28  0:48   ` Jonathan Nieder

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20101027223918.GA1877@neumann \
    --to=szeder@ira.uka.de \
    --cc=brian@gernhardtsoftware.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=kevin@sb.org \
    --cc=marcnarc@xiplink.com \
    --cc=misfire@debugon.org \
    --cc=peter@avirtualhome.com \
    --cc=spearce@spearce.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).