git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Custom git completion
@ 2010-01-29 12:57 David Rhodes Clymer
  2010-01-29 15:11 ` Shawn O. Pearce
  0 siblings, 1 reply; 25+ messages in thread
From: David Rhodes Clymer @ 2010-01-29 12:57 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

Unless I read it incorrectly, the completion script included with
git-core does not make it easy for users to write completion scripts
for custom git commands. I can extend git itself by creating a command
"git-foo", and placing it in my path. The command can then be used
like so: "git foo". However, if I want to add command completion for
that command without modifying (I may not have permission) or
duplicating the system git completion, I can't write a completion
script which matches works on "git foo", only "git-foo", which is not
how I would ever call the script.

Anyway, so I made a simple modification which looks for completion
code for custom commands, and calls that as appropriate. If the
attached patch (or something like it) were applied to the git
completion script, it would be awfully handy.

-davidc

ps. I'm not subscribed to the list, so please copy me on any replies, thanks!

[-- Attachment #2: git_completion.patch --]
[-- Type: text/x-patch, Size: 380 bytes --]

--- /etc/bash_completion.d/git.orig	2010-01-12 14:50:16.000000000 -0500
+++ /etc/bash_completion.d/git	2010-01-12 15:28:27.000000000 -0500
@@ -1403,7 +1403,10 @@
 	svn)         _git_svn ;;
 	tag)         _git_tag ;;
 	whatchanged) _git_log ;;
-	*)           COMPREPLY=() ;;
+	*)
+    COMPREPLY=()
+    $(complete -p |awk '/ '"git-${command}"'$/{print $(NF-1)}')
+  ;;
 	esac
 }
 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Custom git completion
  2010-01-29 12:57 Custom git completion David Rhodes Clymer
@ 2010-01-29 15:11 ` Shawn O. Pearce
  2010-01-29 17:42   ` Junio C Hamano
  2010-01-30 23:00   ` David Rhodes Clymer
  0 siblings, 2 replies; 25+ messages in thread
From: Shawn O. Pearce @ 2010-01-29 15:11 UTC (permalink / raw)
  To: David Rhodes Clymer; +Cc: git

David Rhodes Clymer <david@zettazebra.com> wrote:
> Unless I read it incorrectly, the completion script included with
> git-core does not make it easy for users to write completion scripts
> for custom git commands. I can extend git itself by creating a command
> "git-foo", and placing it in my path.

git config --global alias.foo /home/me/bin/my-git-foo

git foo will now complete correctly.  No need to modify the
completion code.

-- 
Shawn.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Custom git completion
  2010-01-29 15:11 ` Shawn O. Pearce
@ 2010-01-29 17:42   ` Junio C Hamano
  2010-01-29 17:59     ` Shawn O. Pearce
  2010-01-30 23:00   ` David Rhodes Clymer
  1 sibling, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2010-01-29 17:42 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: David Rhodes Clymer, git

"Shawn O. Pearce" <spearce@spearce.org> writes:

> David Rhodes Clymer <david@zettazebra.com> wrote:
>> Unless I read it incorrectly, the completion script included with
>> git-core does not make it easy for users to write completion scripts
>> for custom git commands. I can extend git itself by creating a command
>> "git-foo", and placing it in my path.
>
> git config --global alias.foo /home/me/bin/my-git-foo
>
> git foo will now complete correctly.  No need to modify the
> completion code.

Yes.  Aliases and custom subcommands are found from 'git help" output just
fine (you need to install new subcommand in exec-path).

But.

How does the completion code learn what options and arguments such aliases
and subcommands (e.g. "git foo") take without being told?

An alias that uses another git subcommand (i.e. the ones that do not start
with a bang "!") seems to be handled correctly, but one of my aliases is
this:

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

and the completion code doesn't (and it is unfair to expect it to) notice
that "git log" is run under the hood.  I cannot say "git lgm sp/<TAB>" and
choose from the list of topics from you.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Custom git completion
  2010-01-29 17:42   ` Junio C Hamano
@ 2010-01-29 17:59     ` Shawn O. Pearce
  2010-01-29 18:02       ` Junio C Hamano
  2010-01-30 23:03       ` Custom git completion David Rhodes Clymer
  0 siblings, 2 replies; 25+ messages in thread
From: Shawn O. Pearce @ 2010-01-29 17:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Rhodes Clymer, git

Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > David Rhodes Clymer <david@zettazebra.com> wrote:
> >> Unless I read it incorrectly, the completion script included with
> >> git-core does not make it easy for users to write completion scripts
> >> for custom git commands. I can extend git itself by creating a command
> >> "git-foo", and placing it in my path.
> >
> > git config --global alias.foo /home/me/bin/my-git-foo
> >
> > git foo will now complete correctly.  No need to modify the
> > completion code.
> 
> Yes.  Aliases and custom subcommands are found from 'git help" output just
> fine (you need to install new subcommand in exec-path).
> 
> But.
> 
> How does the completion code learn what options and arguments such aliases
> and subcommands (e.g. "git foo") take without being told?

Sure.  But the patch offered by the original poster also suffered
from this problem, it didn't know how to complete arguments for
the subcommand.

 
> An alias that uses another git subcommand (i.e. the ones that do not start
> with a bang "!") seems to be handled correctly, but one of my aliases is
> this:
> 
>     [alias]
> 	lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"

Doing this is difficult, because its hard to parse that string and
do completion on it.  On the other hand, we could do something like:

  [completion]
  	lgm = log

and have `git lgm` complete using the same rules as `git log`.
Its somewhat ugly though...
 
-- 
Shawn.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Custom git completion
  2010-01-29 17:59     ` Shawn O. Pearce
@ 2010-01-29 18:02       ` Junio C Hamano
  2010-01-29 19:06         ` [PATCH] bash: support user-supplied completion scripts for user's git commands SZEDER Gábor
  2010-01-30 23:03       ` Custom git completion David Rhodes Clymer
  1 sibling, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2010-01-29 18:02 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: David Rhodes Clymer, git

