Re: [PATCH v5] git-completion.bash: add support for path completion

2013-04-26 Thread Felipe Contreras
On Tue, Apr 23, 2013 at 10:25 AM, Manlio Perillo
manlio.peri...@gmail.com wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 Il 21/04/2013 12:14, Felipe Contreras ha scritto:
 On Fri, Jan 11, 2013 at 12:48 PM, Manlio Perillo
 manlio.peri...@gmail.com wrote:
 The git-completion.bash script did not implemented full, git aware,
 support to complete paths, for git commands that operate on files within
 the current working directory or the index.

 +__git_index_file_list_filter_compat ()
 +{
 +   local path
 +
 +   while read -r path; do
 +   case $path in
 +   ?*/*) echo ${path%%/*}/ ;;
 +   *) echo $path ;;
 +   esac
 +   done
 +}
 +
 +__git_index_file_list_filter_bash ()
 +{
 +   local path
 +
 +   while read -r path; do
 +   case $path in
 +   ?*/*)
 +   # XXX if we append a slash to directory names when 
 using
 +   # `compopt -o filenames`, Bash will append another 
 slash.
 +   # This is pretty stupid, and this the reason why we 
 have to
 +   # define a compatible version for this function.
 +   echo ${path%%/*} ;;

 Which version of bash is that? It works perfectly fine here with or
 without the /.


 GNU bash, version 4.1.5(1)-release (i486-pc-linux-gnu)
 on a GNU Linux Debian 6

I compiled 4.1 and I can't reproduce, and I tried on a debian squeeze
chroot and I also can't reproduce. What am I missing?

GNU bash, version 4.1.5(1)-release (x86_64-pc-linux-gnu)

 +# __git_index_files accepts 1 or 2 arguments:
 +# 1: Options to pass to ls-files (required).
 +#Supported options are --cached, --modified, --deleted, --others,
 +#and --directory.
 +# 2: A directory path (optional).
 +#If provided, only files within the specified directory are listed.
 +#Sub directories are never recursed.  Path must have a trailing
 +#slash.
 +__git_index_files ()
 +{
 +   local dir=$(__gitdir) root=${2-.}
 +
 +   if [ -d $dir ]; then
 +   __git_ls_files_helper $root $1 | 
 __git_index_file_list_filter |
 +   sort | uniq
 +   fi
 +}
 +
 +# __git_diff_index_files accepts 1 or 2 arguments:
 +# 1) The id of a tree object.
 +# 2) A directory path (optional).
 +#If provided, only files within the specified directory are listed.
 +#Sub directories are never recursed.  Path must have a trailing
 +#slash.
 +__git_diff_index_files ()
 +{
 +   local dir=$(__gitdir) root=${2-.}
 +
 +   if [ -d $dir ]; then
 +   __git_diff_index_helper $root $1 | 
 __git_index_file_list_filter |
 +   sort | uniq
 +   fi
 +}

 These two are exactly the same, except one calls
 __git_ls_files_helper, and the other one __git_diff_index_helper,
 can't we make another argument that is and select one or the other
 based on that?


 They are not exactly the same.

 The first function requires, as first parameter, (space separed) options
 to pass to ls-files command; the second function, instead, requires the
 id of a tree object.

 IMHO, using only one function may be confusing.

The functions down in the call-stack might be doing something
different but these functions themselves are not. At the end of the
day they simply pass arguments to ls-files or diff-index.

I'm certain these can be simplified greatly.

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] git-completion.bash: add support for path completion

2013-04-23 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 21/04/2013 12:14, Felipe Contreras ha scritto:
 On Fri, Jan 11, 2013 at 12:48 PM, Manlio Perillo
 manlio.peri...@gmail.com wrote:
 The git-completion.bash script did not implemented full, git aware,
 support to complete paths, for git commands that operate on files within
 the current working directory or the index.
 
 +__git_index_file_list_filter_compat ()
 +{
 +   local path
 +
 +   while read -r path; do
 +   case $path in
 +   ?*/*) echo ${path%%/*}/ ;;
 +   *) echo $path ;;
 +   esac
 +   done
 +}
 +
 +__git_index_file_list_filter_bash ()
 +{
 +   local path
 +
 +   while read -r path; do
 +   case $path in
 +   ?*/*)
 +   # XXX if we append a slash to directory names when 
 using
 +   # `compopt -o filenames`, Bash will append another 
 slash.
 +   # This is pretty stupid, and this the reason why we 
 have to
 +   # define a compatible version for this function.
 +   echo ${path%%/*} ;;
 
 Which version of bash is that? It works perfectly fine here with or
 without the /.
 

GNU bash, version 4.1.5(1)-release (i486-pc-linux-gnu)
on a GNU Linux Debian 6

 +# __git_index_files accepts 1 or 2 arguments:
 +# 1: Options to pass to ls-files (required).
 +#Supported options are --cached, --modified, --deleted, --others,
 +#and --directory.
 +# 2: A directory path (optional).
 +#If provided, only files within the specified directory are listed.
 +#Sub directories are never recursed.  Path must have a trailing
 +#slash.
 +__git_index_files ()
 +{
 +   local dir=$(__gitdir) root=${2-.}
 +
 +   if [ -d $dir ]; then
 +   __git_ls_files_helper $root $1 | 
 __git_index_file_list_filter |
 +   sort | uniq
 +   fi
 +}
 +
 +# __git_diff_index_files accepts 1 or 2 arguments:
 +# 1) The id of a tree object.
 +# 2) A directory path (optional).
 +#If provided, only files within the specified directory are listed.
 +#Sub directories are never recursed.  Path must have a trailing
 +#slash.
 +__git_diff_index_files ()
 +{
 +   local dir=$(__gitdir) root=${2-.}
 +
 +   if [ -d $dir ]; then
 +   __git_diff_index_helper $root $1 | 
 __git_index_file_list_filter |
 +   sort | uniq
 +   fi
 +}
 
 These two are exactly the same, except one calls
 __git_ls_files_helper, and the other one __git_diff_index_helper,
 can't we make another argument that is and select one or the other
 based on that?
 

They are not exactly the same.

The first function requires, as first parameter, (space separed) options
to pass to ls-files command; the second function, instead, requires the
id of a tree object.

IMHO, using only one function may be confusing.

  __git_heads ()
  {
 local dir=$(__gitdir)
 @@ -430,6 +543,46 @@ __git_complete_revlist_file ()
  }


 +# __git_complete_index_file requires 1 argument: the options to pass to
 +# ls-file
 +__git_complete_index_file ()
 +{
 +   local pfx cur_=$cur
 +
 +   case $cur_ in
 +   ?*/*)
 +   pfx=${cur_%/*}
 +   cur_=${cur_##*/}
 +   pfx=${pfx}/
 +
 +   __gitcomp_file $(__git_index_files $1 $pfx) $pfx 
 $cur_
 +   ;;
 +   *)
 +   __gitcomp_file $(__git_index_files $1)  $cur_
 +   ;;
 +   esac
 +}
 +
 +# __git_complete_diff_index_file requires 1 argument: the id of a tree
 +# object
 +__git_complete_diff_index_file ()
 +{
 +   local pfx cur_=$cur
 +
 +   case $cur_ in
 +   ?*/*)
 +   pfx=${cur_%/*}
 +   cur_=${cur_##*/}
 +   pfx=${pfx}/
 +
 +   __gitcomp_file $(__git_diff_index_files $1 $pfx) 
 $pfx $cur_
 +   ;;
 +   *)
 +   __gitcomp_file $(__git_diff_index_files $1)  $cur_
 +   ;;
 +   esac
 +}
 
 These are also exactly the same, we could pass the argument to the
 function above.
 

