Re: [PATCH] completion: add option completion for most builtin commands
On Thu, Mar 22, 2018 at 6:56 PM, Junio C Hamano wrote:
> Duy Nguyen writes:
>
>> +__git_main_with_parseopt_helper='
>> + blame cat-file check-attr check-ignore
>> + check-mailmap checkout-index column count-objects fast-export
>> + hash-object index-pack interpret-trailers merge-file mktree
>> + pack-objects pack-refs prune prune-packed read-tree repack
>> + send-pack show-ref stripspace symbolic-ref update-index
>> + update-ref verify-commit verify-tag write-tree
>> +'
>> +__git_complete_command() {
>> + local command="$1"
>> + local completion_func="_git_${command//-/_}"
>> + if declare -f $completion_func >/dev/null 2>/dev/null; then
>> + $completion_func
>> + elif echo $__git_main_with_parseopt_helper | git grep -w "$command"
>> >/dev/null; then
>
> "git grep"???
>
> I imagined that you'd keep an associative shell array (we are not
> constrained by POSIX here) that can be used like so
>
> elif test -n "${__git_main_with_parseopt_helper[$command]}"; then
Nope. We are not constrained by POSIX, but MacOS still runs the
ancient bash 3.x while associative arrays are in 4.x
--
Duy
Re: [PATCH] completion: add option completion for most builtin commands
On Thu, Mar 22, 2018 at 6:56 PM, Junio C Hamano wrote:
> Duy Nguyen writes:
>
>> +__git_main_with_parseopt_helper='
>> + blame cat-file check-attr check-ignore
>> + check-mailmap checkout-index column count-objects fast-export
>> + hash-object index-pack interpret-trailers merge-file mktree
>> + pack-objects pack-refs prune prune-packed read-tree repack
>> + send-pack show-ref stripspace symbolic-ref update-index
>> + update-ref verify-commit verify-tag write-tree
>> +'
>> +__git_complete_command() {
>> + local command="$1"
>> + local completion_func="_git_${command//-/_}"
>> + if declare -f $completion_func >/dev/null 2>/dev/null; then
>> + $completion_func
>> + elif echo $__git_main_with_parseopt_helper | git grep -w "$command"
>> >/dev/null; then
>
> "git grep"???
GIt has taught my fingers some bad habit.
>
> I imagined that you'd keep an associative shell array (we are not
> constrained by POSIX here) that can be used like so
>
> elif test -n "${__git_main_with_parseopt_helper[$command]}"; then
>
Great. We could even kill two existing _git_xxx functions because they
are too simple they could be replaced with this new code. I'll send
out a series later.
--
Duy
Re: [PATCH] completion: add option completion for most builtin commands
Duy Nguyen writes:
> +__git_main_with_parseopt_helper='
> + blame cat-file check-attr check-ignore
> + check-mailmap checkout-index column count-objects fast-export
> + hash-object index-pack interpret-trailers merge-file mktree
> + pack-objects pack-refs prune prune-packed read-tree repack
> + send-pack show-ref stripspace symbolic-ref update-index
> + update-ref verify-commit verify-tag write-tree
> +'
> +__git_complete_command() {
> + local command="$1"
> + local completion_func="_git_${command//-/_}"
> + if declare -f $completion_func >/dev/null 2>/dev/null; then
> + $completion_func
> + elif echo $__git_main_with_parseopt_helper | git grep -w "$command"
> >/dev/null; then
"git grep"???
I imagined that you'd keep an associative shell array (we are not
constrained by POSIX here) that can be used like so
elif test -n "${__git_main_with_parseopt_helper[$command]}"; then
Of course, a more traditional way to write it without spawning grep
or pipe is
case " $__git_main_with_parseopt_helper " in
*" $command "*)
... Yes, $command is on the list ...
;;
*)
... No, $command is not on the list ...
;;
esac
Re: [PATCH] completion: add option completion for most builtin commands
On Thu, Mar 22, 2018 at 10:11:53AM -0700, Junio C Hamano wrote:
> Duy Nguyen writes:
>
> >> And that pattern repeats throughout the patch. I wonder if we can
> >> express the same a lot more concisely by updating the caller that
> >> calls these command specific helpers?
> >
> > Yeah. I almost went to just generate and eval these functions. But we
> > still need to keep a list of "bultin with --git-completion-helper"
> > support somewhere, and people may want to complete arguments without
> > double dashes (e.g. read-tree should take a ref...) which can't be
> > helped by --git-completion-helper.
>
> Hmph, I actually did not have 'eval' in mind.
>
> Rather, I was wondering if it is cleaner to update __git_main where
> it computes $completion_func by mangling $command and then calls
> it---instead call __gitcomp_builtin directly when the $command
> appears in such a "list of builtins that knows --completion-helper
> and no custom completion".
Ah. Something like this? Seems to work fine and definitely not as ugly
as eval.
-- 8< --
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6da95b8095..960e26e09d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3032,6 +3032,32 @@ _git_worktree ()
fi
}
+__git_main_with_parseopt_helper='
+ blame cat-file check-attr check-ignore
+ check-mailmap checkout-index column count-objects fast-export
+ hash-object index-pack interpret-trailers merge-file mktree
+ pack-objects pack-refs prune prune-packed read-tree repack
+ send-pack show-ref stripspace symbolic-ref update-index
+ update-ref verify-commit verify-tag write-tree
+'
+__git_complete_command() {
+ local command="$1"
+ local completion_func="_git_${command//-/_}"
+ if declare -f $completion_func >/dev/null 2>/dev/null; then
+ $completion_func
+ elif echo $__git_main_with_parseopt_helper | git grep -w "$command"
>/dev/null; then
+ case "$cur" in
+ --*)
+ __gitcomp_builtin "$command"
+ return 0
+ ;;
+ esac
+ return 0
+ else
+ return 1
+ fi
+}
+
__git_main ()
{
local i c=1 command __git_dir __git_repo_path
@@ -3091,14 +3117,12 @@ __git_main ()
return
fi
- local completion_func="_git_${command//-/_}"
- declare -f $completion_func >/dev/null 2>/dev/null && $completion_func
&& return
+ __git_complete_command "$command" && return
local expansion=$(__git_aliased_command "$command")
if [ -n "$expansion" ]; then
words[1]=$expansion
- completion_func="_git_${expansion//-/_}"
- declare -f $completion_func >/dev/null 2>/dev/null &&
$completion_func
+ __git_complete_command "$expansion"
fi
}
-- 8< --
Re: [PATCH] completion: add option completion for most builtin commands
Duy Nguyen writes: >> And that pattern repeats throughout the patch. I wonder if we can >> express the same a lot more concisely by updating the caller that >> calls these command specific helpers? > > Yeah. I almost went to just generate and eval these functions. But we > still need to keep a list of "bultin with --git-completion-helper" > support somewhere, and people may want to complete arguments without > double dashes (e.g. read-tree should take a ref...) which can't be > helped by --git-completion-helper. Hmph, I actually did not have 'eval' in mind. Rather, I was wondering if it is cleaner to update __git_main where it computes $completion_func by mangling $command and then calls it---instead call __gitcomp_builtin directly when the $command appears in such a "list of builtins that knows --completion-helper and no custom completion".
Re: [PATCH] completion: add option completion for most builtin commands
On Wed, Mar 21, 2018 at 9:59 PM, Junio C Hamano wrote:
> Nguyễn Thái Ngọc Duy writes:
>
>> These commands can take options and use parse-options so it's quite
>> easy to allow option completion. This does not pollute the command
>> name completion though. "git " will show you the same set as
>> before. This only kicks in when you type the correct command name.
>>
>> Some other builtin commands are not still added because either they
>> don't use parse-options, or they are deprecated, or they are those
>> -helper commands that are used to move some logic back in C for
>> sh-based commands.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy
>> ---
>> contrib/completion/git-completion.bash | 276 +
>> 1 file changed, 276 insertions(+)
>
> Wow, just wow. It however looks a lot of boilerplates, e.g. looking
> at these, we notice ...
>
>> +_git_blame() {
>> + case "$cur" in
>> + --*)
>> + __gitcomp_builtin blame
>> + return
>> + ;;
>> + esac
>> +}
>> +
>>
>> +_git_cat_file() {
>> + case "$cur" in
>> + --*)
>> + __gitcomp_builtin cat-file
>> + return
>> + ;;
>> + esac
>> +}
>> +
>> +_git_check_attr() {
>> + case "$cur" in
>> + --*)
>> + __gitcomp_builtin check-attr
>> + return
>> + ;;
>> + esac
>> +}
>
> ... the only thing we need for the above three is a table that says
> "use blame for blame, cat-file for cat_file, and check-attr for
> check_attr".
>
> And that pattern repeats throughout the patch. I wonder if we can
> express the same a lot more concisely by updating the caller that
> calls these command specific helpers?
>
Yeah. I almost went to just generate and eval these functions. But we
still need to keep a list of "bultin with --git-completion-helper"
support somewhere, and people may want to complete arguments without
double dashes (e.g. read-tree should take a ref...) which can't be
helped by --git-completion-helper.
--
Duy
Re: [PATCH] completion: add option completion for most builtin commands
Nguyễn Thái Ngọc Duy writes:
> These commands can take options and use parse-options so it's quite
> easy to allow option completion. This does not pollute the command
> name completion though. "git " will show you the same set as
> before. This only kicks in when you type the correct command name.
>
> Some other builtin commands are not still added because either they
> don't use parse-options, or they are deprecated, or they are those
> -helper commands that are used to move some logic back in C for
> sh-based commands.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> contrib/completion/git-completion.bash | 276 +
> 1 file changed, 276 insertions(+)
Wow, just wow. It however looks a lot of boilerplates, e.g. looking
at these, we notice ...
> +_git_blame() {
> + case "$cur" in
> + --*)
> + __gitcomp_builtin blame
> + return
> + ;;
> + esac
> +}
> +
>
> +_git_cat_file() {
> + case "$cur" in
> + --*)
> + __gitcomp_builtin cat-file
> + return
> + ;;
> + esac
> +}
> +
> +_git_check_attr() {
> + case "$cur" in
> + --*)
> + __gitcomp_builtin check-attr
> + return
> + ;;
> + esac
> +}
... the only thing we need for the above three is a table that says
"use blame for blame, cat-file for cat_file, and check-attr for
check_attr".
And that pattern repeats throughout the patch. I wonder if we can
express the same a lot more concisely by updating the caller that
calls these command specific helpers?
[PATCH] completion: add option completion for most builtin commands
These commands can take options and use parse-options so it's quite
easy to allow option completion. This does not pollute the command
name completion though. "git " will show you the same set as
before. This only kicks in when you type the correct command name.
Some other builtin commands are not still added because either they
don't use parse-options, or they are deprecated, or they are those
-helper commands that are used to move some logic back in C for
sh-based commands.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
contrib/completion/git-completion.bash | 276 +
1 file changed, 276 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6da95b8095..0cd9180f48 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1202,6 +1202,15 @@ _git_bisect ()
esac
}
+_git_blame() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin blame
+ return
+ ;;
+ esac
+}
+
_git_branch ()
{
local i c=1 only_local_ref="n" has_r="n"
@@ -1254,6 +1263,42 @@ _git_bundle ()
esac
}
+_git_cat_file() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin cat-file
+ return
+ ;;
+ esac
+}
+
+_git_check_attr() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin check-attr
+ return
+ ;;
+ esac
+}
+
+_git_check_ignore() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin check-ignore
+ return
+ ;;
+ esac
+}
+
+_git_check_mailmap() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin check-mailmap
+ return
+ ;;
+ esac
+}
+
_git_checkout ()
{
__git_has_doubledash && return
@@ -1278,6 +1323,15 @@ _git_checkout ()
esac
}
+_git_checkout_index() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin checkout-index
+ return
+ ;;
+ esac
+}
+
_git_cherry ()
{
__git_complete_refs
@@ -1326,6 +1380,15 @@ _git_clone ()
esac
}
+_git_column() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin column
+ return
+ ;;
+ esac
+}
+
__git_untracked_file_modes="all no normal"
_git_commit ()
@@ -1365,6 +1428,15 @@ _git_commit ()
fi
}
+_git_count_objects() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin count-objects
+ return
+ ;;
+ esac
+}
+
_git_describe ()
{
case "$cur" in
@@ -1446,6 +1518,15 @@ _git_difftool ()
__git_complete_revlist_file
}
+_git_fast_export() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin fast-export
+ return
+ ;;
+ esac
+}
+
__git_fetch_recurse_submodules="yes on-demand no"
_git_fetch ()
@@ -1573,6 +1654,15 @@ _git_grep ()
__git_complete_refs
}
+_git_hash_object() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin hash-object
+ return
+ ;;
+ esac
+}
+
_git_help ()
{
case "$cur" in
@@ -1590,6 +1680,15 @@ _git_help ()
"
}
+_git_index_pack() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin index-pack
+ return
+ ;;
+ esac
+}
+
_git_init ()
{
case "$cur" in
@@ -1606,6 +1705,15 @@ _git_init ()
esac
}
+_git_interpret_trailers() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin interpret-trailers
+ return
+ ;;
+ esac
+}
+
_git_ls_files ()
{
case "$cur" in
@@ -1765,6 +1873,15 @@ _git_merge ()
__git_complete_refs
}
+_git_merge_file() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin merge-file
+ return
+ ;;
+ esac
+}
+
_git_mergetool ()
{
case "$cur" in
@@ -1790,6 +1907,15 @@ _git_merge_base ()
__git_complete_refs
}
+_git_mktree() {
+ case "$cur" in
+ --*)
+ __gitcomp_builtin mktree
+ return
+ ;;
+ esac
+}
+
_git_mv ()
{
case "$cur" in
@@ -1853,6 +1979,46 @@ _git_notes ()
esac
}
+_git_pack_objects ()
+{
+ case "$cur" in
+ --*)
+ __gitcomp_builtin pack-objects
+ return
+ ;;
+ esac
+}
+
+_git_pack_refs ()
+{
+ case "$cur" in
+ --*)
+ __gitcomp_builtin pack-refs
+ return
+ ;;
+ esac
+}
+
+_git_prune ()
+{
+ case "$cur" in
+ --*)
+ __gitcomp_builtin prune
+ return
+ ;;
+ esac
+}
+
+_git_prune_packed ()
+{
+ case "$cur" in
+ --*)
+ __gitc