"Shawn O. Pearce" <spearce@spearce.org> writes:

>> An alias that uses another git subcommand (i.e. the ones that do not start
>> with a bang "!") seems to be handled correctly, but one of my aliases is
>> this:
>> 
>>     [alias]
>> 	lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"
>
> Doing this is difficult, because its hard to parse that string and
> do completion on it.

That is why I said it is unfair to expect completion code to do that.

>   [completion]
>   	lgm = log
>
> and have `git lgm` complete using the same rules as `git log`.

I actually like that.  It matches _my_ expectation as a user to be able to
say "this subcommand has args that look like those given to 'log'".

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] bash: support user-supplied completion scripts for user's git commands
  2010-01-29 18:02       ` Junio C Hamano
@ 2010-01-29 19:06         ` SZEDER Gábor
  2010-01-29 19:13           ` Shawn O. Pearce
                             ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: SZEDER Gábor @ 2010-01-29 19:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, David Rhodes Clymer, git

The bash completion script already provides support to complete
aliases, options and refs for aliases (if the alias can be traced back
to a supported git command by __git_aliased_command()), and the user's
custom git commands, but it does not support the options of the user's
custom git commands (of course; how could it know about the options of
a custom git command?).  Users of such custom git commands could
extend git's bash completion script by writing functions to support
their commands, but they might have issues with it: they might not
have the rights to modify a system-wide git completion script, and
they will need to track and merge upstream changes in the future.

This patch addresses this by providing means for users to supply
custom completion scriplets for their custom git commands without
modifying the main git bash completion script.

Instead of having a huge hard-coded list of command-completion
function pairs (in _git()), the completion script will figure out
which completion function to call based on the command's name.  That
is, when completing the options of 'git foo', the main completion
script will check whether the function '_git_foo' is declared, and if
declared, it will invoke that function to perform the completion.  If
such a function is not declared, it will fall back to complete file
names.  So, users will only need to provide this '_git_foo' completion
function in a separate file, source that file, and it will be used the
next time they press TAB after 'git foo '.

There are two git commands (stage and whatchanged), for which the
completion functions of other commands were used, therefore they
got their own completion function.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---

How about something like this for subcommands (not aliases)?  It's a
good code size reduction anyway.

 contrib/completion/git-completion.bash |   67 ++++++--------------------------
 1 files changed, 12 insertions(+), 55 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index da46bf8..2cecf4f 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1433,6 +1433,11 @@ _git_send_email ()
 	COMPREPLY=()
 }
 
+_git_stage ()
+{
+	_git_add
+}
+
 __git_config_get_set_variables ()
 {
 	local prevword word config_file= c=$COMP_CWORD
@@ -2164,6 +2169,11 @@ _git_tag ()
 	esac
 }
 
+_git_whatchanged ()
+{
+	_git_log
+}
+
 _git ()
 {
 	local i c=1 command __git_dir
@@ -2203,61 +2213,8 @@ _git ()
 	local expansion=$(__git_aliased_command "$command")
 	[ "$expansion" ] && command="$expansion"
 
-	case "$command" in
-	am)          _git_am ;;
-	add)         _git_add ;;
-	apply)       _git_apply ;;
-	archive)     _git_archive ;;
-	bisect)      _git_bisect ;;
-	bundle)      _git_bundle ;;
-	branch)      _git_branch ;;
-	checkout)    _git_checkout ;;
-	cherry)      _git_cherry ;;
-	cherry-pick) _git_cherry_pick ;;
-	clean)       _git_clean ;;
-	clone)       _git_clone ;;
-	commit)      _git_commit ;;
-	config)      _git_config ;;
-	describe)    _git_describe ;;
-	diff)        _git_diff ;;
-	difftool)    _git_difftool ;;
-	fetch)       _git_fetch ;;
-	format-patch) _git_format_patch ;;
-	fsck)        _git_fsck ;;
-	gc)          _git_gc ;;
-	grep)        _git_grep ;;
-	help)        _git_help ;;
-	init)        _git_init ;;
-	log)         _git_log ;;
-	ls-files)    _git_ls_files ;;
-	ls-remote)   _git_ls_remote ;;
-	ls-tree)     _git_ls_tree ;;
-	merge)       _git_merge;;
-	mergetool)   _git_mergetool;;
-	merge-base)  _git_merge_base ;;
-	mv)          _git_mv ;;
-	name-rev)    _git_name_rev ;;
-	notes)       _git_notes ;;
-	pull)        _git_pull ;;
-	push)        _git_push ;;
-	rebase)      _git_rebase ;;
-	remote)      _git_remote ;;
-	replace)     _git_replace ;;
-	reset)       _git_reset ;;
-	revert)      _git_revert ;;
-	rm)          _git_rm ;;
-	send-email)  _git_send_email ;;
-	shortlog)    _git_shortlog ;;
-	show)        _git_show ;;
-	show-branch) _git_show_branch ;;
-	stash)       _git_stash ;;
-	stage)       _git_add ;;
-	submodule)   _git_submodule ;;
-	svn)         _git_svn ;;
-	tag)         _git_tag ;;
-	whatchanged) _git_log ;;
-	*)           COMPREPLY=() ;;
-	esac
+	local completion_func="_git_${command//-/_}"
+	declare -F $completion_func >/dev/null && $completion_func
 }
 
 _gitk ()
-- 
1.7.0.rc0.78.g2070a

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] bash: support user-supplied completion scripts for user's git commands
  2010-01-29 19:06         ` [PATCH] bash: support user-supplied completion scripts for user's git commands SZEDER Gábor
@ 2010-01-29 19:13           ` Shawn O. Pearce
  2010-01-29 20:00             ` SZEDER Gábor
  2010-01-29 20:32           ` [PATCH] bash: support user-supplied completion scripts for user's git commands Junio C Hamano
  2010-01-30 23:34           ` David Rhodes Clymer
  2 siblings, 1 reply; 25+ messages in thread
