Re: [PATCH v5 1/3] completion: add new __gitcompadd helper

On Mon, Oct 22, 2012 at 02:41:21AM +0200, Felipe Contreras wrote:

On Wed, Oct 17, 2012 at 7:28 PM, SZEDER Gábor [off-list ref] wrote:

quoted

On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:

quoted

quoted

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d743e56..01325de 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -225,6 +225,11 @@ _get_comp_words_by_ref ()
 fi
 fi

+__gitcompadd ()
+{
+     COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
+}
+
 # Generates completion reply with compgen, appending a space to possible
 # completion words, if necessary.
 # It accepts 1 to 4 arguments:

@@ -238,13 +243,11 @@ __gitcomp ()

      case "$cur_" in
      --*=)
-             COMPREPLY=()
+             __gitcompadd
              ;;
      *)
              local IFS=$'\n'
-             COMPREPLY=($(compgen -P "${2-}" \
-                     -W "$(__gitcomp_1 "${1-}" "${4-}")" \
-                     -- "$cur_"))
+             __gitcompadd "$(__gitcomp_1 "${1-}" "${4-}")" "${2-}" "$cur_" ""
              ;;
      esac
 }

@@ -261,7 +264,7 @@ __gitcomp ()
 __gitcomp_nl ()
 {
      local IFS=$'\n'
-     COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
+     __gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
 }

I feel hesitant about this change.  One of the ways I'm exploring to
fix the issues with shell metacharacters and expansion in compgen is
to actually replace compgen.  We already iterate over all possible
completion words in __gitcomp_1(), so it doesn't make much of a
difference to do the filtering for the current word while we are at
it.  However, the way __gitcompadd() encapsulates COMPREPLY=($(compgen
...)), and tha basic idea of never touching COMPREPLY directly make
this basically impossible.

How is it impossible? You can still replace compgen, all you have to
do is modify __gitcompadd and replace that code with whatever custom
code you want. You can change the arguments and everything. The only
limitation is that it should be the only place where COMPREPLY is
modified, and all is good. Well, it doesn't have to be only _one_
place, but the less functions that do this, the better.

That's exactly the problem: there isn't, there can't be one single
"whatever custom code I want".

The compgen() in __gitcomp() will be replaced by an enhanced version
of the loop in __gitcomp_1(), while in __gitcomp_nl() it will be
replaced by a little awk scriptlet.  And then there is the oddball
$(git ls-tree |sed magic) in __git_complete_revlist_file(), where
possible completion words are filenames possibly containing newlines,
therefore requiring yet another approach.