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

2012-12-19 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 17/12/2012 20:42, Junio C Hamano ha scritto:
 [...]
 I am not sure how you would handle the last parameter to git mv,
 though.  That is by definition a path that does not exist,
 i.e. cannot be completed.

 Right, the code should be changed.
 No completion should be done for the second parameter.
 
 I deliberately wrote the last not the second, as you can do
 
   $ mkdir X
 $ git mv COPYING README X/.
 
 You do need to expand the second parameter to README when the user
 types
 
   git mv COPYING REAMDE X
 
 then goes back with \C-b to M, types \C-d three times to remove
 MDE, and then finally says TAB, to result in
 
   git mv COPYING README X
 

Assuming X is a new untracked directory, do you think it is an usability
problem if an user try to do:

git mv COPYING README TAB

and X does not appear in the completion list?

As far as I know, the solution is to only support custom expansion the
first parameter, unless the user will do something like:

git mv COPYING README -- TAB


Regards  Manlio

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

iEYEARECAAYFAlDSOWAACgkQscQJ24LbaUSOnACfds93RtX1CDOeGbwCGM5/N8HI
yVwAn0AZEO6rE083gKgFimGIbiRTyN5Z
=z7K5
-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] git-completion.bash: add support for path completion

2012-12-19 Thread Junio C Hamano
Manlio Perillo manlio.peri...@gmail.com writes:

  git mv COPYING README X

 Assuming X is a new untracked directory, do you think it is an usability
 problem if an user try to do:

   git mv COPYING README TAB

 and X does not appear in the completion list?

It is hard to say.  Will it show Documentation/ in the list?  Will
it show git.c in the list?

Your git mv git.TAB completing to git mv git.c would be an
improvement compared to the stock less git.TAB that offers
git.c and git.o as choices.  For things like mv and rm, this
may not make too much of a difference, git add TAB would be a
vast improvement from the stock one.  The users will notice that the
completion knows what it is doing, and will come to depend on it.

But at that point, if git mv COPYING README TAB offers only
directories that contain tracked files, the users may get irritated,
because it is likely that the destination directory was created
immediately before the user started typing git mv.  You will hear
Surely I created X, it is there, why aren't you showing it to me?

The updated completion knows what it is doing everywhere else, and
it sets the user-expectation at that level.  Uneven cleverness will
stand out like a sore thumb and hurts the user perception, which is
arguably unfair, but nothing in life is fair X-.

I think over-showing the choices is much better than hiding some
choices, if we cannot do a perfect completion in some cases.  You
should know that I won't be moving these files in Y, as I already
marked it to be ignored in the .gitignore file! is less grave a
gripe than You know I created X just a minute ago, and it is there,
why aren't you showing it to me? because you can simply say but Y
is there as a directory. admitting that you are less clever than
the user expects you to be, but at least you are giving the choice
to the user of not picking it.

In the ideal world (read: I am *not* saying you should implement it
this way, or we won't look at your patch), I think you would want
to show tracked files (because it may be the third path that the
user wants to move with the command, not the destination directory)
and any directory on the filesystem (it could be the third path that
is being moved if it has tracked files, or it could be the final
destination directory argument).
--
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] git-completion.bash: add support for path completion

2012-12-19 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 19/12/2012 23:49, Junio C Hamano ha scritto:
 Manlio Perillo manlio.peri...@gmail.com writes:
 
 git mv COPYING README X

 Assuming X is a new untracked directory, do you think it is an usability
 problem if an user try to do:

  git mv COPYING README TAB

 and X does not appear in the completion list?
 
 It is hard to say.  Will it show Documentation/ in the list?  Will
 it show git.c in the list?
 

Currently all cached files will be showed.

 Your git mv git.TAB completing to git mv git.c would be an
 improvement compared to the stock less git.TAB that offers
 git.c and git.o as choices.  For things like mv and rm, this
 may not make too much of a difference, git add TAB would be a
 vast improvement from the stock one.  The users will notice that the
 completion knows what it is doing, and will come to depend on it.
 

Yes, this is precisely the reason why I'm implementing it ;-).

I also use Mercurial (I discovered Git just a few weeks ago, after
reading Pro Git), and Mercurial do have path completion (completion list
does not stop at directory boundary, however).

