subject:"\[PATCH v5 1\/3\] completion\: add new _

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

2012-10-30 Thread SZEDER Gábor

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 sze...@ira.uka.de wrote:
  On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:
 
  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.

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

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

2012-10-21 Thread Felipe Contreras

On Wed, Oct 17, 2012 at 7:28 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:

 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.

  __git_heads ()
 @@ -486,7 +489,7 @@ __git_complete_remote_or_refspec ()
   case $cmd in
   push) no_complete_refspec=1 ;;
   fetch)
 - COMPREPLY=()
 + __gitcompadd
   return
   ;;
   *) ;;
 @@ -502,7 +505,7 @@ __git_complete_remote_or_refspec ()
   return
   fi
   if [ $no_complete_refspec = 1 ]; then
 - COMPREPLY=()
 + __gitcompadd
   return
   fi
   [ $remote = . ]  remote=
 @@ -776,7 +779,7 @@ _git_am ()
   
   return
   esac
 - COMPREPLY=()
 + __gitcompadd

 These changes effectively run compgen in a subshell to generate an
 empty completion reply.  While it doesn't really matter on Linux,
 it'll add another half a tenth of a second delay in those cases on my
 Windows machine.  At least it should be conditional, i.e. $(compgen
 ...) shouldn't be executed when there are no possible completion
 words.

 However, I think those COMPREPLY=() assignments are pointless anyway.
 COMPREPLY is always empty when completion functions are invoked, so
 there is no need to explicitly set it to an empty array when we don't
 provide any words for completion.  Their only use is basically to
 explicitly tell us humans that in those cases we don't offer any words
 for completion.  But we don't do that consistently: there are several
 places without offering words for completion and without COMPREPLY=(),
 e.g. the '__git_has_doubledash  return' pattern.

 Perhaps it would be time to get rid of these COMPREPLY=() assignments?

I'm all for it, I never understood what was the purpose of that. I
believe zsh could benefit from this information to decide whether to
run the default completion (e.g. files) or not, but as you said, if
it's not used consistently for bash, there's no point in trying.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-10-17 Thread SZEDER Gábor

On Sun, Oct 14, 2012 at 05:52:49PM +0200, Felipe Contreras wrote:
 The idea is to never touch the COMPREPLY variable directly.
 
 This allows other completion systems override __gitcompadd, and do
 something different instead.
 
 Also, this allows the simplifcation of the completino tests (separate
 patch).
 
 There should be no functional changes.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  contrib/completion/git-completion.bash | 65 
 ++
  1 file changed, 34 insertions(+), 31 deletions(-)
 
 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.

  __git_heads ()
 @@ -486,7 +489,7 @@ __git_complete_remote_or_refspec ()
   case $cmd in
   push) no_complete_refspec=1 ;;
   fetch)
 - COMPREPLY=()
 + __gitcompadd
   return
   ;;
   *) ;;
 @@ -502,7 +505,7 @@ __git_complete_remote_or_refspec ()
   return
   fi
   if [ $no_complete_refspec = 1 ]; then
 - COMPREPLY=()
 + __gitcompadd
   return
   fi
   [ $remote = . ]  remote=
 @@ -776,7 +779,7 @@ _git_am ()
   
   return
   esac
 - COMPREPLY=()
 + __gitcompadd

These changes effectively run compgen in a subshell to generate an
empty completion reply.  While it doesn't really matter on Linux,
it'll add another half a tenth of a second delay in those cases on my
Windows machine.  At least it should be conditional, i.e. $(compgen
...) shouldn't be executed when there are no possible completion
words.

However, I think those COMPREPLY=() assignments are pointless anyway.
COMPREPLY is always empty when completion functions are invoked, so
there is no need to explicitly set it to an empty array when we don't
provide any words for completion.  Their only use is basically to
explicitly tell us humans that in those cases we don't offer any words
for completion.  But we don't do that consistently: there are several
places without offering words for completion and without COMPREPLY=(),
e.g. the '__git_has_doubledash  return' pattern.

