Re: Fwd: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
Hi,
On Mon, Nov 12, 2012 at 03:07:46PM -0500, Marc Khouzam wrote:
Hi,
[...]
Signed-off-by: Marc Khouzam marc.khou...@gmail.com
[...]
Thanks
Marc
---
contrib/completion/git-completion.bash | 53
+++-
contrib/completion/git-completion.tcsh | 34
2 files changed, 86 insertions(+), 1 deletions(-)
create mode 100755 contrib/completion/git-completion.tcsh
Please have a look at Documentation/SubmittingPatches to see how to
properly format the commit message, i.e. no greeting and sign-off in
the commit message part, and the S-o-b line should be the last before
the '---'.
Your patch seems to be severely line-wrapped. That document also
contains a few MUA-specific tips to help avoid that.
Other than that, it's a good description of the changes and
considerations. I agree that this approach seems to be the best from
the three.
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index be800e0..6d4b57a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,4 +1,6 @@
-#!bash
+#!/bin/bash
+# The above line is important as this script can be executed when used
+# with another shell such as tcsh
See comment near the end.
#
# bash/zsh completion support for core Git.
#
@@ -2481,3 +2483,52 @@ __git_complete gitk __gitk_main
if [ Cygwin = $(uname -o 2/dev/null) ]; then
__git_complete git.exe __git_main
fi
+
+# Method that will output the result of the completion done by
+# the bash completion script, so that it can be re-used in another
+# context than the bash complete command.
+# It accepts 1 to 2 arguments:
+# 1: The command-line to complete
+# 2: The index of the word within argument #1 in which the cursor is
+#located (optional). If parameter 2 is not provided, it will be
+#determined as best possible using parameter 1.
+_git_complete_with_output ()
+{
+ # Set COMP_WORDS to the command-line as bash would.
+ COMP_WORDS=($1)
That comment is only true for older Bash versions. Since v4 Bash
splits the command line at characters that the readline library treats
as word separators when performing word completion. But the
completion script has functions to deal with both, so this shouldn't
be a problem.
+
+ # Set COMP_CWORD to the cursor location as bash would.
+ if [ -n $2 ]; then
+ COMP_CWORD=$2
+ else
+ # Assume the cursor is at the end of parameter #1.
+ # We must check for a space as the last character which will
+ # tell us that the previous word is complete and the cursor
+ # is on the next word.
+ if [ ${1: -1} == ]; then
+ # The last character is a space, so our
location is at the end
+ # of the command-line array
+ COMP_CWORD=${#COMP_WORDS[@]}
+ else
+ # The last character is not a space, so our
location is on the
+ # last word of the command-line array, so we
must decrement the
+ # count by 1
+ COMP_CWORD=$((${#COMP_WORDS[@]}-1))
+ fi
+ fi
+
+ # Call _git() or _gitk() of the bash script, based on the first
+ # element of the command-line
+ _${COMP_WORDS[0]}
+
+ # Print the result that is stored in the bash variable ${COMPREPLY}
Really? ;)
I like the above comments about setting COMP_CWORD, because they
explain why you do what you do, which would be otherwise difficult to
figure out. But telling that an echo in a for loop over an array
prints that array is, well, probably not necessary.
+ for i in ${COMPREPLY[@]}; do
+ echo $i
+ done
There is no need for the loop here to print the array one element per
line:
local IFS=$'\n'
echo ${COMPREPLY[*]}
+}
+
+if [ -n $1 ] ; then
+ # If there is an argument, we know the script is being executed
+ # so go ahead and run the _git_complete_with_output function
+ _git_complete_with_output $1 $2
Where does the second argument come from? Below you run this script
as '${__git_tcsh_completion_script} ${COMMAND_LINE}', i.e. $2 is
never set. Am I missing something?
+fi
diff --git a/contrib/completion/git-completion.tcsh
b/contrib/completion/git-completion.tcsh
new file mode 100755
index 000..7b7baea
--- /dev/null
+++ b/contrib/completion/git-completion.tcsh
@@ -0,0 +1,34 @@
+#!tcsh
+#
+# tcsh completion support for core Git.
+#
+# Copyright (C) 2012 Marc Khouzam marc.khou...@gmail.com
+# Distributed under the GNU General Public License, version 2.0.
+#
+# This script makes use of the git-completion.bash script to
+# determine the proper completion for git commands under tcsh.
+#
+# To use this completion script:
+#
+#1) Copy both
Re: Fwd: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
Thanks for the review.
On Tue, Nov 13, 2012 at 6:14 AM, SZEDER Gábor sze...@ira.uka.de wrote:
Hi,
On Mon, Nov 12, 2012 at 03:07:46PM -0500, Marc Khouzam wrote:
Hi,
[...]
Signed-off-by: Marc Khouzam marc.khou...@gmail.com
[...]
Thanks
Marc
---
contrib/completion/git-completion.bash | 53
+++-
contrib/completion/git-completion.tcsh | 34
2 files changed, 86 insertions(+), 1 deletions(-)
create mode 100755 contrib/completion/git-completion.tcsh
Please have a look at Documentation/SubmittingPatches to see how to
properly format the commit message, i.e. no greeting and sign-off in
the commit message part, and the S-o-b line should be the last before
the '---'.
Sorry about that, since I should have noticed it in the doc.
I will do my best to address all submission issues mentioned
when I post the next version of the patch.
Your patch seems to be severely line-wrapped. That document also
contains a few MUA-specific tips to help avoid that.
Other than that, it's a good description of the changes and
considerations. I agree that this approach seems to be the best from
the three.
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index be800e0..6d4b57a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,4 +1,6 @@
-#!bash
+#!/bin/bash
+# The above line is important as this script can be executed when used
+# with another shell such as tcsh
See comment near the end.
Great suggestion below. I removed the above change.
+ # Set COMP_WORDS to the command-line as bash would.
+ COMP_WORDS=($1)
That comment is only true for older Bash versions. Since v4 Bash
splits the command line at characters that the readline library treats
as word separators when performing word completion. But the
completion script has functions to deal with both, so this shouldn't
be a problem.
I've updated the comment to be more general and left the code
the same since it is supported by the script.
+ # Print the result that is stored in the bash variable ${COMPREPLY}
Really? ;)
Removed :)
+ for i in ${COMPREPLY[@]}; do
+ echo $i
+ done
There is no need for the loop here to print the array one element per
line:
local IFS=$'\n'
echo ${COMPREPLY[*]}
Better. Thanks.
+if [ -n $1 ] ; then
+ # If there is an argument, we know the script is being executed
+ # so go ahead and run the _git_complete_with_output function
+ _git_complete_with_output $1 $2
Where does the second argument come from? Below you run this script
as '${__git_tcsh_completion_script} ${COMMAND_LINE}', i.e. $2 is
never set. Am I missing something?
This second argument is optional and, if present, will be put in
$COMP_CWORD. If not present, $COMP_CWORD must be computed
from $1. Also see comment above _git_complete_with_output ().
tcsh does not provide me with this information, so I cannot make use of it.
However, I thought it would be more future-proof to allow it for other shells
which may have that information.
It is not necessary for tcsh, so I can remove if you prefer?
+# Make the script executable if it is not
+if ( ! -x ${__git_tcsh_completion_script} ) then
+ chmod u+x ${__git_tcsh_completion_script}
+endif
Not sure about this. If I source a script to provide completion for a
command, then I definitely don't expect it to change file permissions.
However, even if the git completion script is not executable, you can
still run it with 'bash ${__git_tcsh_completion_script}'. This way
neither the user would need to set permissions, not the script would
need to set it behind the users back. Furthermore, this would also
make changing the shebang line unnecessary.
Very nice! Done.
+complete git 'p/*/`${__git_tcsh_completion_script} ${COMMAND_LINE}
| sort | uniq`/'
+complete gitk 'p/*/`${__git_tcsh_completion_script} ${COMMAND_LINE}
| sort | uniq`/'
Is the 'sort | uniq' really necessary? After the completion function
returns Bash automatically sorts the elements in COMPREPLY and removes
any duplicates. Doesn't tcsh do the same? I have no idea about tcsh
completion.
On my machine, tcsh does not remove duplicates. It does sort the results
but that is done after I've run 'uniq', which is too late. I'm not
happy about this
either, but the other option is to improve git-completion.bash to
avoid duplicates,
which seemed less justified.
Does the git completion script returns any duplicates at all?
It does. 'help' is returned twice for example.
Also, when completing 'git checkout ' in the git repo, I can see multiple
'todo' branches, as well as 'master', 'pu', 'next', etc.
You can actually try it without tcsh by running my proposed version of
git-completion.bash like this:
cd git/contrib/completion
bash git-completion.bash git checkout | sort |
Re: Fwd: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
Hi,
On Tue, Nov 13, 2012 at 03:12:44PM -0500, Marc Khouzam wrote:
+if [ -n $1 ] ; then
+ # If there is an argument, we know the script is being executed
+ # so go ahead and run the _git_complete_with_output function
+ _git_complete_with_output $1 $2
Where does the second argument come from? Below you run this script
as '${__git_tcsh_completion_script} ${COMMAND_LINE}', i.e. $2 is
never set. Am I missing something?
This second argument is optional and, if present, will be put in
$COMP_CWORD. If not present, $COMP_CWORD must be computed
from $1. Also see comment above _git_complete_with_output ().
tcsh does not provide me with this information, so I cannot make use of it.
However, I thought it would be more future-proof to allow it for other shells
which may have that information.
It is not necessary for tcsh, so I can remove if you prefer?
I see. I read those comments and understood what it is about. I was
just surprised that the code is there to make use of it, yet it's not
specified when invoking that function.
Since it's a trivial piece of code, I would say let's keep it. Could
you please add a sentence about it (that it's for possible future
users and it's not used at the moment) to the commit message for
future reference?
+complete git 'p/*/`${__git_tcsh_completion_script} ${COMMAND_LINE}
| sort | uniq`/'
+complete gitk 'p/*/`${__git_tcsh_completion_script} ${COMMAND_LINE}
| sort | uniq`/'
Is the 'sort | uniq' really necessary? After the completion function
returns Bash automatically sorts the elements in COMPREPLY and removes
any duplicates. Doesn't tcsh do the same? I have no idea about tcsh
completion.
On my machine, tcsh does not remove duplicates. It does sort the results
but that is done after I've run 'uniq', which is too late. I'm not
happy about this
either, but the other option is to improve git-completion.bash to
avoid duplicates,
which seemed less justified.
Ok. Then keep it for the time being, and we'll see what we can do to
avoid those duplicates.
Does the git completion script returns any duplicates at all?
It does. 'help' is returned twice for example.
Right. Now that you mentioned it, I remember I noticed it a while
ago, too. I even wrote a patch to fix it, but not sure what became of
it. Will try to dig it up.
Also, when completing 'git checkout ' in the git repo, I can see multiple
'todo' branches, as well as 'master', 'pu', 'next', etc.
You can actually try it without tcsh by running my proposed version of
git-completion.bash like this:
cd git/contrib/completion
bash git-completion.bash git checkout | sort | uniq --repeated
Interesting, I can't reproduce. Are the duplicates also there, if you
start a bash, source git-completion.bash, and run __git_refs ?
--
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: Fwd: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
Hi,
I've got two more comments.
On Mon, Nov 12, 2012 at 03:07:46PM -0500, Marc Khouzam wrote:
@@ -2481,3 +2483,52 @@ __git_complete gitk __gitk_main
if [ Cygwin = $(uname -o 2/dev/null) ]; then
__git_complete git.exe __git_main
fi
+
+# Method that will output the result of the completion done by
+# the bash completion script, so that it can be re-used in another
+# context than the bash complete command.
+# It accepts 1 to 2 arguments:
+# 1: The command-line to complete
+# 2: The index of the word within argument #1 in which the cursor is
+#located (optional). If parameter 2 is not provided, it will be
+#determined as best possible using parameter 1.
+_git_complete_with_output ()
We differentiate between _git_whatever() and __git_whatever()
functions. The former performs completion for the 'whatever' git
command/alias, the latter is a completion helper function. This
is a helper function, so it should begin with double underscores.
+{
+ # Set COMP_WORDS to the command-line as bash would.
+ COMP_WORDS=($1)
+
+ # Set COMP_CWORD to the cursor location as bash would.
+ if [ -n $2 ]; then
A while ago the completion script was made 'set -u'-clean. (If 'set
-u' is enabled, then it's an error to access undefined variables).
I'm not sure how many people are out there who'd use this script for
tcsh while having 'set -u' in their profile... probably not that
many. Still, I think it would be great to keep it up.
Here $2 would be undefined, so accessingit it would cause an error
under those semantincs. Please use ${2-} instead (use empty string
when undefined).
+if [ -n $1 ] ; then
Same here.
+ # If there is an argument, we know the script is being executed
+ # so go ahead and run the _git_complete_with_output function
+ _git_complete_with_output $1 $2
And here.
Thanks
Gábor
--
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: Fwd: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
On Tue, Nov 13, 2012 at 6:46 PM, SZEDER Gábor sze...@ira.uka.de wrote:
Hi,
On Tue, Nov 13, 2012 at 03:12:44PM -0500, Marc Khouzam wrote:
+if [ -n $1 ] ; then
+ # If there is an argument, we know the script is being executed
+ # so go ahead and run the _git_complete_with_output function
+ _git_complete_with_output $1 $2
Where does the second argument come from? Below you run this script
as '${__git_tcsh_completion_script} ${COMMAND_LINE}', i.e. $2 is
never set. Am I missing something?
This second argument is optional and, if present, will be put in
$COMP_CWORD. If not present, $COMP_CWORD must be computed
from $1. Also see comment above _git_complete_with_output ().
tcsh does not provide me with this information, so I cannot make use of it.
However, I thought it would be more future-proof to allow it for other shells
which may have that information.
It is not necessary for tcsh, so I can remove if you prefer?
I see. I read those comments and understood what it is about. I was
just surprised that the code is there to make use of it, yet it's not
specified when invoking that function.
Since it's a trivial piece of code, I would say let's keep it. Could
you please add a sentence about it (that it's for possible future
users and it's not used at the moment) to the commit message for
future reference?
Will do.
+complete git 'p/*/`${__git_tcsh_completion_script} ${COMMAND_LINE}
| sort | uniq`/'
+complete gitk 'p/*/`${__git_tcsh_completion_script} ${COMMAND_LINE}
| sort | uniq`/'
Is the 'sort | uniq' really necessary? After the completion function
returns Bash automatically sorts the elements in COMPREPLY and removes
any duplicates. Doesn't tcsh do the same? I have no idea about tcsh
completion.
On my machine, tcsh does not remove duplicates. It does sort the results
but that is done after I've run 'uniq', which is too late. I'm not
happy about this
either, but the other option is to improve git-completion.bash to
avoid duplicates,
which seemed less justified.
Ok. Then keep it for the time being, and we'll see what we can do to
avoid those duplicates.
Thanks.
Does the git completion script returns any duplicates at all?
It does. 'help' is returned twice for example.
Right. Now that you mentioned it, I remember I noticed it a while
ago, too. I even wrote a patch to fix it, but not sure what became of
it. Will try to dig it up.
Thanks for already posting the patch.
Also, when completing 'git checkout ' in the git repo, I can see multiple
'todo' branches, as well as 'master', 'pu', 'next', etc.
You can actually try it without tcsh by running my proposed version of
git-completion.bash like this:
cd git/contrib/completion
bash git-completion.bash git checkout | sort | uniq --repeated
Interesting, I can't reproduce. Are the duplicates also there, if you
start a bash, source git-completion.bash, and run __git_refs ?
Running __git_refs does not show the duplicates, but running
__git refs '' 1
does show them.
That second parameter causes __git_refs to
use the guess heuristic employed by checkout for tracking branches
I don't quite understand this, but what I can see is that my remote
branches GitHub/master and origin/master each cause another
'master' to be listed:
$ __git_refs '' 1|grep master
master
GitHub/master
origin/master
master
master
All fixes are done and I'll post a second version of the patch
as soon as I can figure out the formatting properly.
Thanks again
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
