public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Felipe Contreras <felipe.contreras@gmail•com>
Cc: git@vger•kernel.org, "Ramkumar Ramachandra" <artagnon@gmail•com>,
	"SZEDER Gábor" <szeder@ira•uka.de>
Subject: Re: [PATCH 5/5] completion: fix completion of certain aliases
Date: Wed, 09 Apr 2014 12:33:35 -0700	[thread overview]
Message-ID: <xmqqa9bu1enk.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1397069404-7451-6-git-send-email-felipe.contreras@gmail.com> (Felipe Contreras's message of "Wed, 9 Apr 2014 13:50:04 -0500")

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

> Some commands need the first word to determine the actual action that is
> being executed, however, the command is wrong when we use an alias, for
> example 'alias.p=push', if we try to complete 'git p origin ', the
> result would be wrong because __git_complete_remote_or_refspec() doesn't
> know where it come from.
>
> So let's override words[1], so the alias 'p' is override by the actual
> command, 'push'.
>
> Reported-by: Aymeric Beaumet <aymeric.beaumet@gmail•com>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail•com>
> ---

Does "some commands" above refer to anything that uses
__git_complete_remote_or_refspec, or is the set of commands larger
than that?

I am wondering if it is safer to introduce a new "local" variable
that is set by the caller of __git_complete_remote_or_refspec and
inspected by __git_complete_remote_or_refspec (instead of words[1])
to communiate the real name of the git subcommand being completed,
without touching words[] in place.

That way, we wouldn't have to worry about all the other references
of words[c], words[i], words[CURRENT] etc. in the script seeing the
word that the end-user did not type and did not actually appear on
the command line.

But perhaps we muck with the contents of words[] in a similar way in
many different places in the existing completion code often enough
that such an attempt not to touch the words[] array does not buy us
much safety anyway.  I didn't check (and that is why I am asking
with "I am wondering...").

Thanks, will queue.

[Ram and Szeder CC'ed as they appear in shortlog for the past 12
months].

>  contrib/completion/git-completion.bash | 1 +
>  contrib/completion/git-completion.zsh  | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 9525343..893ae5d 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2547,6 +2547,7 @@ __git_main ()
>  
>  	local expansion=$(__git_aliased_command "$command")
>  	if [ -n "$expansion" ]; then
> +		words[1]=$expansion
>  		completion_func="_git_${expansion//-/_}"
>  		declare -f $completion_func >/dev/null && $completion_func
>  	fi
> diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
> index 6b77968..9f6f0fa 100644
> --- a/contrib/completion/git-completion.zsh
> +++ b/contrib/completion/git-completion.zsh
> @@ -104,6 +104,7 @@ __git_zsh_bash_func ()
>  
>  	local expansion=$(__git_aliased_command "$command")
>  	if [ -n "$expansion" ]; then
> +		words[1]=$expansion
>  		completion_func="_git_${expansion//-/_}"
>  		declare -f $completion_func >/dev/null && $completion_func
>  	fi

next prev parent reply	other threads:[~2014-04-09 19:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-09 18:49 [PATCH 0/5] Fixes Felipe Contreras
2014-04-09 18:50 ` [PATCH 1/5] remote-helpers: allow all tests running from any dir Felipe Contreras
2014-04-09 18:50 ` [PATCH 2/5] remote-hg: always normalize paths Felipe Contreras
2014-04-09 18:50 ` [PATCH 3/5] remote-bzr: add support for older versions Felipe Contreras
2014-04-09 18:50 ` [PATCH 4/5] remote-bzr: include authors field in pushed commits Felipe Contreras
2014-04-09 18:50 ` [PATCH 5/5] completion: fix completion of certain aliases Felipe Contreras
2014-04-09 19:33   ` Junio C Hamano [this message]
2014-04-09 20:36     ` Felipe Contreras
  -- strict thread matches above, loose matches on Subject: below --
2014-04-13  7:08 Gábor Szeder
2014-04-14 20:20 ` Junio C Hamano
2014-04-19  1:26   ` 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=xmqqa9bu1enk.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=artagnon@gmail$(echo .)com \
    --cc=felipe.contreras@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=szeder@ira$(echo .)uka.de \
    /path/to/YOUR_REPLY

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox