Re: [PATCH] Handle path completion and colon for tcsh script

2013-02-03 Thread Manlio Perillo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 02/02/2013 21:10, Junio C Hamano ha scritto:
 Marc Khouzam marc.khou...@ericsson.com writes:
 
 Recent enhancements to git-completion.bash provide
 intelligent path completion for git commands.  Such
 completions do not add the '/' at the end of directories
 for recent versions of bash.
 ...
 Here is the update for tcsh completion which is needed to handle
 the cool new path completion feature just pushed to 'next'.

 [...]
 But I have to wonder if this is sweeping a problem under the rug.
 Shouldn't the completion for bash users end completed directory name
 with '/', even if we didn't have to worry about tcsh?
 

The problem is that when using the new
`compopt -o filenames` command, Bash assumes COMPREPLY contains a list
of filenames, and when it detects a directory name, it adds a slash.

The problem is, if the directory name *already* has a slash, Bash adds
another slash!

I don't know if this can be considered a bug or a feature.


Regards  Manlio

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

iEYEARECAAYFAlEOwaQACgkQscQJ24LbaUSjwgCfbgb1id5DcNG0Q75FWwgNPCqb
qkUAnAmMzCahB745/BWeDJTHbJFXucxs
=vf+P
-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] Handle path completion and colon for tcsh script

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

 The problem is that when using the new
 `compopt -o filenames` command, Bash assumes COMPREPLY contains a list
 of filenames, and when it detects a directory name, it adds a slash.

 The problem is, if the directory name *already* has a slash, Bash adds
 another slash!

So bash users do see the trailing slash because bash adds one to
what we compute and return, which we do strip the trailing slash
exactly because we know bash will add one.  Because tcsh completion
uses what we compute directly, without bash massaging our output to
add the trailing slash, it needs some magic.

OK, that makes sense.  It was this part from the originally proposed
log message:

 ... Such completions do not add the '/' at the end of directories
 for recent versions of bash.  However, the '/' is needed by tcsh,
 ...

with a large gap between the two sentences that fooled me, and the
explanation in your message helped to fill the gap to understand the
situation better.

Perhaps

... for recent versions of bash, which will then add the
trailing slash for paths that are directory to the result of
our completion.  The completion for tcsh however uses the
result of our completion directly, so it either needs to add
the necessary slash itself, or needs to ask us to keep the
trailiing slash.  This patch does the latter.

or something?


--
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] Handle path completion and colon for tcsh script

2013-02-03 Thread Marc Khouzam
  The problem is, if the directory name *already* has a slash, Bash adds
  another slash!
 
 So bash users do see the trailing slash because bash adds one to
 what we compute and return, which we do strip the trailing slash
 exactly because we know bash will add one.  

The problem is slightly more obscure than that, and I wonder if it
should be documented somewhere in the bash script?
Manlio explained in a previous
exchange with me, that bash will properly deal with an existing
trailing slash when doing the completion on the command-line, but
will screw it up by adding a second slash when dealing with multiple
possible completions and printing those for the user to choose from.

For example:

$ git status
# On branch tcsh_next
# Untracked files:
#   (use git add file... to include in what will be committed)
#
#   fish/
#   fishing/
nothing added to commit but untracked files present (use git add to track)

$ git add fishtab
fish//fishing// notice the double slashes

$ git add fishitab

# will complete the command line properly to the below, with a single slash.

$ git add fishing/



 Because tcsh completion
 uses what we compute directly, without bash massaging our output to
 add the trailing slash, it needs some magic.

Yes, that is right.

 OK, that makes sense.  It was this part from the originally proposed
 log message:
 
  ... Such completions do not add the '/' at the end of directories
  for recent versions of bash.  However, the '/' is needed by tcsh,
  ...
 
 with a large gap between the two sentences that fooled me, and the
 explanation in your message helped to fill the gap to understand the
 situation better.

Sorry about the lack of details.
I'm see more and more the importance of these commit messages :)

 Perhaps
 
 ... for recent versions of bash, which will then add the
 trailing slash for paths that are directory to the result of
 our completion.  The completion for tcsh however uses the
 result of our completion directly, so it either needs to add
 the necessary slash itself, or needs to ask us to keep the
 trailiing slash.  This patch does the latter.
 
 or something?

How about the following, which gives a little more detail about
the solution for tcsh?  I think it is worth putting the below extra
details because I feel a mistake could easily be made about this
trailing slash issue, which I had gotten wrong for my own version
of the script for a couple of weeks, before figuring out the mistake.


Handle path completion and colon for tcsh script

Recent enhancements to git-completion.bash provide
intelligent path completion for git commands.  Such
completions do not provide the '/' at the end of directories
for recent versions of bash; instead, bash itself will add the
trailing slash to directories to the result provided by 
git-completion.bash.  However, the completion for tcsh uses
the result of the bash completion script directly, so it either 
needs to add the necessary slash itself, or needs to ask the
bash script to keep the trailing slash.  Adding the slash itself
is difficult because we cannot easily tell if an entry of the 
output of the bash script is a directory or something else.
For example, assuming there is a directory named 'commit'
in the current directory, then, when completing
  git add committab
we would need to add a slash, but for
  git help committab
we should not.
Figuring out such differences would require adding much
intelligence to the tcsh completion script.  Instead, it is 
simpler to ask the bash script to keep the trailing slash.
This patch does this.

Also, tcsh does not handle the colon as a completion
separator so we remove it from the list of separators.

Signed-off-by: Marc Khouzam marc.khou...@ericsson.com

Thanks

Marc
--
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] Handle path completion and colon for tcsh script

2013-02-03 Thread Junio C Hamano
Thanks for a detailed explanation.  The two examples illustrating
different interpretation of the same word were really good.

Will replace and requeue.
--
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