[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH] bash: support user-supplied completion scripts for
From:       SZEDER =?iso-8859-1?Q?G=E1bor?= <szeder () ira ! uka ! de>
Date:       2010-01-31 19:19:36
Message-ID: 20100131191936.GA30466 () neumann
[Download RAW message or body]

On Fri, Jan 29, 2010 at 12:04:31PM -0800, Shawn O. Pearce wrote:
> SZEDER G?bor <szeder@ira.uka.de> wrote:
> > 
> > _git_lgm () {
> >         _git_log
> > }
> > 
> > Unfortunately, it doesn't work at all.
> > 
> > In _git() first we have 'lgm' in $command, which is ok, but then comes
> > this alias handling thing
> > 
> >         local expansion=$(__git_aliased_command "$command")
> >         [ "$expansion" ] && command="$expansion"
> > 
> > which writes '!sh' into $command, and that doesn't look quite right
> 
> __git_aliased_command is returning the first word out of the alias.

Actually, it returns the first word from the alias which does not
start with a dash.  It behaves this way since its introduction in
367dce2a (Bash completion support for aliases, 2006-10-28).  I'm not
sure what the original intent was behind ignoring words starting with
a dash, but it gave me some ideas.

> I think we need to change this block here to:
> 
>   case "$expansion" of
>   \!*) : leave command as alias ;;
>   '')  : leave command alone ;;
>   *)   command="$expansion" ;;
>   esac
> 
> Or something like that.  Because an alias whose value starts with
> ! is a shell command to be executed, so we want to use _git_$command
> for completion, but other aliases are builtin commands and we should
> use their first word token (what __git_aliased_command returns)
> as the name of the completion function.

After pondering about it for a while, I think that in this case the
real issue is not _git() not handling __git_aliased_command()'s return
value corretly, but rather __git_aliased_command() returning junk in
case of a more advanced alias.  And while fixing it up, we can also
improve on it to return the right command in some more cases, too.

Let's have an other look at Junio's alias:

    [alias]
        lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"

While it's clear that full parsing of something like that in the
completion code is unfeasible, we can easily get rid of stuff that is
definitely not a git command: !sh shell commands, options, and
environment variables.


diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 45a393f..faddbdf 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -625,10 +625,15 @@ __git_aliased_command ()
 	local word cmdline=$(git --git-dir="$(__gitdir)" \
 		config --get "alias.$1")
 	for word in $cmdline; do
-		if [ "${word##-*}" ]; then
-			echo $word
+		case "$word" in
+		\!*)	: shell command alias ;;
+		-*)	: option ;;
+		*=*)	: setting env ;;
+		git)	: git itself ;;
+		*)
+			echo "$word"
 			return
-		fi
+		esac
 	done
 }

 
and this way it would correctly return 'log' for Junio's 'lgm' alias.
With a bit tweaking we could also extend it to handle !gitk aliases,
too.

Of course, it isn't perfect either, and could be fooled easily.  It's
not hard to construct an alias, in which a word does not match any of
these filter patterns, but is still not a git command (e.g.  by
setting an environment variable to a value which contains spaces).  It
may even return false positives, when the output of a git command is
piped into an other git command, and the second gets the command line
options via $@, but the first command will be returned.  However, such
problematic cases could be handled by a custom completion function
provided by the user.

What do you think?


Best,
Gábor

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]