From: Shawn O. Pearce @ 2010-01-29 19:13 UTC (permalink / raw)
  To: SZEDER G?bor; +Cc: Junio C Hamano, David Rhodes Clymer, git

SZEDER G?bor <szeder@ira.uka.de> wrote:
> The bash completion script already provides support to complete
> aliases, options and refs for aliases (if the alias can be traced back
> to a supported git command by __git_aliased_command()), and the user's
> custom git commands, but it does not support the options of the user's
> custom git commands (of course; how could it know about the options of
> a custom git command?).  Users of such custom git commands could
> extend git's bash completion script by writing functions to support
> their commands, but they might have issues with it: they might not
> have the rights to modify a system-wide git completion script, and
> they will need to track and merge upstream changes in the future.
> 
> This patch addresses this by providing means for users to supply
> custom completion scriplets for their custom git commands without
> modifying the main git bash completion script.
> 
> Instead of having a huge hard-coded list of command-completion
> function pairs (in _git()), the completion script will figure out
> which completion function to call based on the command's name.  That
> is, when completing the options of 'git foo', the main completion
> script will check whether the function '_git_foo' is declared, and if
> declared, it will invoke that function to perform the completion.  If
> such a function is not declared, it will fall back to complete file
> names.  So, users will only need to provide this '_git_foo' completion
> function in a separate file, source that file, and it will be used the
> next time they press TAB after 'git foo '.
> 
> There are two git commands (stage and whatchanged), for which the
> completion functions of other commands were used, therefore they
> got their own completion function.
> 
> Signed-off-by: SZEDER G?bor <szeder@ira.uka.de>
> ---
> 
> How about something like this for subcommands (not aliases)?  It's a
> good code size reduction anyway.

Hmm, I like this.  I just didn't know how to implement it...  :-)

Acked-by: Shawn O. Pearce <spearce@spearce.org>

> +	local completion_func="_git_${command//-/_}"
> +	declare -F $completion_func >/dev/null && $completion_func

Yay for knowing bash.  :-)

-- 
Shawn.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] bash: support user-supplied completion scripts for user's git commands
  2010-01-29 19:13           ` Shawn O. Pearce
@ 2010-01-29 20:00             ` SZEDER Gábor
  2010-01-29 20:04               ` Shawn O. Pearce
  0 siblings, 1 reply; 25+ messages in thread
From: SZEDER Gábor @ 2010-01-29 20:00 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, David Rhodes Clymer, git

Hi Shawn,

On Fri, Jan 29, 2010 at 11:13:26AM -0800, Shawn O. Pearce wrote:
> SZEDER G?bor <szeder@ira.uka.de> wrote:
> > How about something like this for subcommands (not aliases)?  It's a
> > good code size reduction anyway.
> 
> Hmm, I like this.  I just didn't know how to implement it...  :-)
> 
> Acked-by: Shawn O. Pearce <spearce@spearce.org>
> 
> > +	local completion_func="_git_${command//-/_}"
> > +	declare -F $completion_func >/dev/null && $completion_func
> 
> Yay for knowing bash.  :-)

Heh.  I've found out about this 'declare -F' thing about two hours ago
(;


However.

I thought this should actually "Just Work" for aliases, too.  e.g.
Junio could use the following completion function to get 'git log's
options for his lgm alias:

_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
for me, although I admit that I can't seem to figure out how this
__git_aliased_command() is supposed to work (so much about knowing
bash ;).  Any insight?


Best,
Gábor

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] bash: support user-supplied completion scripts for user's git commands
  2010-01-29 20:00             ` SZEDER Gábor
@ 2010-01-29 20:04               ` Shawn O. Pearce
  2010-01-31 19:19                 ` SZEDER Gábor
  0 siblings, 1 reply; 25+ messages in thread
From: Shawn O. Pearce @ 2010-01-29 20:04 UTC (permalink / raw)
  To: SZEDER G?bor; +Cc: Junio C Hamano, David Rhodes Clymer, git

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.
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.

I think.  :-)

-- 
Shawn.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] bash: support user-supplied completion scripts for user's git commands
  2010-01-29 19:06         ` [PATCH] bash: support user-supplied completion scripts for user's git commands SZEDER Gábor
  2010-01-29 19:13           ` Shawn O. Pearce
@ 2010-01-29 20:32           ` Junio C Hamano
  2010-02-26 15:27             ` SZEDER Gábor
  2010-01-30 23:34           ` David Rhodes Clymer
  2 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2010-01-29 20:32 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Shawn O. Pearce, David Rhodes Clymer, git

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

> Instead of having a huge hard-coded list of command-completion
> function pairs (in _git()), the completion script will figure out
> which completion function to call based on the command's name.  That
> is, when completing the options of 'git foo', the main completion
> script will check whether the function '_git_foo' is declared, and if
> declared, it will invoke that function to perform the completion.  If
> such a function is not declared, it will fall back to complete file
> names.  So, users will only need to provide this '_git_foo' completion
> function in a separate file, source that file, and it will be used the
> next time they press TAB after 'git foo '.

I think the basic idea is sound, but I have a minor issue with the names.

Admittedly, we have already taken over _git_foo (and "_git") namespace,
and anybody who uses bash with the completion support cannot write their
own shell function with these names for purposes that are unrelated to
completion, so in that sense, the patch is not introducing a new problem,
but making it a documented interface and casting it in stone will make the
namespace contamination issue harder to rectify later.

So if we were to go in the direction as the patch proposes (which I think
is a good idea), we might want to rename them to __git_completion_foo or
something that is less likely to collide with whatever names users might
want to use.  It is my understanding that the only published interface so
far is __git_ps1.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Custom git completion
  2010-01-29 15:11 ` Shawn O. Pearce
  2010-01-29 17:42   ` Junio C Hamano
@ 2010-01-30 23:00   ` David Rhodes Clymer
  1 sibling, 0 replies; 25+ messages in thread
From: David Rhodes Clymer @ 2010-01-30 23:00 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

On Fri, Jan 29, 2010 at 10:11 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> David Rhodes Clymer <david@zettazebra.com> wrote:
>> Unless I read it incorrectly, the completion script included with
>> git-core does not make it easy for users to write completion scripts
>> for custom git commands. I can extend git itself by creating a command
>> "git-foo", and placing it in my path.
>
> git config --global alias.foo /home/me/bin/my-git-foo
>
> git foo will now complete correctly.  No need to modify the
> completion code.

This doesn't do what I want. This workaround only allows the command
name itself to be completed. I want my _custom_ completion code  to be
used for my custom command.

-davidc

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Custom git completion
  2010-01-29 17:59     ` Shawn O. Pearce
  2010-01-29 18:02       ` Junio C Hamano
@ 2010-01-30 23:03       ` David Rhodes Clymer
  1 sibling, 0 replies; 25+ messages in thread
From: David Rhodes Clymer @ 2010-01-30 23:03 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git

On Fri, Jan 29, 2010 at 12:59 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
>>
>> How does the completion code learn what options and arguments such aliases
>> and subcommands (e.g. "git foo") take without being told?
>
> Sure.  But the patch offered by the original poster also suffered
> from this problem, it didn't know how to complete arguments for
> the subcommand.
>

My patch allows custom completion code to be called if available. I
don't expect git completion to know anything about my new command.

-davidc

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] bash: support user-supplied completion scripts for user's  git commands
  2010-01-29 19:06         ` [PATCH] bash: support user-supplied completion scripts for user's git commands SZEDER Gábor
  2010-01-29 19:13           ` Shawn O. Pearce
  2010-01-29 20:32           ` [PATCH] bash: support user-supplied completion scripts for user's git commands Junio C Hamano
@ 2010-01-30 23:34           ` David Rhodes Clymer
  2 siblings, 0 replies; 25+ messages in thread
From: David Rhodes Clymer @ 2010-01-30 23:34 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Junio C Hamano, Shawn O. Pearce, git

2010/1/29 SZEDER Gábor <szeder@ira.uka.de>:
> The bash completion script already provides support to complete
> aliases, options and refs for aliases (if the alias can be traced back
> to a supported git command by __git_aliased_command()), and the user's
> custom git commands, but it does not support the options of the user's
> custom git commands (of course; how could it know about the options of
> a custom git command?).  Users of such custom git commands could
> extend git's bash completion script by writing functions to support
> their commands, but they might have issues with it: they might not
> have the rights to modify a system-wide git completion script, and
> they will need to track and merge upstream changes in the future.
>
> This patch addresses this by providing means for users to supply
> custom completion scriplets for their custom git commands without
> modifying the main git bash completion script.
>
> Instead of having a huge hard-coded list of command-completion
> function pairs (in _git()), the completion script will figure out
> which completion function to call based on the command's name.  That
> is, when completing the options of 'git foo', the main completion
> script will check whether the function '_git_foo' is declared, and if
> declared, it will invoke that function to perform the completion.  If
> such a function is not declared, it will fall back to complete file
> names.  So, users will only need to provide this '_git_foo' completion
> function in a separate file, source that file, and it will be used the
> next time they press TAB after 'git foo '.
>
> There are two git commands (stage and whatchanged), for which the
> completion functions of other commands were used, therefore they
> got their own completion function.
>

Excellent! This looks just like what I was after. Among other things,
this is much better than my use of awk. ;o)

-davidc

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] bash: support user-supplied completion scripts for user's git commands
  2010-01-29 20:04               ` Shawn O. Pearce
@ 2010-01-31 19:19                 ` SZEDER Gábor
  2010-02-23 21:02                   ` [PATCH 0/4] bash: support user-supplied completion scripts for custom git commands and aliases SZEDER Gábor
  0 siblings, 1 reply; 25+ messages in thread
From: SZEDER Gábor @ 2010-01-31 19:19 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, David Rhodes Clymer, git

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 0/4] bash: support user-supplied completion scripts for custom git commands and aliases
  2010-01-31 19:19                 ` SZEDER Gábor
@ 2010-02-23 21:02                   ` SZEDER Gábor
  2010-02-23 21:02                     ` [PATCH 1/4] bash: improve aliased command recognition SZEDER Gábor
                                       ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: SZEDER Gábor @ 2010-02-23 21:02 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, David Rhodes Clymer, Teemu Matilainen, git,
	SZEDER Gábor

Hi,

here is the full series, extended for aliases.

I didn't want to push an obviously post-v1.7.0 change during the -rc
period, and then forgot about it until Teemu (on CC) sent similar
patches today[*].  His two patches do basically the same as my 2/4 (with
minor differences).

Junio was concerned about possible namespace issues.  This series does
not addresses his concern, but I have some thoughts about it, and I will
try to discuss it after dinner.

Best,
Gábor

[*] gmane: http://thread.gmane.org/gmane.comp.version-control.git/140804
    message id: <1266936193-10644-1-git-send-email-teemu.matilainen@iki.fi>


SZEDER Gábor (4):
  bash: improve aliased command recognition
  bash: support user-supplied completion scripts for user's git
    commands
  bash: support user-supplied completion scripts for aliases
  bash: completion for gitk aliases

 contrib/completion/git-completion.bash |   94 ++++++++++++--------------------
 1 files changed, 34 insertions(+), 60 deletions(-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/4] bash: improve aliased command recognition
  2010-02-23 21:02                   ` [PATCH 0/4] bash: support user-supplied completion scripts for custom git commands and aliases SZEDER Gábor
@ 2010-02-23 21:02                     ` SZEDER Gábor
  2010-02-23 22:11                       ` Junio C Hamano
  2010-02-23 21:02                     ` [PATCH 2/4] bash: support user-supplied completion scripts for user's git commands SZEDER Gábor
                                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 25+ messages in thread
From: SZEDER Gábor @ 2010-02-23 21:02 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, David Rhodes Clymer, Teemu Matilainen, git,
	SZEDER Gábor

To support completion for aliases, the completion script tries to
figure out which git command is invoked by an alias.  Its
implementation in __git_aliased_command() is rather straightforward:
it returns the first word from the alias.  For simple aliases starting
with the git command (e.g. alias.last = cat-file commit HEAD) this
gives the right results.  Unfortunately, it does not work with shell
command aliases, which can get rather complex, as illustrated by one
of Junio's aliases:

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

In this case the current implementation returns "!sh" as the aliased
git command, which is obviosly wrong.

The full parsing of a shell command alias like that in the completion
code is clearly unfeasible.  However, we can easily improve on aliased
command recognition by eleminating stuff that is definitely not a git
command: shell commands (anything starting with '!'), command line
options (anything starting with '-'), environment variables (anything
with a '=' in it), and git itself.  This way the above alias would be
handled correctly, and the completion script would correctly recognize
"log" as the aliased git command.

Of course, this solution is not 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 options for the first one are
offered.  However, the following patches will enable the user to
supply custom completion scripts for aliases, which can be used to
remedy these problematic cases.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fe93747..78c4983 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
 }
 
-- 
1.7.0.119.g9b76

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/4] bash: support user-supplied completion scripts for user's git commands
  2010-02-23 21:02                   ` [PATCH 0/4] bash: support user-supplied completion scripts for custom git commands and aliases SZEDER Gábor
  2010-02-23 21:02                     ` [PATCH 1/4] bash: improve aliased command recognition SZEDER Gábor
@ 2010-02-23 21:02                     ` SZEDER Gábor
  2010-02-23 21:02                     ` [PATCH 3/4] bash: support user-supplied completion scripts for aliases SZEDER Gábor
  2010-02-23 21:03                     ` [PATCH 4/4] bash: completion for gitk aliases SZEDER Gábor
  3 siblings, 0 replies; 25+ messages in thread
From: SZEDER Gábor @ 2010-02-23 21:02 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, David Rhodes Clymer, Teemu Matilainen, git,
	SZEDER Gábor

The bash completion script already provides support to complete
aliases, options and refs for aliases (if the alias can be traced back
to a supported git command by __git_aliased_command()), and the user's
custom git commands, but it does not support the options of the user's
custom git commands (of course; how could it know about the options of
a custom git command?).  Users of such custom git commands could
extend git's bash completion script by writing functions to support
their commands, but they might have issues with it: they might not
have the rights to modify a system-wide git completion script, and
they will need to track and merge upstream changes in the future.

This patch addresses this by providing means for users to supply
custom completion scriplets for their custom git commands without
modifying the main git bash completion script.

Instead of having a huge hard-coded list of command-completion
function pairs (in _git()), the completion script will figure out
which completion function to call based on the command's name.  That
is, when completing the options of 'git foo', the main completion
script will check whether the function '_git_foo' is declared, and if
declared, it will invoke that function to perform the completion.  If
such a function is not declared, it will fall back to complete file
names.  So, users will only need to provide this '_git_foo' completion
function in a separate file, source that file, and it will be used the
next time they press TAB after 'git foo '.

There are two git commands (stage and whatchanged), for which the
completion functions of other commands were used, therefore they
got their own completion function.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   67 ++++++--------------------------
 1 files changed, 12 insertions(+), 55 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 78c4983..2ac3567 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1439,6 +1439,11 @@ _git_send_email ()
 	COMPREPLY=()
 }
 
+_git_stage ()
+{
+	_git_add
+}
+
 __git_config_get_set_variables ()
 {
 	local prevword word config_file= c=$COMP_CWORD
@@ -2170,6 +2175,11 @@ _git_tag ()
 	esac
 }
 
+_git_whatchanged ()
+{
+	_git_log
+}
+
 _git ()
 {
 	local i c=1 command __git_dir
@@ -2209,61 +2219,8 @@ _git ()
 	local expansion=$(__git_aliased_command "$command")
 	[ "$expansion" ] && command="$expansion"
 
-	case "$command" in
-	am)          _git_am ;;
-	add)         _git_add ;;
-	apply)       _git_apply ;;
-	archive)     _git_archive ;;
-	bisect)      _git_bisect ;;
-	bundle)      _git_bundle ;;
-	branch)      _git_branch ;;
-	checkout)    _git_checkout ;;
-	cherry)      _git_cherry ;;
-	cherry-pick) _git_cherry_pick ;;
-	clean)       _git_clean ;;
-	clone)       _git_clone ;;
-	commit)      _git_commit ;;
-	config)      _git_config ;;
-	describe)    _git_describe ;;
-	diff)        _git_diff ;;
-	difftool)    _git_difftool ;;
-	fetch)       _git_fetch ;;
-	format-patch) _git_format_patch ;;
-	fsck)        _git_fsck ;;
-	gc)          _git_gc ;;
-	grep)        _git_grep ;;
-	help)        _git_help ;;
-	init)        _git_init ;;
-	log)         _git_log ;;
-	ls-files)    _git_ls_files ;;
-	ls-remote)   _git_ls_remote ;;
-	ls-tree)     _git_ls_tree ;;
-	merge)       _git_merge;;
-	mergetool)   _git_mergetool;;
-	merge-base)  _git_merge_base ;;
-	mv)          _git_mv ;;
-	name-rev)    _git_name_rev ;;
-	notes)       _git_notes ;;
-	pull)        _git_pull ;;
-	push)        _git_push ;;
-	rebase)      _git_rebase ;;
-	remote)      _git_remote ;;
-	replace)     _git_replace ;;
-	reset)       _git_reset ;;
-	revert)      _git_revert ;;
-	rm)          _git_rm ;;
-	send-email)  _git_send_email ;;
-	shortlog)    _git_shortlog ;;
-	show)        _git_show ;;
-	show-branch) _git_show_branch ;;
-	stash)       _git_stash ;;
-	stage)       _git_add ;;
-	submodule)   _git_submodule ;;
-	svn)         _git_svn ;;
-	tag)         _git_tag ;;
-	whatchanged) _git_log ;;
-	*)           COMPREPLY=() ;;
-	esac
+	local completion_func="_git_${command//-/_}"
+	declare -F $completion_func >/dev/null && $completion_func
 }
 
 _gitk ()
-- 
1.7.0.119.g9b76

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/4] bash: support user-supplied completion scripts for aliases
  2010-02-23 21:02                   ` [PATCH 0/4] bash: support user-supplied completion scripts for custom git commands and aliases SZEDER Gábor
  2010-02-23 21:02                     ` [PATCH 1/4] bash: improve aliased command recognition SZEDER Gábor
  2010-02-23 21:02                     ` [PATCH 2/4] bash: support user-supplied completion scripts for user's git commands SZEDER Gábor
@ 2010-02-23 21:02                     ` SZEDER Gábor
  2010-02-23 21:03                     ` [PATCH 4/4] bash: completion for gitk aliases SZEDER Gábor
  3 siblings, 0 replies; 25+ messages in thread
From: SZEDER Gábor @ 2010-02-23 21:02 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, David Rhodes Clymer, Teemu Matilainen, git,
	SZEDER Gábor

Shell command aliases can get rather complex, and the completion
script can not always determine correctly the git command invoked by
such an alias.  For such cases users might want to provide custom
completion scripts the same way like for their custom commands made
possible by the previous patch.

The current completion script does not allow this, because if it
encounters an alias, then it will unconditionally perform completion
for the aliased git command (in case it can determine the aliased git
command, of course).  With this patch the completion script will first
search for a completion function for the command given on the command
line, be it a git command, a custom git command of the user, or an
alias, and invoke that function to perform the completion.  This has
no effect on git commands, because they can not be aliased anyway.  If
it is an alias and there is a completion function for that alias (e.g.
_git_foo() for the alias 'foo'), then it will be invoked to perform
completion, allowing users to provide custom completion functions for
aliases.  If such a completion function can not be found, only then
will the completion script check whether the command given on the
command line is an alias or not, and proceed as usual (i.e. find out
the aliased git command and provide completion for it).

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2ac3567..8593fd7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2216,11 +2216,14 @@ _git ()
 		return
 	fi
 
-	local expansion=$(__git_aliased_command "$command")
-	[ "$expansion" ] && command="$expansion"
-
 	local completion_func="_git_${command//-/_}"
-	declare -F $completion_func >/dev/null && $completion_func
+	declare -F $completion_func >/dev/null && $completion_func && return
+
+	local expansion=$(__git_aliased_command "$command")
+	if [ -n "$expansion" ]; then
+		completion_func="_git_${expansion//-/_}"
+		declare -F $completion_func >/dev/null && $completion_func
+	fi
 }
 
 _gitk ()
-- 
1.7.0.119.g9b76

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/4] bash: completion for gitk aliases
  2010-02-23 21:02                   ` [PATCH 0/4] bash: support user-supplied completion scripts for custom git commands and aliases SZEDER Gábor
                                       ` (2 preceding siblings ...)
  2010-02-23 21:02                     ` [PATCH 3/4] bash: support user-supplied completion scripts for aliases SZEDER Gábor
@ 2010-02-23 21:03                     ` SZEDER Gábor
  3 siblings, 0 replies; 25+ messages in thread
From: SZEDER Gábor @ 2010-02-23 21:03 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, David Rhodes Clymer, Teemu Matilainen, git,
	SZEDER Gábor

gitk aliases either start with "!gitk", or look something like "!sh -c
FOO=bar gitk", IOW they contain the "gitk" word.  With this patch the
completion script will recognize these cases and will offer gitk's
options.

Just like the earlier change improving on aliased command recognition,
this change can also be fooled easily by some complex aliases, but
users of such aliases could remedy it with custom completion
functions.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8593fd7..3029f16 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -626,6 +626,10 @@ __git_aliased_command ()
 		config --get "alias.$1")
 	for word in $cmdline; do
 		case "$word" in
+		\!gitk|gitk)
+			echo "gitk"
+			return
+			;;
 		\!*)	: shell command alias ;;
 		-*)	: option ;;
 		*=*)	: setting env ;;
@@ -1087,6 +1091,11 @@ _git_gc ()
 	COMPREPLY=()
 }
 
+_git_gitk ()
+{
+	_gitk
+}
+
 _git_grep ()
 {
 	__git_has_doubledash && return
-- 
1.7.0.119.g9b76

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/4] bash: improve aliased command recognition
  2010-02-23 21:02                     ` [PATCH 1/4] bash: improve aliased command recognition SZEDER Gábor
@ 2010-02-23 22:11                       ` Junio C Hamano
  2010-02-24  1:04                         ` SZEDER Gábor
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2010-02-23 22:11 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Shawn O. Pearce, David Rhodes Clymer, Teemu Matilainen, git

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

> [alias]
>     lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"
>
> The full parsing of a shell command alias like that in the completion
> code is clearly unfeasible.  However, we can easily improve on aliased
> command recognition by eleminating stuff that is definitely not a git
> command: shell commands (anything starting with '!'), command line
> options (anything starting with '-'), environment variables (anything
> with a '=' in it), and git itself.  This way the above alias would be
> handled correctly, and the completion script would correctly recognize
> "log" as the aliased git command.

I personally do not think such a heuristic is worth the trouble (both for
writing and maintaining the completion code nor runtime overhead to
iterate over words on the expansion).

I vaguely recall somebody floated an idea to tell completion code that
"you may not know what lgm is, but it takes the same set of options as
log" (either via config or a shell function---I don't recall the details).
I think that would be a lot more robust, efficient and easy to explain
solution to the same problem.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/4] bash: improve aliased command recognition
  2010-02-23 22:11                       ` Junio C Hamano
@ 2010-02-24  1:04                         ` SZEDER Gábor
  2010-02-24  2:56                           ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: SZEDER Gábor @ 2010-02-24  1:04 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Shawn O. Pearce, David Rhodes Clymer, Teemu Matilainen, git

Hi,

