threads / patch / 32812

patchHandle path completion and colon for tcsh script

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

## tl;dr

6 messages between Feb 2, 2013 and Feb 4, 2013. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Marc Khouzam· Feb 2, 2013, 19:43 UTC · lore

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. However, the '/' is needed by tcsh, so we must tell the bash script to append it by using a compatibility method available for older bash versions.

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.khouzam@ericsson.com>
---
Hi,

Here is the update for tcsh completion which is needed to handle the cool new path completion feature just pushed to 'next'.

Also, Manlio reported that tcsh completion was broken when using the colon, and this patch fixes the issue.

I haven't quite figured out the process to indicate which branch a patch is meant for. Do I just mention it in the email? Or are all patches meant for the 'pu' branch? In this case, 'pu' or 'next' would be appropriate.

Thanks!
Marc
 contrib/completion/git-completion.tcsh | 12 ++++++++++++
 1 file changed, 12 insertions(+)
Show changes to contrib/completion/git-completion.tcsh +12 −0
diff --git a/contrib/completion/git-completion.tcsh b/contrib/completion/git-completion.tcsh
index 3e3889f..eaacaf0 100644
--- a/contrib/completion/git-completion.tcsh
+++ b/contrib/completion/git-completion.tcsh
@@ -52,6 +52,18 @@ cat << EOF > ${__git_tcsh_completion_script}
 
 source ${__git_tcsh_completion_original_script}
 
+# Remove the colon as a completion separator because tcsh cannot handle it
+COMP_WORDBREAKS=\${COMP_WORDBREAKS//:}
+
+# For file completion, tcsh needs the '/' to be appended to directories.
+# By default, the bash script does not do that.
+# We can achieve this by using the below compatibility
+# method of the git-completion.bash script.
+__git_index_file_list_filter ()
+{
+	__git_index_file_list_filter_compat
+}
+
 # Set COMP_WORDS in a way that can be handled by the bash script.
 COMP_WORDS=(\$2)
 
-- 
1.8.1.367.g8e14972.dirty
Junio C Hamano· Feb 2, 2013, 20:10 UTC · re: Marc Khouzam · lore

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

Marc Khouzam <marc.khouzam@ericsson.com> writes:
Show 13 quoted lines
> 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'.
>
> Also, Manlio reported that tcsh completion was broken when using
> the colon, and this patch fixes the issue.
>
> I haven't quite figured out the process to indicate which branch
> a patch is meant for.  Do I just mention it in the email?

Yes, instead of wondering "Do I mention it here?", saying "This should come on top of Manlio's completion update." is good.

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?

Manlio Perillo· Feb 3, 2013, 19:59 UTC · re: Junio C Hamano · lore

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 02/02/2013 21:10, Junio C Hamano ha scritto:
Show 15 quoted lines
> Marc Khouzam <marc.khouzam@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-----

Junio C Hamano· Feb 3, 2013, 20:43 UTC · re: Manlio Perillo · lore

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

Manlio Perillo <manlio.perillo@gmail.com> writes:
Show 6 quoted lines
> 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?
Marc Khouzam· Feb 4, 2013, 02:50 UTC · re: Junio C Hamano · lore

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

Show 6 quoted lines
> > 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 fish<tab> fish// fishing// <-------- notice the double slashes

$ git add fishi<tab>
# 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.
Show 10 quoted lines
> 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 :)

Show 10 quoted lines
> 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 commit<tab>
    we would need to add a slash, but for
      git help commit<tab>
    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.khouzam@ericsson.com>
Thanks
Marc
Junio C Hamano· Feb 4, 2013, 03:02 UTC · re: Marc Khouzam · lore

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

Thanks for a detailed explanation. The two examples illustrating different interpretation of the same word were really good.

Will replace and requeue.

← back to recent threads