When I started to use Git, one of the first thing I noticed was the lack
of path completion for git add.

 [...]
 In the ideal world (read: I am *not* saying you should implement it
 this way, or we won't look at your patch), I think you would want
 to show tracked files (because it may be the third path that the
 user wants to move with the command, not the destination directory)
 and any directory on the filesystem (it could be the third path that
 is being moved if it has tracked files, or it could be the final
 destination directory argument).

What about this?

* show all and only cached files for the first argument
* show all cached + untracked directories and files for all other
  arguments

This should solve most of the problem, and will still not show ignored
files.  And, most important, it is very easy to implement.


The only issue is that git ls-files -o does not show empty
directories, and git ls-files --directory -o will add a trailing slash.



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

iEYEARECAAYFAlDSUr4ACgkQscQJ24LbaURf0gCeJVZviwfKHgHNZ8vYBjnjwP8+
WF4AnAn3/KPJciJg9r387qIzCajx4j0s
=/10k
-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] git-completion.bash: add support for path completion

2012-12-18 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 17/12/2012 20:42, Junio C Hamano ha scritto:
 [...]
 I am not sure how you would handle the last parameter to git mv,
 though.  That is by definition a path that does not exist,
 i.e. cannot be completed.

 Right, the code should be changed.
 No completion should be done for the second parameter.
 
 I deliberately wrote the last not the second, as you can do
 
   $ mkdir X
 $ git mv COPYING README X/.
 

The patch is ready, however I decided to leave git mv completion simple.
Pressing TAB will always try to autocomplete using all cached files.
I have added a note to remember it needs more work.


P.S.:
git-completion.bash has a lot of other things that may be improved:

* adding missing commands
 (as an example, there is strangely no custom support fot git status)

* completion support for commands like git checkout is not complete.
  git checkout TAB will correctly try to complete the tree-ish,
  however git checkout HEAD -- TAB will try to complete the path
  using *all* files in the working directory.

  This is easy to fix, using the new functions I have added

* not all long options are supported.
  The script documentation says that only common long options are
  supported, so I'm not sure it is ok to add support for all available
  long options.


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

iEYEARECAAYFAlDQmQgACgkQscQJ24LbaUSw9QCfT1lCH/yjA4Lgmb2nMspNWM3l
hMMAn26UxWesuoOxMbuwhqaypPjkmN84
=Wh4c
-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] git-completion.bash: add support for path completion

2012-12-17 Thread Junio C Hamano
Manlio Perillo manlio.peri...@gmail.com writes:

 As long as all of the above stops completion at directory boundary,
 I think the above sounds like a sensible thing to do.  e.g. when
 ls-files gives Documentation/Makefile and Documentation/git.txt,
 git cmd DocTAB first would give git cmd Documentation/ and
 then the second TAB would offer these two paths as choices.  That
 way, the user can choose to just execute git cmd Documentation/
 without even looking at these individual paths.

 Right, this is what bash usually do.
 However I don't know how to implement this with git.

That sounds like a regression to me.

 I am not sure how you would handle the last parameter to git mv,
 though.  That is by definition a path that does not exist,
 i.e. cannot be completed.

 Right, the code should be changed.
 No completion should be done for the second parameter.

I deliberately wrote the last not the second, as you can do

$ mkdir X
$ git mv COPYING README X/.

You do need to expand the second parameter to README when the user
types

git mv COPYING REAMDE X

then goes back with \C-b to M, types \C-d three times to remove
MDE, and then finally says TAB, to result in

git mv COPYING README X

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

2012-12-16 Thread Junio C Hamano
Manlio Perillo manlio.peri...@gmail.com writes:

 The git-completion.bash script did not implemented full support for
 completion, for git commands that operate on files from the current
 working directory or the index.

 For these commands, only options completion was available.

Hrm, git mv COTAB completes it to COPYING for me.  Same for:

git rm COTAB
git clean DocTAB
git commit --edit DocTAB

There must be something missing from the above description, and the
claim the above two paragraphs make does no make sense without that
something that is missing.

 * the path completion for the git mv and git rm commands is provided
   using git ls-files --exclude-standard

Does the above mean git mv COPYING DocTAB would complete the
command line to move it to Documentation/ directory?  