On Tue, Feb 23, 2010 at 02:11:20PM -0800, Junio C Hamano wrote:
> SZEDER Gábor <szeder@ira.uka.de> writes:
> 
> > [alias]
> >     lgm = "!sh -c 'GIT_NOTES_REF=refs/notes/amlog git log \"$@\" || :' -"
> >
> > The full parsing of a shell command alias like that in the completion
> > code is clearly unfeasible.  However, we can easily improve on aliased
> > command recognition by eleminating stuff that is definitely not a git
> > command: shell commands (anything starting with '!'), command line
> > options (anything starting with '-'), environment variables (anything
> > with a '=' in it), and git itself.  This way the above alias would be
> > handled correctly, and the completion script would correctly recognize
> > "log" as the aliased git command.
> 
> I personally do not think such a heuristic is worth the trouble (both for
> writing and maintaining the completion code nor runtime overhead to
> iterate over words on the expansion).

Well, the code is already written, so ;)

Runtime overhead seems not to be an issue:

$ git config alias.shortalias log
$ git config alias.longalias \!"sh -c '$(for i in $(seq 1 100) ;do echo -n "A=$i " ;done) git log -1'"
$ time __git_aliased_command shortalias
log

real    0m0.008s
user    0m0.004s
sys     0m0.004s
$ time __git_aliased_command longalias
log

real    0m0.017s
user    0m0.012s
sys     0m0.004s

That is, although it takes around twice as long in case of a 100+ word
long alias than with a trivial one, it is still in the hundredth of a
second range.  And I doubt that many people have that long aliases.

Maintenance might be an issue, or, erm...  well, it already is.  An
alias like "!sh -c 'git log -1'" does not work, because "'git" does
not match any of the patterns, therefore it is returned as the aliased
command.

> I vaguely recall somebody floated an idea to tell completion code that
> "you may not know what lgm is, but it takes the same set of options as
> log" (either via config or a shell function---I don't recall the details).

Shawn proposed the approach via config variables and patches 2/4 and
3/4 actually implement the shell function approach.

Personally, I prefer the shell function approach.  It needs less 'git
config' queries; actually, in this respect it is better than current
master, because there is no 'git config' query at all for the git
command case.  Furthermore, I don't really like the idea of putting
completion related stuff into git configuration files, but this is, of
course, subjective.

> I think that would be a lot more robust, efficient and easy to explain
> solution to the same problem.

I agree that this heuristic is not 100% robust.  Efficiency is not an
issue, as shown above.  But I think we should also look at efficiency
and overhead at the user, too.  That is, this heuristic will work
without any action required from the user, while the other two
approaches require the user to explicitly specify the completion rules
for his non-trivial aliases.  Finally, I think it is not all that
difficult to explain:

  "The bash completion script uses some heuristics to find out the git
  command invoked by aliases.  If you have an alias for which the
  completion script does nothing or outputs garbage, then you should
  write a one-liner shell function, which ..." and here comes what you
  would need to explain anyway.


Note, that patches 1/4 and 4/4 are independent from the changes in 2/4
and 3/4, so if you are not satisfied with these heuristic changes, you
can still drop only those but not the custom completion patches.


Best,
Gábor

(and it's already tomorrow here, so the thoughts about completion
namespaces will have to wait)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/4] bash: improve aliased command recognition
  2010-02-24  1:04                         ` SZEDER Gábor
@ 2010-02-24  2:56                           ` Junio C Hamano
  0 siblings, 0 replies; 25+ messages in thread
From: Junio C Hamano @ 2010-02-24  2:56 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Junio C Hamano, Shawn O. Pearce, David Rhodes Clymer,
	Teemu Matilainen, git

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

> Personally, I prefer the shell function approach.  It needs less 'git
> config' queries; actually, in this respect it is better than current
> master, because there is no 'git config' query at all for the git
> command case.  Furthermore, I don't really like the idea of putting
> completion related stuff into git configuration files, but this is, of
> course, subjective.

Well, at least both of us seem to share the same subjective criteria ;-).
The custom shell function approach is the most straightforward.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] bash: support user-supplied completion scripts for user's git commands
  2010-01-29 20:32           ` [PATCH] bash: support user-supplied completion scripts for user's git commands Junio C Hamano
@ 2010-02-26 15:27             ` SZEDER Gábor
  2010-02-26 20:04               ` Junio C Hamano
  0 siblings, 1 reply; 25+ messages in thread
From: SZEDER Gábor @ 2010-02-26 15:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, David Rhodes Clymer, git

Hi,

On Fri, Jan 29, 2010 at 12:32:12PM -0800, Junio C Hamano wrote:
> Admittedly, we have already taken over _git_foo (and "_git") namespace,
> and anybody who uses bash with the completion support cannot write their
> own shell function with these names for purposes that are unrelated to
> completion,

Actually, the "_" namespace is taken over by bash completion in
general, so writing shell functions starting with "_" is probably
not a good idea anyway.  E.g. to see all non-completion-related shell
functions you can do a "declare -F |grep -v ' _'", but if you name
shell functions not related to completion as _git_foo(), then this
will no longer work.

> so in that sense, the patch is not introducing a new problem,
> but making it a documented interface and casting it in stone will make the
> namespace contamination issue harder to rectify later.
> 
> So if we were to go in the direction as the patch proposes (which I think
> is a good idea), we might want to rename them to __git_completion_foo or
> something that is less likely to collide with whatever names users might
> want to use. It is my understanding that the only published interface so
> far is __git_ps1.

I would say that __git_ps1() is the only interface that is advertised
as being public.  If someone is unsatisfied with the completion
script, because he wanted completion for a custom git command or for a
frequently used plumbing command, then I bet he just reused existing
functions, e.g. when he needed refs, he just used __git_refs(), or
when he needed git log's options, he used _git_log().  I did that,
probably others too.  If we were to rename completion functions, these
people's setup will break (although they will likely get merge
conflicts caused by this patch anyway).  On the other hand: should we
really care that much about such users, who use non-pulic interfaces
from contrib/ ?

Having said all that, I don't really care either way.  If you or Shawn
would prefer to have the completion functions renamed, I will do a
s/this/that/ preparation patch for the series.  BTW, Mercurial's
completion script uses _hg_cmd_foo() for hg commands and
_hg_ext_bar() for extensions, so we might as well be a bit consistent,
and call our completion functions _git_cmd_foo().


Best,
Gábor

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] bash: support user-supplied completion scripts for user's git commands
  2010-02-26 15:27             ` SZEDER Gábor