See previous note.



Regards  Manlio Perillo
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlF2p+QACgkQscQJ24LbaUQVcACfeYFO8umJDbgTrXWChqqbk69E
CE4AniZFP7PQkOZCbBY+6hZ2gMpNIJTn
=HqAf
-END PGP SIGNATURE-
--
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] git-completion.bash: add support for path completion

2013-01-13 Thread Junio C Hamano
Manlio Perillo manlio.peri...@gmail.com writes:

 +# Skip git (first argument)
 +for ((i=1; i  ${#words[@]}; i++)); do
 +word=${words[i]}
 +
 +case $word in
 +--)

 Sorry, I have incorrectly (again) indented the case labels.
 I have now configured my editor to correctly indent this.

Yeah, thanks for spotting.

I wouldn't worry *too* much about the style in this script at this
point, though.  It uses a style on its own that is totally different
from the rest of the system (e.g. [ instead of test, semicolon
in if ...; then, etc.) and it probably is better to emulate the
surrounding code, and leave the style fixes to a separate topic,
if we want to (as a contrib/ material that is not POSIX but bash
specific, I do not know if that is even worth it).

 +# Good; we can assume that the following are 
 only non
 +# option arguments.
 +((c = 0))
 +;;

 Here I was thinking to do something like this (not tested):

   -*)
   if [ -n ${2-} ]; then
   # Assume specified git command only
 # accepts simple options
   # (without arguments)
   ((c = 0))

 Since git mv only accepts simple options, this will make the use of '--'
 not required.

Unless you have a file whose name begins with a dash, perhaps?
--
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] git-completion.bash: add support for path completion

2013-01-12 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 11/01/2013 19:48, Manlio Perillo ha scritto:
 The git-completion.bash script did not implemented full, git aware,
 support to complete paths, for git commands that operate on files within
 the current working directory or the index.
 [...]
  
 +# Try to count non option arguments passed on the command line for the
 +# specified git command.
 +# When options are used, it is necessary to use the special -- option to
 +# tell the implementation were non option arguments begin.
 +# XXX this can not be improved, since options can appear everywhere, as
 +# an example:
 +#git mv x -n y
 +#
 +# __git_count_arguments requires 1 argument: the git command executed.
 +__git_count_arguments ()
 +{
 + local word i c=0
 +
 + # Skip git (first argument)
 + for ((i=1; i  ${#words[@]}; i++)); do
 + word=${words[i]}
 +
 + case $word in
 + --)

Sorry, I have incorrectly (again) indented the case labels.
I have now configured my editor to correctly indent this.

 + # Good; we can assume that the following are 
 only non
 + # option arguments.
 + ((c = 0))
 + ;;

Here I was thinking to do something like this (not tested):

-*)
if [ -n ${2-} ]; then
# Assume specified git command only
# accepts simple options
# (without arguments)
((c = 0))

Since git mv only accepts simple options, this will make the use of '--'
not required.

Note that I'm assuming the single '-' character is used as a non-option
argument; not sure this is the case of Git.

 [...]


Regards  Manlio
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDxXLkACgkQscQJ24LbaUR+QQCaA4WZP5h5lktXJqSB7c494fAY
B6IAoIRWyIzBq29S7+l+TfRjbyp19HNL
=JRpR
-END PGP SIGNATURE-
--
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] git-completion.bash: add support for path completion

2013-01-12 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 11/01/2013 23:02, Junio C Hamano ha scritto:
 Manlio Perillo manlio.peri...@gmail.com writes:
 
 +# Process path list returned by ls-files and diff-index --name-only
 +# commands, in order to list only file names relative to a specified
 +# directory, and append a slash to directory names.
 +__git_index_file_list_filter ()
 +{
 +# Default to Bash = 4.x
 +__git_index_file_list_filter_bash
 +}
 +
 +# Execute git ls-files, returning paths relative to the directory
 +# specified in the first argument, and using the options specified in
 +# the second argument.
 +__git_ls_files_helper ()
 +{
 +# NOTE: $2 is not quoted in order to support multiple options
 +cd $1  git ls-files --exclude-standard $2
 +} 2/dev/null
 
 I think this redirection is correct but a bit tricky;

It's not tricky: it is POSIX:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_10


 it is in
 effect during the execution of the { block } (in other words, it is
 not about squelching errors during the function definition).
 

What do you mean by squelching?

Note that I originally wrote the code as

__git_ls_files_helper ()
{
# NOTE: $2 is not quoted in order to support multiple options
 { cd $1  git ls-files --exclude-standard $2 } 2/dev/null
}

but then I checked the POSIX standard, noting that it is redundant.

 -- 8 --
 #!/bin/sh
 cat t.sh \EOF 
 echo I am $1
 t () { echo Goes to stdout; echo 2 Goes to stderr; } 2/dev/null
 t
 for sh in bash dash ksh zsh
 do
   $sh t.sh $sh
 done
 -- 8 --
 

There is a missing EOF delimiter.
And I'm not sure to understand the meaning of  after EOF.

 Bash does (so do dash and real ATT ksh) grok this correctly, but
 zsh does not seem to (I tried zsh 4.3.10 and 4.3.17; also zsh
 pretending to be ksh gets this wrong as well).  Not that what ksh
 does matters, as it won't be dot-sourcing bash completion script.


I have added tcsh to the sh list, but it fails with:
Badly placed ()'s.


 It however may affect zsh, which does seem to dot-source this file.
 Perhaps zsh completion may have to be rewritten in a similar way as
 tcsh completion is done (i.e. does not dot-source this file but ask
 bash to do the heavy-lifting).
 

Ok, I was wrong on assuming all modern shells were POSIX compliant.
I will change the code to use a nested {} group.

 This function seems to be always called in an subshell (e.g. as an
 upstream of a pipeline), so the cd may be harmless, but don't you
 need to disable CDPATH while doing this?
 

I don't know.

 [..]


 +# Try to count non option arguments passed on the command line for the
 +# specified git command.
 +# When options are used, it is necessary to use the special -- option to
 +# tell the implementation were non option arguments begin.
 +# XXX this can not be improved, since options can appear everywhere, as
 +# an example:
 +#   git mv x -n y
 
 If that is the case, it is a bug in the command line parser, I
 think.  We should reject it, and the command line completer
 certainly should not encourage it.
 

$ mkdir y
$ git mv x -n y
Checking rename of 'x' to 'y/x'
Renaming x to y/x
$ git status
# On branch master
nothing to commit, working directory clean

I was assuming it to be normal, given how complex Git command line
parsing is (IMHO).


Thanks  Manlio
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDxeMgACgkQscQJ24LbaUTmaQCeMbZ0lRJxZIx3U31gMPmcqTLp
54sAmwYrjJVuvRYcsbGaMa3rb9/EKrBU
=ky30
-END PGP SIGNATURE-
--
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


[PATCH v5] git-completion.bash: add support for path completion

2013-01-11 Thread Manlio Perillo
The git-completion.bash script did not implemented full, git aware,
support to complete paths, for git commands that operate on files within
the current working directory or the index.

As an example:

git add TAB

will suggest all files in the current working directory, including
ignored files and files that have not been modified.

Support path completion, for git commands where the non-option arguments
always refer to paths within the current working directory or the index,
as follows:

* the path completion for the git rm and git ls-files
  commands will suggest all cached files.

* the path completion for the git add command will suggest all
  untracked and modified files.  Ignored files are excluded.

* the path completion for the git clean command will suggest all
  untracked files.  Ignored files are excluded.

* the path completion for the git mv command will suggest all cached
  files when expanding the first argument, and all untracked and cached
  files for subsequent arguments.  In the latter case, empty directories
  are included and ignored files are excluded.

* the path completion for the git commit command will suggest all
  files that have been modified from the HEAD, if HEAD exists, otherwise
  it will suggest all cached files.

For all affected commands, completion will always stop at directory
boundary.  Only standard ignored files are excluded, using the
--exclude-standard option of the ls-files command.

When using a recent Bash version, Git path completion will be the same
as builtin file completion, e.g.

git add contrib/

will suggest relative file names.

Signed-off-by: Manlio Perillo manlio.peri...@gmail.com
---

Changes:

* Applied Junio patch to fix completion inside a subdirectory.
* Quoted the hopefully last incorrectly unquoted variable.
* Fixed coding style (removed stdout file descriptor in shell
  redirection, since it is redundant).
* Fixed regression in path completion, when using non canonicalized
  or absolute path names.
  The problem has been solved making sure to chdir to the specified
  directory before executing ls-files and diff-index commands.

  The only issue is that there is no tilde expansion, but this is
  harmless, since default bash completion will be used (the old
  behaviour).
* Improved path completion when the new compopt builtin is available
  (Bash = 4.x).
  Now git paths completion is done in exactly the same way as Bash
  builtin filenames completion.
* Updated the zsh compatibility code to use the improved path
  completion support
* Fixed incorrect git mv arguments count used to check the first
  path to be renamed.
  When options are used (unless they are git main options), -- is
  required to separate options from non options arguments.
  It is harmless to not use --; in this case bash will suggest
  untracked files and directories for the first argument.

  XXX: should I add this implementation note in the commit message?
* Make sure to sort ls-files and diff-index filtered output before
  removing duplicate directories.
* Merged master.
 
Please note that before merging this patch in next, we need to update the
zsh and tcsh completion scripts.
I have the changes ready, but I will post them later since both scripts
needs more patches (I have posted an informal patch for zsh, and changes
to tcsh should be in pu, but I need to test them).

 contrib/completion/git-completion.bash | 250 ++---
 1 file changed, 234 insertions(+), 16 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index a4c48e1..51b8b3b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -13,6 +13,7 @@
 #*) .git/remotes file names
 #*) git 'subcommands'
 #*) tree paths within 'ref:path/to/file' expressions
+#*) file paths within current working directory and index
 #*) common --long-options
 #
 # To use these routines:
@@ -233,6 +234,118 @@ __gitcomp_nl ()
COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
 }
 
+# Generates completion reply with compgen from newline-separated possible
+# completion filenames.
+# It accepts 1 to 3 arguments:
+# 1: List of possible completion filenames, separated by a single newline.
+# 2: A directory prefix to be added to each possible completion filename
+#(optional).
+# 3: Generate possible completion matches for this word (optional).
+__gitcomp_file ()
+{
+   local IFS=$'\n'
+
+   # XXX does not work when the directory prefix contains a tilde,
+   # since tilde expansion is not applied.
+   # This means that COMPREPLY will be empty and Bash default
+   # completion will be used.
+   COMPREPLY=($(compgen -P ${2-} -W $1 

Re: [PATCH v5] git-completion.bash: add support for path completion

2013-01-11 Thread Junio C Hamano
Manlio Perillo manlio.peri...@gmail.com writes:

 +# Process path list returned by ls-files and diff-index --name-only
 +# commands, in order to list only file names relative to a specified
 +# directory, and append a slash to directory names.
 +__git_index_file_list_filter ()
 +{
 + # Default to Bash = 4.x
 + __git_index_file_list_filter_bash
 +}
 +
 +# Execute git ls-files, returning paths relative to the directory
 +# specified in the first argument, and using the options specified in
 +# the second argument.
 +__git_ls_files_helper ()
 +{
 + # NOTE: $2 is not quoted in order to support multiple options
 + cd $1  git ls-files --exclude-standard $2
 +} 2/dev/null

I think this redirection is correct but a bit tricky; it is in
effect during the execution of the { block } (in other words, it is
not about squelching errors during the function definition).

-- 8 --
#!/bin/sh
cat t.sh \EOF 
echo I am $1
t () { echo Goes to stdout; echo 2 Goes to stderr; } 2/dev/null
t
for sh in bash dash ksh zsh
do
$sh t.sh $sh
done
-- 8 --

Bash does (so do dash and real ATT ksh) grok this correctly, but
zsh does not seem to (I tried zsh 4.3.10 and 4.3.17; also zsh
pretending to be ksh gets this wrong as well).  Not that what ksh
does matters, as it won't be dot-sourcing bash completion script.

It however may affect zsh, which does seem to dot-source this file.
Perhaps zsh completion may have to be rewritten in a similar way as
tcsh completion is done (i.e. does not dot-source this file but ask
bash to do the heavy-lifting).

This function seems to be always called in an subshell (e.g. as an
upstream of a pipeline), so the cd may be harmless, but don't you
need to disable CDPATH while doing this?

 +# Execute git diff-index, returning paths relative to the directory
 +# specified in the first argument, and using the tree object id
 +# specified in the second argument.
 +__git_diff_index_helper ()
 +{
 + cd $1  git diff-index --name-only --relative $2
 +} 2/dev/null

Ditto.

 @@ -722,6 +875,43 @@ __git_has_doubledash ()
   return 1
  }
  
 +# Try to count non option arguments passed on the command line for the
 +# specified git command.
 +# When options are used, it is necessary to use the special -- option to
 +# tell the implementation were non option arguments begin.
 +# XXX this can not be improved, since options can appear everywhere, as
 +# an example:
 +#git mv x -n y

If that is the case, it is a bug in the command line parser, I
think.  We should reject it, and the command line completer
certainly should not encourage it.

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