I think using X is of secondary importance.  Reviewers and future
developers (who are reading git log output) can read it from the
patch.  What is expected in the log message is why the implemenation
was chosen, and in order to achieve what effect.

For example, something like:

Each parameter between the first one and the one before the last
one to 'git mv' is completed to a tracked path or a leading
directory of a tracked path

should come first to explain what your patch wanted to do.  It of
course is helpful to explain how you implemented that behaviour, by
appending , using 'ls-files --exclude-standard' at the end.

 * the path completion for the git add command is provided using
   git ls-files --exclude-standard -o -m

Likewise.  You are adding either a modified, or a new and
unignored and use ls-files --exclude-standard -o -m to achieve
it.

 * the path completion for the git clean command is provided using
   git ls-files --exclude-standard -o

 * the path completion for the git commit command is provides using
   git diff-index --name-only HEAD

As long as all of the above stops completion at directory boundary,
I think the above sounds like a sensible thing to do.  e.g. when
ls-files gives Documentation/Makefile and Documentation/git.txt,
git cmd DocTAB first would give git cmd Documentation/ and
then the second TAB would offer these two paths as choices.  That
way, the user can choose to just execute git cmd Documentation/
without even looking at these individual paths.

I am not sure how you would handle the last parameter to git mv,
though.  That is by definition a path that does not exist,
i.e. cannot be completed.

 Signed-off-by: Manlio Perillo manlio.peri...@gmail.com
 ---
  contrib/completion/git-completion.bash | 39 
 ++
  1 file changed, 26 insertions(+), 13 deletions(-)

 diff --git a/contrib/completion/git-completion.bash 
 b/contrib/completion/git-completion.bash
 index 0b77eb1..8b348c3 100644
 --- a/contrib/completion/git-completion.bash
 +++ b/contrib/completion/git-completion.bash
 @@ -233,6 +233,25 @@ __gitcomp_nl ()
   COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
  }
  
 +__git_files ()
 +{
 + local dir=$(__gitdir)
 + if [ -d $dir ]; then
 + git --git-dir=$dir ls-files --exclude-standard $*
 + return
 + fi
 +}
 +
 +# Return all staged files with modification from current HEAD
 +__git_commit_files ()
 +{
 + local dir=$(__gitdir)
 + if [ -d $dir ]; then
 + git --git-dir=$dir diff-index --name-only HEAD
 + return
 + fi
 +}
 +
  __git_heads ()
  {
   local dir=$(__gitdir)
 @@ -770,8 +789,6 @@ _git_apply ()
  
  _git_add ()
  {
 - __git_has_doubledash  return
 -
   case $cur in
   --*)
   __gitcomp 
 @@ -780,7 +797,8 @@ _git_add ()
   
   return
   esac
 - COMPREPLY=()
 + # XXX should we care for --update and --all options ?
 + __gitcomp_nl $(__git_files -o -m)  $cur 
  }
  
  _git_archive ()
 @@ -930,15 +948,14 @@ _git_cherry_pick ()
  
  _git_clean ()
  {
 - __git_has_doubledash  return
 -
   case $cur in
   --*)
   __gitcomp --dry-run --quiet
   return
   ;;
   esac
 - COMPREPLY=()
 + # TODO: check for -x option
 + __gitcomp_nl $(__git_files -o)  $cur 
  }
  
  _git_clone ()
 @@ -969,8 +986,6 @@ _git_clone ()
  
  _git_commit ()
  {
 - __git_has_doubledash  return
 -
   case $cur in
   --cleanup=*)
   __gitcomp default strip verbatim whitespace
 @@ -998,7 +1013,7 @@ _git_commit ()
   
   return
   esac
 - COMPREPLY=()
 + __gitcomp_nl $(__git_commit_files)  $cur 
  }
  
  _git_describe ()
 @@ -1362,7 +1377,7 @@ _git_mv ()
   return
   ;;
   esac
 - COMPREPLY=()
 + __gitcomp_nl $(__git_files)  $cur 
  }
  
  _git_name_rev ()
 @@ -2068,15 +2083,13 @@ _git_revert ()
  
  _git_rm ()
  {
 - __git_has_doubledash  return
 -
   case $cur in
   --*)
   __gitcomp --cached --dry-run --ignore-unmatch