Re: [PATCH] tcsh-completion re-using git-completion.bash

2012-11-20 Thread Marc Khouzam
On Sat, Nov 17, 2012 at 1:01 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:

 I gather that using a wrapper for zsh causes concerns about
 backwards-compatibility.

 I don't see any concerns.

 if [[ -n ${ZSH_VERSION-} ]]; then
   # replace below by zsh completion commands calling `bash
 ${HOME}/.git-completion.bash`

   complete git   'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
   complete gitk 'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'

 That doesn't work in zsh. It might be possible to do something
 similar, but it would probably require many more lines.

Hi,

since there doesn't seem to be an agreement that the approach to achieve tcsh
git-completion would be useful for zsh (the other possible shell that could use
it is ksh, but I haven't looked into that), maybe the simplest thing
is to keep the
tcsh solution contained in a tcsh-only script.  This is the latest solution as
proposed here:

[1] http://www.mail-archive.com/git@vger.kernel.org/msg12192.html

For reference, the more general solution was proposed here:
[2] http://www.mail-archive.com/git@vger.kernel.org/msg12122.html

If there is interest in merging [1], please let me know and I'll post another
version which adds a check to make sure that the user properly copied
git-completion.bash to be used by the new git-completion.tcsh.

Thanks for your input.

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] tcsh-completion re-using git-completion.bash

2012-11-20 Thread Junio C Hamano
Marc Khouzam marc.khou...@gmail.com writes:

 This one is already merged to 'next'.

 Awesome!  I didn't notice.

 If I want to suggest an improvement (like checking if the bash
 script is available), do I just post a patch here?

Yes, as a follow-up patch (or two).

Thanks.
--
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] tcsh-completion re-using git-completion.bash

2012-11-17 Thread SZEDER Gábor
On Fri, Nov 16, 2012 at 10:46:16PM +0100, Felipe Contreras wrote:
 On Fri, Nov 16, 2012 at 10:22 PM, SZEDER Gábor sze...@ira.uka.de wrote:
  On Fri, Nov 16, 2012 at 10:03:41PM +0100, Felipe Contreras wrote:
 
   As I understand the main issues with using the completion script with
   zsh are the various little incompatibilities between the two shells
   and bugs in zsh's emulation of Bash's completion-related builtins.
   Running the completion script under Bash and using its results in zsh
   would solve these issues at the root.  And would allow as to remove
   some if [[ -n ${ZSH_VERSION-} ]] code.
 
  We can remove that code already, because we now have code that is
  superior than zsh's bash completion emulation:
 
  http://article.gmane.org/gmane.comp.version-control.git/208173
 
  Which depends on the completion script having a wrapper function
  around compgen filling COMPREPLY.
 
 No, it does not. Previous incarnations didn't have this dependency:
 
 http://article.gmane.org/gmane.comp.version-control.git/196720

Good.

  However, COMPREPLY will be soon
  filled by hand-rolled code to prevent expansion issues with compgen,
  and there will be no such wrapper.
 
 I'm still waiting to see a resemblance of that code, but my bet would
 be that there will be a way to fill both COMPREPLY, and call zsh's
 compadd. But it's hard to figure that out without any code. Which is
 why I'm thinking on doing it myself.
 
 But even in that case, if push comes to shoves, this zsh wrapper can
 ultimately read COMPREPLY and figure things backwards, as even more
 previous versions did:
 
 http://article.gmane.org/gmane.comp.version-control.git/189310

Even better.  I was just going to propose that zsh's completion could
just read the contents of COMPREPLY at the end of _git() and _gitk(),
because this way no zsh-induced helper functions and changes would be
needed to the completion script at all.

However, running the completion script with Bash would also prevent
possible issues caused by incompatibilities between the two shells
mentioned below.

  This is the equivalent of what Marc is doing, except that zsh has no
  problems running bash's code. Note there's a difference with zsh's
  emulation bash (or rather bourne shell, or k shell), and zsh's
  emulation of bash's _completion_. The former is fine, the later is
  not.
 
  There are a couple of constructs supported by Bash but not by zsh,
  which we usually try to avoid.
 
 Yes, and is that a big deal?

Not that big, but I wanted to point out that it's not fine either.
Just a slight maintenance burden, because we have to pay attention not
to use such constructs.

--
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] tcsh-completion re-using git-completion.bash

2012-11-17 Thread Felipe Contreras
On Sat, Nov 17, 2012 at 11:56 AM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Fri, Nov 16, 2012 at 10:46:16PM +0100, Felipe Contreras wrote:

 But even in that case, if push comes to shoves, this zsh wrapper can
 ultimately read COMPREPLY and figure things backwards, as even more
 previous versions did:

 http://article.gmane.org/gmane.comp.version-control.git/189310

 Even better.  I was just going to propose that zsh's completion could
 just read the contents of COMPREPLY at the end of _git() and _gitk(),
 because this way no zsh-induced helper functions and changes would be
 needed to the completion script at all.

I would rather modify the __gitcomp function. Parsing COMPREPLY is too
cumbersome.

 However, running the completion script with Bash would also prevent
 possible issues caused by incompatibilities between the two shells
 mentioned below.

It could, but it doesn't now.

  This is the equivalent of what Marc is doing, except that zsh has no
  problems running bash's code. Note there's a difference with zsh's
  emulation bash (or rather bourne shell, or k shell), and zsh's
  emulation of bash's _completion_. The former is fine, the later is
  not.
 
  There are a couple of constructs supported by Bash but not by zsh,
  which we usually try to avoid.

 Yes, and is that a big deal?

 Not that big, but I wanted to point out that it's not fine either.
 Just a slight maintenance burden, because we have to pay attention not
 to use such constructs.

Do we have to pay attention?

I say when we encounter one of such maintenance burden issues _then_
we think about it. In the meantime for all we know sourcing bash's
script from zsh is fine.

-- 
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] tcsh-completion re-using git-completion.bash

2012-11-17 Thread SZEDER Gábor
On Sat, Nov 17, 2012 at 12:46:27PM +0100, Felipe Contreras wrote:
 On Sat, Nov 17, 2012 at 11:56 AM, SZEDER Gábor sze...@ira.uka.de wrote:
  On Fri, Nov 16, 2012 at 10:46:16PM +0100, Felipe Contreras wrote:
 
  But even in that case, if push comes to shoves, this zsh wrapper can
  ultimately read COMPREPLY and figure things backwards, as even more
  previous versions did:
 
  http://article.gmane.org/gmane.comp.version-control.git/189310
 
  Even better.  I was just going to propose that zsh's completion could
  just read the contents of COMPREPLY at the end of _git() and _gitk(),
  because this way no zsh-induced helper functions and changes would be
  needed to the completion script at all.
 
 I would rather modify the __gitcomp function. Parsing COMPREPLY is too
 cumbersome.

Each element of COMPREPLY contains a possible completion word.  What
parsing is needed to use that, that is so cumbersome?

  However, running the completion script with Bash would also prevent
  possible issues caused by incompatibilities between the two shells
  mentioned below.
 
 It could, but it doesn't now.
 
   This is the equivalent of what Marc is doing, except that zsh has no
   problems running bash's code. Note there's a difference with zsh's
   emulation bash (or rather bourne shell, or k shell), and zsh's
   emulation of bash's _completion_. The former is fine, the later is
   not.
  
   There are a couple of constructs supported by Bash but not by zsh,
   which we usually try to avoid.
 
  Yes, and is that a big deal?
 
  Not that big, but I wanted to point out that it's not fine either.
  Just a slight maintenance burden, because we have to pay attention not
  to use such constructs.
 
 Do we have to pay attention?

Unless you don't mind possible breakages of zsh completion, yes.

 I say when we encounter one of such maintenance burden issues _then_
 we think about it. In the meantime for all we know sourcing bash's
 script from zsh is fine.

That's a cool argument, will remember it when it again comes to
refactoring the __gitcomp() tests.  For now those tests work just
fine.  When we encounter maintenance burden issues, like fixing a bug
requiring the same modification to all of those tests, then we'll
think about 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


Re: [PATCH] tcsh-completion re-using git-completion.bash

2012-11-17 Thread Marc Khouzam
On Fri, Nov 16, 2012 at 4:56 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 10:20 PM, Junio C Hamano gits...@pobox.com wrote:

 The point is not about the quality of zsh's emulation
 of (k)sh when it is run under that mode, but is about not having to
 have that logic in bash-only part in the first place.

 As I said, that logic can be moved away _if_ my wrapper is merged. But
 then again, that would cause regressions to existing users.

Please forgive me as I don't know the background of the efforts for
zsh git-completion or
the syntax for zsh completion, but I thought I'd mention another
approach I tried for tcsh
which may work for zsh.

I gather that using a wrapper for zsh causes concerns about
backwards-compatibility.
So, what could be done is have the bash script do both jobs: setup the
zsh completion
commands, and output the git completion using bash itself.  At the top
of git-completion.bash
(or it could be even pushed at the bottom using if/else) we could use:

if [[ -n ${ZSH_VERSION-} ]]; then
  # replace below by zsh completion commands calling `bash