@ 2010-02-26 20:04               ` Junio C Hamano
  2010-02-26 20:17                 ` Shawn O. Pearce
  0 siblings, 1 reply; 25+ messages in thread
From: Junio C Hamano @ 2010-02-26 20:04 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Shawn O. Pearce, David Rhodes Clymer, git

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

>> so in that sense, the patch is not introducing a new problem,
>> but making it a documented interface and casting it in stone will make the
>> namespace contamination issue harder to rectify later.

I won't quote the first paragraph where you are repeating what I said,
while sounding as if you were disagreeing with me.

>> So if we were to ...
>> ... It is my understanding that the only published interface so
>> far is __git_ps1.
>
> I would say that __git_ps1() is the only interface that is advertised
> as being public.  ...
> ...  If we were to rename completion functions, these
> people's setup will break (although they will likely get merge
> conflicts caused by this patch anyway).  On the other hand: should we
> really care that much about such users, who use non-pulic interfaces
> from contrib/ ?

I see we are in agreement in the first half of your paragraph; my answer
to the question in the latter half is:

 - we shouldn't care about people who already used unpublished interface
   in contrib/ so far; _but_

 - because we will be advertising it as a way to override and enhance
   completion to define your own shell functions, the naming _will_ become
   part of published interface---what we decide _now_ will matter.

That is why I wanted people to at least think about renaming _git_frotz to
something less generic.  The name tells us that it is a helper shell
function about the "git frotz" command, but it does not say what aspect of
"git frotz" it is meant to help, i.e. completion.  _git_complete_frotz or
a variant of such would not have that problem, and will keep the door open
for future shell helpers that are about different aspect "xxx" that is
unrelated to completion---they can then name theirs _git_xxx_frotz.

> ...  BTW, Mercurial's
> completion script uses _hg_cmd_foo() for hg commands and
> _hg_ext_bar() for extensions, so we might as well be a bit consistent,
> and call our completion functions _git_cmd_foo().

In Hg's context it might make sense to name a function _hg_cmd_foo vs
_hg_ext_bar iff the end users need to be very aware of the distinction
between commands and extensions, but for us I think "git_cmd_foo" is
probably the most meaningless rename, as it doesn't add any extra
information (we know 'git foo' is a command already without 'cmd').

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] bash: support user-supplied completion scripts for user's git commands
  2010-02-26 20:04               ` Junio C Hamano
@ 2010-02-26 20:17                 ` Shawn O. Pearce
  0 siblings, 0 replies; 25+ messages in thread
From: Shawn O. Pearce @ 2010-02-26 20:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: SZEDER G??bor, David Rhodes Clymer, git

Junio C Hamano <gitster@pobox.com> wrote:
> > ...  BTW, Mercurial's
> > completion script uses _hg_cmd_foo() for hg commands and
> > _hg_ext_bar() for extensions, so we might as well be a bit consistent,
> > and call our completion functions _git_cmd_foo().
> 
> In Hg's context it might make sense to name a function _hg_cmd_foo vs
> _hg_ext_bar iff the end users need to be very aware of the distinction
> between commands and extensions, but for us I think "git_cmd_foo" is
> probably the most meaningless rename, as it doesn't add any extra
> information (we know 'git foo' is a command already without 'cmd').

I agree.  _git_cmd_foo is pointless.

But I would be ok with _git_completion_foo for the completion
function of git foo.  As Junio pointed out, better to do it now
before users start to really build their own extension library on
top of the package.

-- 
Shawn.

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2010-02-26 20:17 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-29 12:57 Custom git completion David Rhodes Clymer
2010-01-29 15:11 ` Shawn O. Pearce
2010-01-29 17:42   ` Junio C Hamano
2010-01-29 17:59     ` Shawn O. Pearce
2010-01-29 18:02       ` Junio C Hamano
2010-01-29 19:06         ` [PATCH] bash: support user-supplied completion scripts for user's git commands SZEDER Gábor
2010-01-29 19:13           ` Shawn O. Pearce
2010-01-29 20:00             ` SZEDER Gábor
2010-01-29 20:04               ` Shawn O. Pearce
2010-01-31 19:19                 ` SZEDER Gábor
2010-02-23 21:02                   ` [PATCH 0/4] bash: support user-supplied completion scripts for custom git commands and aliases SZEDER Gábor
2010-02-23 21:02                     ` [PATCH 1/4] bash: improve aliased command recognition SZEDER Gábor
2010-02-23 22:11                       ` Junio C Hamano
2010-02-24  1:04                         ` SZEDER Gábor
2010-02-24  2:56                           ` Junio C Hamano
2010-02-23 21:02                     ` [PATCH 2/4] bash: support user-supplied completion scripts for user's git commands SZEDER Gábor
2010-02-23 21:02                     ` [PATCH 3/4] bash: support user-supplied completion scripts for aliases SZEDER Gábor
2010-02-23 21:03                     ` [PATCH 4/4] bash: completion for gitk aliases SZEDER Gábor
2010-01-29 20:32           ` [PATCH] bash: support user-supplied completion scripts for user's git commands Junio C Hamano
2010-02-26 15:27             ` SZEDER Gábor
2010-02-26 20:04               ` Junio C Hamano
2010-02-26 20:17                 ` Shawn O. Pearce
2010-01-30 23:34           ` David Rhodes Clymer
2010-01-30 23:03       ` Custom git completion David Rhodes Clymer
2010-01-30 23:00   ` David Rhodes Clymer

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).