Perhaps it would be time to get rid of these COMPREPLY=() assignments?

  }
  
  _git_apply ()
 @@ -796,7 +799,7 @@ _git_apply ()
   
   return
   esac
 - COMPREPLY=()
 + __gitcompadd
  }
  
  _git_add ()
 @@ -811,7 +814,7 @@ _git_add ()
   
   return
   esac
 - COMPREPLY=()
 + __gitcompadd
  }
  
  _git_archive ()
 @@ -856,7 +859,7 @@ _git_bisect ()
   __gitcomp_nl $(__git_refs)
   ;;
   *)
 - COMPREPLY=()
 + __gitcompadd
   ;;
   esac
  }
 @@ -965,7 +968,7 @@ _git_clean ()
   return
   ;;
   esac
 - COMPREPLY=()
 + __gitcompadd
  }
  
  _git_clone ()
 @@ -989,7 +992,7 @@ _git_clone ()
   return
   ;;
   esac
 - COMPREPLY=()
 + __gitcompadd
  }
  
  _git_commit ()
 @@ -1023,7 +1026,7 @@ _git_commit ()
   
   return
   esac
 - COMPREPLY=()
 + __gitcompadd
  }
  
  _git_describe ()
 @@ -1154,7 +1157,7 @@ _git_fsck ()
   return
   ;;
   esac
 -

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

2012-10-14 Thread Felipe Contreras

The idea is to never touch the COMPREPLY variable directly.

This allows other completion systems override __gitcompadd, and do
something different instead.

Also, this allows the simplifcation of the completino tests (separate
patch).

There should be no functional changes.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/completion/git-completion.bash | 65 ++
 1 file changed, 34 insertions(+), 31 deletions(-)

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- }
 }
 
 __git_heads ()
@@ -486,7 +489,7 @@ __git_complete_remote_or_refspec ()
case $cmd in
push) no_complete_refspec=1 ;;
fetch)
-   COMPREPLY=()
+   __gitcompadd
return
;;
*) ;;
@@ -502,7 +505,7 @@ __git_complete_remote_or_refspec ()
return
fi
if [ $no_complete_refspec = 1 ]; then
-   COMPREPLY=()
+   __gitcompadd
return
fi
[ $remote = . ]  remote=
@@ -776,7 +779,7 @@ _git_am ()

return
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_apply ()
@@ -796,7 +799,7 @@ _git_apply ()

return
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_add ()
@@ -811,7 +814,7 @@ _git_add ()

return
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_archive ()
@@ -856,7 +859,7 @@ _git_bisect ()
__gitcomp_nl $(__git_refs)
;;
*)
-   COMPREPLY=()
+   __gitcompadd
;;
esac
 }
@@ -965,7 +968,7 @@ _git_clean ()
return
;;
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_clone ()
@@ -989,7 +992,7 @@ _git_clone ()
return
;;
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_commit ()
@@ -1023,7 +1026,7 @@ _git_commit ()

return
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_describe ()
@@ -1154,7 +1157,7 @@ _git_fsck ()
return
;;
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_gc ()
@@ -1165,7 +1168,7 @@ _git_gc ()
return
;;
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_gitk ()
@@ -1242,7 +1245,7 @@ _git_init ()
return
;;
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_ls_files ()
@@ -1261,7 +1264,7 @@ _git_ls_files ()
return
;;
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_ls_remote ()
@@ -1377,7 +1380,7 @@ _git_mergetool ()
return
;;
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_merge_base ()
@@ -1393,7 +1396,7 @@ _git_mv ()
return
;;
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_name_rev ()
@@ -1563,7 +1566,7 @@ _git_send_email ()
return
;;
esac
-   COMPREPLY=()
+   __gitcompadd
 }
 
 _git_stage ()
@@ -1616,7 +1619,7 @@ _git_config ()
local remote=${prev#remote.}
remote=${remote%.fetch}
if [ -z $cur ]; then
-   COMPREPLY=(refs/heads/)
+   __gitcompadd refs/heads/
return
fi
__gitcomp_nl $(__git_refs_remotes $remote)
@@ -1676,7 +1679,7 @@ _git_config ()
return
;;
*.*)
-   COMPREPLY=()
+   __gitcompadd

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information