${HOME}/.git-completion.bash`
  complete git   'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
  complete gitk 'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
  exit
fi

That way the zsh user would still simply do 'source
~/.git-completion.bash' which would
only execute the two zsh completion setup commands.  Then, when completion is
triggered, it calls `bash ${HOME}/.git-completion.bash ${COMMAND_LINE}` and
processes the output like tcsh does.  This limits the zsh-specific
code to 2 lines for
the entire script.

I got this to work for tcsh (solution (B)) adding the following a the top of
git-completion.bash:

test $tcsh !=   \
   complete git  'p,*,`${HOME}/.git-completion.sh
${COMMAND_LINE}|\sort|\uniq`,'  \
   complete gitk 'p,*,`${HOME}/.git-completion.sh
${COMMAND_LINE}|\sort|\uniq`,'  \
   exit

but I didn't think people would go for that since those lines have to
work in both bash
and tcsh syntax.  I thought this made the script a bit brittle.

Just a thought.

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] tcsh-completion re-using git-completion.bash

2012-11-17 Thread Felipe Contreras
On Sat, Nov 17, 2012 at 6:15 PM, Marc Khouzam marc.khou...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 4:56 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 10:20 PM, Junio C Hamano gits...@pobox.com wrote:

 The point is not about the quality of zsh's emulation
 of (k)sh when it is run under that mode, but is about not having to
 have that logic in bash-only part in the first place.

 As I said, that logic can be moved away _if_ my wrapper is merged. But
 then again, that would cause regressions to existing users.

 Please forgive me as I don't know the background of the efforts for
 zsh git-completion or
 the syntax for zsh completion, but I thought I'd mention another
 approach I tried for tcsh
 which may work for zsh.

 I gather that using a wrapper for zsh causes concerns about
 backwards-compatibility.

I don't see any concerns.

 if [[ -n ${ZSH_VERSION-} ]]; then
   # replace below by zsh completion commands calling `bash
 ${HOME}/.git-completion.bash`

   complete git   'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'
   complete gitk 'p/*/`bash ${HOME}/.git-completion.bash ${COMMAND_LINE}`/'

That doesn't work in zsh. It might be possible to do something
similar, but it would probably require many more lines.

And we can achieve the same by essentially moving the relevant code of
my wrapper:

--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -23,10 +23,6 @@
 #3) Consider changing your PS1 to also show the current branch,
 #   see git-prompt.sh for details.

-if [[ -n ${ZSH_VERSION-} ]]; then
-   autoload -U +X bashcompinit  bashcompinit
-fi
-
 case $COMP_WORDBREAKS in
 *:*) : great ;;
 *)   COMP_WORDBREAKS=$COMP_WORDBREAKS:
@@ -2404,6 +2400,32 @@ __gitk_main ()
__git_complete_revlist
 }

+if [[ -n ${ZSH_VERSION-} ]]; then
+   emulate -L zsh
+
+   __gitcompadd ()
+   {
+   compadd -Q -S $4 -P ${(M)cur#*[=:]} -p $2 --
${=1}  _ret=0
+   }
+
+   _git ()
+   {
+   local _ret=1
+   () {
+ emulate -L ksh
+   local cur cword prev
+   cur=${words[CURRENT-1]}
+   prev=${words[CURRENT-2]}
+   let cword=CURRENT-1
+   __${service}_main
+   }
+   let _ret  _default -S ''  _ret=0
+   return _ret
+   }
+   compdef _git git gitk
+   return
+fi
+
 __git_func_wrap ()
 {
if [[ -n ${ZSH_VERSION-} ]]; then

-- 
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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Marc Khouzam
On Thu, Nov 15, 2012 at 8:41 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Thu, Nov 15, 2012 at 12:51 PM, Marc Khouzam marc.khou...@gmail.com wrote:
 The current tcsh-completion support for Git, as can be found on the
 Internet, takes the approach of defining the possible completions
 explicitly.  This has the obvious draw-back to require constant
 updating as the Git code base evolves.

 The approach taken by this commit is to to re-use the advanced bash
 completion script and use its result for tcsh completion.  This is
 achieved by executing (versus sourcing) the bash script and
 outputting the completion result for tcsh consumption.

 Three solutions were looked at to implement this approach with (A)
 being retained:

   A) Modifications:
   git-completion.bash and new git-completion.tcsh

 As I said, I don't think this is needed. It can be done in a single
 stand-alone script without modifications to git-completion.bash.

 This works:

Thank you for taking the time to try things out.

What you suggest below is an improvement on solution (C).
I had chosen (A) instead because (C) creates a third script
which gets generated each time a new shell is started.
It should be safe, but it felt a little wrong.
But I have to admit I was on the fence between the two
solutions.

If you guys don't think it is bad to generate a third script
(that the user may notice in his ${HOME}),
I'll post a new patch (and try once more to get gmail not to
replace the tabs with spaces), using your improved
solution (C).

 set called = ($_)

I fought with this a lot before posting to the list.
It seems that $_ is not set when a double sourcing
happens.  Testing the solution as an actual user
showed me that when I start a new shell it
sources ~/.tcshrc, which then sources ~/.git-completion.tcsh
and then $_ is empty for some reason.

I couldn't find another way to figure out where the script
is located, which is why I had to force the user to use
${HOME} for everything.

 set script = ${called[2]}.tmp

 cat \EOF  $script
 source $HOME/.git-completion.sh

This is nice.  Shame on me not to have thought about it.
In my version I actually 'cat' the entire bash script into $script
instead of simply sourcing it.

 # Set COMP_WORDS in a way that can be handled by the bash script.
 COMP_WORDS=($1)

 # Set COMP_CWORD to the cursor location as bash would.
 if [ -n ${2-} ]; then
 COMP_CWORD=$2
 else

Since this code will be part of a tcsh-only script, I don't think
we need to prepare for a possible $2.  tcsh won't provide it.
So, I'll remove that logic, which will simplify things slightly.

 # 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]}

 IFS=$'\n'
 echo ${COMPREPLY[*]}
 \EOF

 complete git  'p/*/`bash ${script} ${COMMAND_LINE} | sort | uniq`/'
 complete gitk 'p/*/`bash ${script} ${COMMAND_LINE} | sort | uniq`/'

I am worried about 'sort' and 'uniq' being aliased by the user, so I was
thinking of using '\sort | \uniq'

I'll work on the new version of the solution.

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


Re: [PATCH] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Felipe Contreras
On Fri, Nov 16, 2012 at 3:39 PM, Marc Khouzam marc.khou...@gmail.com wrote:
 On Thu, Nov 15, 2012 at 8:41 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 On Thu, Nov 15, 2012 at 12:51 PM, Marc Khouzam marc.khou...@gmail.com 
 wrote:
 The current tcsh-completion support for Git, as can be found on the
 Internet, takes the approach of defining the possible completions
 explicitly.  This has the obvious draw-back to require constant
 updating as the Git code base evolves.

 The approach taken by this commit is to to re-use the advanced bash
 completion script and use its result for tcsh completion.  This is
 achieved by executing (versus sourcing) the bash script and
 outputting the completion result for tcsh consumption.

 Three solutions were looked at to implement this approach with (A)
 being retained:

   A) Modifications:
   git-completion.bash and new git-completion.tcsh

 As I said, I don't think this is needed. It can be done in a single
 stand-alone script without modifications to git-completion.bash.

 This works:

 Thank you for taking the time to try things out.

 What you suggest below is an improvement on solution (C).
 I had chosen (A) instead because (C) creates a third script
 which gets generated each time a new shell is started.

We could generate the script only when it's not already present. The
disadvantage is that if this script is updated, the helper one would
not. One way to solve the problem would be to append the current
version of git, and figure a way to query it out. Another would be to
checksum it. But then again, maybe it's more expensive to check the
version or checksum than just write the file again.

Is it possible to just check if this is a login shell?

 set called = ($_)

 I fought with this a lot before posting to the list.
 It seems that $_ is not set when a double sourcing
 happens.  Testing the solution as an actual user
 showed me that when I start a new shell it
 sources ~/.tcshrc, which then sources ~/.git-completion.tcsh
 and then $_ is empty for some reason.

 I couldn't find another way to figure out where the script
 is located, which is why I had to force the user to use
 ${HOME} for everything.

Ah :(

-- 
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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Marc Khouzam
On Fri, Nov 16, 2012 at 10:33 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 3:39 PM, Marc Khouzam marc.khou...@gmail.com wrote:
 On Thu, Nov 15, 2012 at 8:41 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 On Thu, Nov 15, 2012 at 12:51 PM, Marc Khouzam marc.khou...@gmail.com 
 wrote:
 The current tcsh-completion support for Git, as can be found on the
 Internet, takes the approach of defining the possible completions
 explicitly.  This has the obvious draw-back to require constant
 updating as the Git code base evolves.

 The approach taken by this commit is to to re-use the advanced bash
 completion script and use its result for tcsh completion.  This is
 achieved by executing (versus sourcing) the bash script and
 outputting the completion result for tcsh consumption.

 Three solutions were looked at to implement this approach with (A)
 being retained:

   A) Modifications:
   git-completion.bash and new git-completion.tcsh

 As I said, I don't think this is needed. It can be done in a single
 stand-alone script without modifications to git-completion.bash.

 This works:

 Thank you for taking the time to try things out.

 What you suggest below is an improvement on solution (C).
 I had chosen (A) instead because (C) creates a third script
 which gets generated each time a new shell is started.

 We could generate the script only when it's not already present. The
 disadvantage is that if this script is updated, the helper one would
 not.

I didn't like that too much either.

 One way to solve the problem would be to append the current
 version of git, and figure a way to query it out. Another would be to
 checksum it. But then again, maybe it's more expensive to check the
 version or checksum than just write the file again.

Yeah, I'm also thinking that re-generating the script is not bad enough
to introduce this complexity.

 Is it possible to just check if this is a login shell?

I think it would be nice to allow the user to manually
source git-completion.tcsh, in case they want to make
manual modifications to it.

I think the most user-friendly option is to actually re-generate the
script each time.  It feels wrong, but it works well :)

 set called = ($_)

 I fought with this a lot before posting to the list.
 It seems that $_ is not set when a double sourcing
 happens.  Testing the solution as an actual user
 showed me that when I start a new shell it
 sources ~/.tcshrc, which then sources ~/.git-completion.tcsh
 and then $_ is empty for some reason.

 I couldn't find another way to figure out where the script
 is located, which is why I had to force the user to use
 ${HOME} for everything.

 Ah :(

 --
 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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Felipe Contreras
On Fri, Nov 16, 2012 at 4:48 PM, Marc Khouzam marc.khou...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 10:33 AM, Felipe Contreras
 felipe.contre...@gmail.com wrote:

 Is it possible to just check if this is a login shell?

 I think it would be nice to allow the user to manually
 source git-completion.tcsh, in case they want to make
 manual modifications to it.

Yeah, they could still do that... because they would be running in a
login shell.

What I meant is that if the user does: tcsh
my_script_that_has_nothing_to_do_with_completion.sh, they would not be
executing this whole script.

 I think the most user-friendly option is to actually re-generate the
 script each time.  It feels wrong, but it works well :)

I'm not too strongly opposed to add that function to the bash
completion, but to do it only for tcsh doesn't sound right, specially
when there are other alternatives. Correct me if I'm wrong, but very
few people use tcsh.

-- 
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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Marc Khouzam
On Fri, Nov 16, 2012 at 12:18 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 4:48 PM, Marc Khouzam marc.khou...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 10:33 AM, Felipe Contreras
 felipe.contre...@gmail.com wrote:

 Is it possible to just check if this is a login shell?

 I think it would be nice to allow the user to manually
 source git-completion.tcsh, in case they want to make
 manual modifications to it.

 Yeah, they could still do that... because they would be running in a
 login shell.

 What I meant is that if the user does: tcsh
 my_script_that_has_nothing_to_do_with_completion.sh, they would not be
 executing this whole script.

Oh, I see now.

I can put a check in the script for the existence of the $prompt variable.
This will indicate if it is a login shell or not.
However, a good .cshrc file should already have such a check to avoid
sourcing a bunch of useless things.  So, I personally think that we
should not add it to the git-completion.tcsh script but let the tcsh
user decide to do it herself.  But I don't mind being overruled :)

 I think the most user-friendly option is to actually re-generate the
 script each time.  It feels wrong, but it works well :)

 I'm not too strongly opposed to add that function to the bash
 completion, but to do it only for tcsh doesn't sound right, specially
 when there are other alternatives.

I agree, and this is why I made the proposed
__git_complete_with_output () generic.  That way it could be
used by other shells or programs.  But at this time, only tcsh
would make use of it.

If you think having __git_complete_with_output () could
be useful for others, I think we should go with solution (A).
If you don't think so, or if it is better to wait until a need
arises first, then solution (C) will work fine.

 Correct me if I'm wrong, but very few people use tcsh.

Less than I originally thought, when I started working
on this patch :-\  But I'm still hoping that the those people
will be a little happier with their git completion.

Marc


 --
 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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Felipe Contreras
On Fri, Nov 16, 2012 at 7:20 PM, Marc Khouzam marc.khou...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 12:18 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 4:48 PM, Marc Khouzam marc.khou...@gmail.com wrote:
 On Fri, Nov 16, 2012 at 10:33 AM, Felipe Contreras
 felipe.contre...@gmail.com wrote:

 Is it possible to just check if this is a login shell?

 I think it would be nice to allow the user to manually
 source git-completion.tcsh, in case they want to make
 manual modifications to it.

 Yeah, they could still do that... because they would be running in a
 login shell.

 What I meant is that if the user does: tcsh
 my_script_that_has_nothing_to_do_with_completion.sh, they would not be
 executing this whole script.

 Oh, I see now.

 I can put a check in the script for the existence of the $prompt variable.
 This will indicate if it is a login shell or not.
 However, a good .cshrc file should already have such a check to avoid
 sourcing a bunch of useless things.  So, I personally think that we
 should not add it to the git-completion.tcsh script but let the tcsh
 user decide to do it herself.  But I don't mind being overruled :)

Sounds sensible to me.

 I think the most user-friendly option is to actually re-generate the
 script each time.  It feels wrong, but it works well :)

 I'm not too strongly opposed to add that function to the bash
 completion, but to do it only for tcsh doesn't sound right, specially
 when there are other alternatives.

 I agree, and this is why I made the proposed
 __git_complete_with_output () generic.  That way it could be
 used by other shells or programs.  But at this time, only tcsh
 would make use of it.

 If you think having __git_complete_with_output () could
 be useful for others, I think we should go with solution (A).
 If you don't think so, or if it is better to wait until a need
 arises first, then solution (C) will work fine.

I don't see how it could be useful to others, and if we find out that
it could, we can always move the code.

 Correct me if I'm wrong, but very few people use tcsh.

 Less than I originally thought, when I started working
 on this patch :-\  But I'm still hoping that the those people
 will be a little happier with their git completion.

I think they would :) But we don't need to modify bash's script for
that (for now).

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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread SZEDER Gábor
On Fri, Nov 16, 2012 at 09:04:06PM +0100, Felipe Contreras wrote:
  I agree, and this is why I made the proposed
  __git_complete_with_output () generic.  That way it could be
  used by other shells or programs.  But at this time, only tcsh
  would make use of it.
 
  If you think having __git_complete_with_output () could
  be useful for others, I think we should go with solution (A).
  If you don't think so, or if it is better to wait until a need
  arises first, then solution (C) will work fine.

I think it would be useful.

 I don't see how it could be useful to others, and if we find out that
 it could, we can always move the code.

For zsh, perhaps?

As I understand the main issues with using the completion script with
zsh are the various little incompatibilities between the two shells
and bugs in zsh's emulation of Bash's completion-related builtins.
Running the completion script under Bash and using its results in zsh
would solve these issues at the root.  And would allow as to remove
some if [[ -n ${ZSH_VERSION-} ]] code.

--
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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Felipe Contreras
On Fri, Nov 16, 2012 at 9:40 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Fri, Nov 16, 2012 at 09:04:06PM +0100, Felipe Contreras wrote:
  I agree, and this is why I made the proposed
  __git_complete_with_output () generic.  That way it could be
  used by other shells or programs.  But at this time, only tcsh
  would make use of it.
 
  If you think having __git_complete_with_output () could
  be useful for others, I think we should go with solution (A).
  If you don't think so, or if it is better to wait until a need
  arises first, then solution (C) will work fine.

 I think it would be useful.

For what?

 I don't see how it could be useful to others, and if we find out that
 it could, we can always move the code.

 For zsh, perhaps?

Nope.

 As I understand the main issues with using the completion script with
 zsh are the various little incompatibilities between the two shells
 and bugs in zsh's emulation of Bash's completion-related builtins.
 Running the completion script under Bash and using its results in zsh
 would solve these issues at the root.  And would allow as to remove
 some if [[ -n ${ZSH_VERSION-} ]] code.

We can remove that code already, because we now have code that is
superior than zsh's bash completion emulation:

http://article.gmane.org/gmane.comp.version-control.git/208173

This is the equivalent of what Marc is doing, except that zsh has no
problems running bash's code. Note there's a difference with zsh's
emulation bash (or rather bourne shell, or k shell), and zsh's
emulation of bash's _completion_. The former is fine, the later is
not.

Of course, people might not be aware of this new script, and would
expect sourcing the bash one to work right away. Maybe at some point
we might throw a warning to suggest them to use my new script. But I
think we should wait a few releases just to make sure that people test
it and nothing is broken.

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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Junio C Hamano
SZEDER Gábor sze...@ira.uka.de writes:

 For zsh, perhaps?

Yeah, I was wondering about that.

If we make zsh completion read output from a little stub in bash
completion, just like Felipe steered this series for tcsh, we do not
have to worry about zsh does not split words unless emulating a
shell and here is a way to tell zsh to do so kind of stuff in bash
completion.  The point is not about the quality of zsh's emulation
of (k)sh when it is run under that mode, but is about not having to
have that logic in bash-only part in the first place.



--
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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread SZEDER Gábor
On Fri, Nov 16, 2012 at 10:03:41PM +0100, Felipe Contreras wrote:
 On Fri, Nov 16, 2012 at 9:40 PM, SZEDER Gábor sze...@ira.uka.de wrote:
  On Fri, Nov 16, 2012 at 09:04:06PM +0100, Felipe Contreras wrote:
   I agree, and this is why I made the proposed
   __git_complete_with_output () generic.  That way it could be
   used by other shells or programs.  But at this time, only tcsh
   would make use of it.
  
   If you think having __git_complete_with_output () could
   be useful for others, I think we should go with solution (A).
   If you don't think so, or if it is better to wait until a need
   arises first, then solution (C) will work fine.
 
  I think it would be useful.
 
 For what?

For zsh.

  I don't see how it could be useful to others, and if we find out that
  it could, we can always move the code.
 
  For zsh, perhaps?
 
 Nope.

Sure.

  As I understand the main issues with using the completion script with
  zsh are the various little incompatibilities between the two shells
  and bugs in zsh's emulation of Bash's completion-related builtins.
  Running the completion script under Bash and using its results in zsh
  would solve these issues at the root.  And would allow as to remove
  some if [[ -n ${ZSH_VERSION-} ]] code.
 
 We can remove that code already, because we now have code that is
 superior than zsh's bash completion emulation:
 
 http://article.gmane.org/gmane.comp.version-control.git/208173

Which depends on the completion script having a wrapper function
around compgen filling COMPREPLY.  However, COMPREPLY will be soon
filled by hand-rolled code to prevent expansion issues with compgen,
and there will be no such wrapper.

 This is the equivalent of what Marc is doing, except that zsh has no
 problems running bash's code. Note there's a difference with zsh's
 emulation bash (or rather bourne shell, or k shell), and zsh's
 emulation of bash's _completion_. The former is fine, the later is
 not.

There are a couple of constructs supported by Bash but not by zsh,
which we usually try to avoid.

--
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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Felipe Contreras
On Fri, Nov 16, 2012 at 10:22 PM, SZEDER Gábor sze...@ira.uka.de wrote:
 On Fri, Nov 16, 2012 at 10:03:41PM +0100, Felipe Contreras wrote:

  As I understand the main issues with using the completion script with
  zsh are the various little incompatibilities between the two shells
  and bugs in zsh's emulation of Bash's completion-related builtins.
  Running the completion script under Bash and using its results in zsh
  would solve these issues at the root.  And would allow as to remove
  some if [[ -n ${ZSH_VERSION-} ]] code.

 We can remove that code already, because we now have code that is
 superior than zsh's bash completion emulation:

 http://article.gmane.org/gmane.comp.version-control.git/208173

 Which depends on the completion script having a wrapper function
 around compgen filling COMPREPLY.

No, it does not. Previous incarnations didn't have this dependency:

http://article.gmane.org/gmane.comp.version-control.git/196720

I just thought it was neater this way.

 However, COMPREPLY will be soon
 filled by hand-rolled code to prevent expansion issues with compgen,
 and there will be no such wrapper.

I'm still waiting to see a resemblance of that code, but my bet would
be that there will be a way to fill both COMPREPLY, and call zsh's
compadd. But it's hard to figure that out without any code. Which is
why I'm thinking on doing it myself.

But even in that case, if push comes to shoves, this zsh wrapper can
ultimately read COMPREPLY and figure things backwards, as even more
previous versions did:

http://article.gmane.org/gmane.comp.version-control.git/189310

 This is the equivalent of what Marc is doing, except that zsh has no
 problems running bash's code. Note there's a difference with zsh's
 emulation bash (or rather bourne shell, or k shell), and zsh's
 emulation of bash's _completion_. The former is fine, the later is
 not.

 There are a couple of constructs supported by Bash but not by zsh,
 which we usually try to avoid.

Yes, and is that a big deal?

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] tcsh-completion re-using git-completion.bash

2012-11-16 Thread Felipe Contreras
On Fri, Nov 16, 2012 at 10:20 PM, Junio C Hamano gits...@pobox.com wrote:
 SZEDER Gábor sze...@ira.uka.de writes:

 For zsh, perhaps?

 Yeah, I was wondering about that.

 If we make zsh completion read output from a little stub in bash
 completion, just like Felipe steered this series for tcsh, we do not
 have to worry about zsh does not split words unless emulating a
 shell and here is a way to tell zsh to do so kind of stuff in bash
 completion.

Do we worry about that now? If we do, the only reason is because we
hadn't had a proper wrapper, like the one I'm proposing to merge. So,
we had to put things inside if [[ -n ${ZSH_VERSION-} ]]. Those things
would move to my wrapper.

The only exception where we had to change code outside of that chunk
that I'm aware of is '8d58c90 completion: Use parse-options raw output
for simple long options', which is probably fixed in later versions of
zsh, and if not, we could always replace those functions inside my
wrapper.

 The point is not about the quality of zsh's emulation
 of (k)sh when it is run under that mode, but is about not having to
 have that logic in bash-only part in the first place.

As I said, that logic can be moved away _if_ my wrapper is merged. But
then again, that would cause regressions to existing users.

Maybe we should warn them right now that they should be using my
wrapper, and that this method of zsh support would be obsoleted. But
my wrapper probably hasn't received enough testing, so do we really
want to do that right now?

Either way, I'm confident that whatever code we need can be
consolidated in git-completion.zsh, even without having to run bash.

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


[PATCH] tcsh-completion re-using git-completion.bash

2012-11-15 Thread Marc Khouzam
The current tcsh-completion support for Git, as can be found on the
Internet, takes the approach of defining the possible completions
explicitly.  This has the obvious draw-back to require constant
updating as the Git code base evolves.

The approach taken by this commit is to to re-use the advanced bash
completion script and use its result for tcsh completion.  This is
achieved by executing (versus sourcing) the bash script and
outputting the completion result for tcsh consumption.

Three solutions were looked at to implement this approach with (A)
being retained:

  A) Modifications:
  git-completion.bash and new git-completion.tcsh

 Modify the existing git-completion.bash script to support
 being sourced using bash (as now), but also executed using bash.
 When being executed, the script will output the result of the
 computed completion to be re-used elsewhere (e.g., in tcsh).

 The modification to git-completion.bash is made not to be
 tcsh-specific, but to allow future users to also re-use its
 output.  Therefore, to be general, git-completion.bash accepts a
 second optional parameter, which is not used by tcsh, but could
 prove useful for other users.

 Pros:
   1- allows the git-completion.bash script to easily be re-used
   2- tcsh support is mostly isolated in git-completion.tcsh
 Cons (for tcsh users only):
   1- requires the user to copy both git-completion.tcsh and
  git-completion.bash to ${HOME}
   2- requires bash script to have a fixed name and location:
  ${HOME}/.git-completion.bash

  B) Modifications:
  git-completion.bash

 Modify the existing git-completion.bash script to support
 being sourced using bash (as now), but also executed using bash,
 and sourced using tcsh.

 Pros:
   1- only requires the user to deal with a single file
   2- maintenance more obvious for tcsh since it is entirely part
  of the same git-completion.bash script.
 Cons:
   1- tcsh support could affect bash support as they share the
  same script
   2- small tcsh section must use syntax suitable for both tcsh
  and bash and must be at the beginning of the script
   3- requires script to have a fixed name and location:
  ${HOME}/.git-completion.sh (for tcsh users only)

  C) Modifications:
  New git-completion.tcsh

 Provide a short tcsh script that converts git-completion.bash
 into an executable script suitable to be used by tcsh.

 Pros:
   1- tcsh support is entirely isolated in git-completion.tcsh
   2- new tcsh script can be as complex as needed
 Cons (for tcsh users only):
   1- requires the user to copy both git-completion.tcsh and
  git-completion.bash to ${HOME}
   2- requires bash script to have a fixed name and location:
  ${HOME}/.git-completion.bash
   3- sourcing the new script will generate a third script

Approach (A) was selected to keep the tcsh completion support well
isolated without introducing excessive complexity.

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

Here is the updated version of the patch.
I got git send-email to work, so I hope the formatting will be correct.

Thanks in advance.

Marc

 contrib/completion/git-completion.bash |   47 
 contrib/completion/git-completion.tcsh |   29 +++
 2 files changed, 76 insertions(+), 0 deletions(-)
 create mode 100644 contrib/completion/git-completion.tcsh

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index be800e0..d71a016 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2481,3 +2481,50 @@ __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 in a way that can be handled by the bash script.
+   COMP_WORDS=($1)
+
+   # 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 

Re: [PATCH] tcsh-completion re-using git-completion.bash

2012-11-15 Thread Felipe Contreras
On Thu, Nov 15, 2012 at 12:51 PM, Marc Khouzam marc.khou...@gmail.com wrote:
 The current tcsh-completion support for Git, as can be found on the
 Internet, takes the approach of defining the possible completions
 explicitly.  This has the obvious draw-back to require constant
 updating as the Git code base evolves.

 The approach taken by this commit is to to re-use the advanced bash
 completion script and use its result for tcsh completion.  This is
 achieved by executing (versus sourcing) the bash script and
 outputting the completion result for tcsh consumption.

 Three solutions were looked at to implement this approach with (A)
 being retained:

   A) Modifications:
   git-completion.bash and new git-completion.tcsh

As I said, I don't think this is needed. It can be done in a single
stand-alone script without modifications to git-completion.bash.

This works:

set called = ($_)
set script = ${called[2]}.tmp

cat \EOF  $script
source $HOME/.git-completion.sh

# Set COMP_WORDS in a way that can be handled by the bash script.
COMP_WORDS=($1)

# 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]}

IFS=$'\n'
echo ${COMPREPLY[*]}
\EOF

complete git  'p/*/`bash ${script} ${COMMAND_LINE} | sort | uniq`/'
complete gitk 'p/*/`bash ${script} ${COMMAND_LINE} | sort | uniq`/'

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