* Re: Re: [PATCH v3 1/3] git-submodule add: Add -r/--record option
From: Heiko Voigt @ 2012-11-17 15:04 UTC (permalink / raw)
To: W. Trevor King
Cc: Junio C Hamano, Git, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <20121110190232.GD2739@mjolnir>
Hi,
sorry for the late reply but my git time is limited.
On Sat, Nov 10, 2012 at 02:02:32PM -0500, W. Trevor King wrote:
> On Fri, Nov 09, 2012 at 05:29:27PM +0100, Heiko Voigt wrote:
> > I think we should agree on a behavior for this option and implement it
> > the same time when add learns about it. When we were discussing floating
> > submodules as an important option for the gerrit people I already started
> > to implement a proof of concept. Please have a look here:
> >
> > https://github.com/hvoigt/git/commits/hv/floating_submodules
>
> After skimming through this, something like
>
> $ git submodule update --pull
>
> would probably be better than introducing a new command:
Yeah along the lines of that, but one thing to keep in mind:
We already have --rebase and --merge which do slightly different things
(I think). Adding --pull here should behave similar to them. Like fetch
and merge is the same to pull without submodules.
If I am understanding your goal correctly your --pull would be
different. On the other hand: A --pull makes no sense if we apply it to
the existing --merge option since it merges the recorded sha1 into the
current HEAD. Just a fetch would not really make a difference.
Thinking along the existing options I would probably still expect --pull
to merge something into the current HEAD. So maybe we have to iron out
where this command/option should go. But changing that once we have a
patch to discuss should not be that much work. So please proceed with
--pull and once we know exactly what it does we can polish that.
> On Sat, Nov 10, 2012 at 01:44:37PM -0500, W. Trevor King wrote:
> > $ git submodule pull-branch
>
> I think "floating submodules" is a misleading name for this feature
> though, since the checkout SHA is explicitly specified. We're just
> making it more convenient to explicitly update the SHA. How about
> "tracking submodules"?
Until now we have always called this workflow floating submodules. I
imaging since the submodule floats to the newest revision (whatever the
user chooses that to be) instead of staying at the recorded sha1.
"tracking submodules" sounds strange to me since the term tracked in git
is mainly used in combination with exact recorded history (e.g. tracking
branch). Since it is about *not* checking out the recorded sha1 but
something that can change I think that could cause confusion.
I think floating is a more unambiguous term and already known on the
list.
Cheers Heiko
^ permalink raw reply
* Re: using multiple version of git simultaneously
From: Tomas Carnecky @ 2012-11-17 14:50 UTC (permalink / raw)
To: arif, git
In-Reply-To: <k886on$nn5$1@ger.gmane.org>
On Sat, 17 Nov 2012 20:25:21 +0600, arif <aftnix@gmail.com> wrote:
> I'm trying to use different version of git simultaneously. So how can i
> append some suffix (like "--program-suffix=git1.8) so that i can
> distinguish between different versions.
Install each version into its own prefix (~/git/1.8.0/, ~/git/1.7.0/ etc).
^ permalink raw reply
* Re: [PATCH] Rename V15_MINGW_HEADERS into CYGWIN_OLD_WINSOCK_HEADERS
From: Mark Levedahl @ 2012-11-17 14:38 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <201211170809.50395.tboegi@web.de>
On 11/17/2012 02:09 AM, Torsten Bögershausen wrote:
> See commit 380a4d927bff693c42fc6b22c3547bdcaac4bdc3:
> "Update cygwin.c for new mingw-64 win32 api headers"
> Cygwin up to 1.7.16 uses some header file from the WINE project
> Cygwin 1.7.17 uses some header file from the mingw-64 project
> As the old cygwin (like 1.5) never used mingw,
> the name V15_MINGW_HEADERS is confusing.
> Rename it into CYGWIN_OLD_WINSOCK_HEADERS
>
>
> diff --git a/Makefile b/Makefile
> index c3edf8c..c2ea735 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1089,7 +1089,7 @@ ifeq ($(uname_O),Cygwin)
> NO_SYMLINK_HEAD = YesPlease
> NO_IPV6 = YesPlease
> OLD_ICONV = UnfortunatelyYes
> - V15_MINGW_HEADERS = YesPlease
> + CYGWIN_OLD_WINSOCK_HEADERS = YesPlease
>
WINSOCK is certainly a part of the win32 api implementation, but it is
is the entire win32api that changed, not just the tiny bit dealing with
sockets.
Basically, WINDOWS.h, and everything it includes, and all of the dlls it
touches, and the .o files, changed. Calling it "OLD" is not helpful,
what happens in the future with the next change? The only version info
we really have is the dll version. We are switching between the win32
api implementation shipped with cygwin dll version 1.5.x and the one
that is now current. And, the shift is not tied to any particular cygwin
1.7.x dll version either (there are no cross dependencies between the
win32api implementation and any particular dll in the 1.7.x series, just
a coincidence in time as to what packages got updated when). So my
suggestion in the bike shedding category is to
s/V15_MINGW_HEADERS/CYGWIN_V15_WIN32API/
/end of bike shedding.
If this is really worth a second patch, I'll be happy to send one :^)
Mark
^ permalink raw reply
* using multiple version of git simultaneously
From: arif @ 2012-11-17 14:25 UTC (permalink / raw)
To: git
Hi,
I'm trying to use different version of git simultaneously. So how can i
append some suffix (like "--program-suffix=git1.8) so that i can
distinguish between different versions.
--
Cheers
arif
^ permalink raw reply
* Re: [PATCH] tcsh-completion re-using git-completion.bash
From: SZEDER Gábor @ 2012-11-17 14:17 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Marc Khouzam, git
In-Reply-To: <CAMP44s3FFEGJDa6cnwVY0aJkoU_-OdvDPD0gPQtrUqdY2JCpWw@mail.gmail.com>
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 <szeder@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. ;)
^ permalink raw reply
* Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
From: SZEDER Gábor @ 2012-11-17 14:14 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <CAMP44s1ZgNM1WXPu_-q9aFkz8Ui3czwcUqHWvs7Yspi_p9kuNQ@mail.gmail.com>
On Sat, Nov 17, 2012 at 12:50:39PM +0100, Felipe Contreras wrote:
> On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>
> > __gitcomp_nl ()
> > {
> > local IFS=$'\n'
> > - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
> > + COMPREPLY=($(awk -v pfx="${2-}" -v sfx="${4- }" -v cur="${3-$cur}" '
> > + BEGIN {
> > + FS="\n";
> > + len=length(cur);
> > + }
> > + {
> > + if (cur == substr($1, 1, len))
> > + print pfx$1sfx;
> > + }' <<< "$1" ))
> > }
>
> Does this really perform better than my alternative?
>
> + for x in $1; do
> + if [[ "$x" = "$3"* ]]; then
> + COMPREPLY+=("$2$x$4")
> + fi
> + done
It does:
My version:
$ refs="$(for i in {0..9999} ; do echo branch$i ; done)"
$ time __gitcomp_nl "$refs"
real 0m0.109s
user 0m0.096s
sys 0m0.012s
Yours:
$ time __gitcomp_nl "$refs"
real 0m0.321s
user 0m0.312s
sys 0m0.008s
^ permalink raw reply
* Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1
From: SZEDER Gábor @ 2012-11-17 14:13 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <CAMP44s3OG+dzxZNpR+qELvcS37KDWh+Bnf0K1zGze4f3P4OWNg@mail.gmail.com>
On Sat, Nov 17, 2012 at 12:27:40PM +0100, Felipe Contreras wrote:
> On Sat, Nov 17, 2012 at 11:58 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>
> >> # The following function is based on code from:
> >> @@ -249,10 +246,16 @@ __gitcomp ()
> >> --*=)
> >> ;;
> >> *)
> >> - local IFS=$'\n'
> >> - __gitcompadd "$(__gitcomp_1 "${1-}" "${4-}")" "${2-}" "$cur_" ""
> >> + local c IFS=$' \t\n'
> >> + for c in ${1-}; do
> >> + c=`__gitcomp_1 "$c${4-}"`
> >
> > 1. Backticks.
> > 2. A subshell for every word in the wordlist?
>
> Fine, lets make it hard for zsh then:
No, it's about keeping it usable. With this change offering the
approximately 170 commands for 'git help <TAB>' would take more than 4
seconds on Windows.
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -56,19 +56,6 @@ __gitdir ()
> fi
> }
>
> -__gitcomp_1 ()
> -{
> - local c IFS=$' \t\n'
> - for c in $1; do
> - c="$c$2"
> - case $c in
> - --*=*|*.) ;;
> - *) c="$c " ;;
> - esac
> - printf '%s\n' "$c"
> - done
> -}
> -
> # The following function is based on code from:
> #
> # bash_completion - programmable completion functions for bash 3.2+
> @@ -241,12 +228,22 @@ __gitcomp ()
> COMPREPLY=()
> ;;
> *)
> - local IFS=$'\n'
> - COMPREPLY=($(compgen -P "${2-}" \
> - -W "$(__gitcomp_1 "${1-}" "${4-}")" \
> - -- "$cur_"))
> + local c i IFS=$' \t\n'
> + i=0
> + for c in ${1-}; do
> + c="$c${4-}"
> + case $c in
> + --*=*|*.) ;;
> + *) c="$c " ;;
> + esac
> + if [[ "$c" = "$cur_"* ]]; then
> + (( i++ ))
> + COMPREPLY[$i]="${2-}$c"
> + fi
> + done
> ;;
> esac
> +
> }
>
> # Generates completion reply with compgen from newline-separated possible
>
> --
> Felipe Contreras
^ permalink raw reply
* Re: [RFC/PATCH 4/5] completion: get rid of compgen
From: SZEDER Gábor @ 2012-11-17 14:12 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <CAMP44s21CUb3_KhHBfJXW+Eqd45kz1hcbx3GCbs+f0HNRDEAzw@mail.gmail.com>
On Sat, Nov 17, 2012 at 12:42:38PM +0100, Felipe Contreras wrote:
> On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> > On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:
> >> The functionality we use is very simple, plus, this fixes a known
> >> breakage 'complete tree filename with metacharacters'.
> >>
> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >> ---
> >> contrib/completion/git-completion.bash | 6 +++++-
> >> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> >> index 975ae13..ad3e1fe 100644
> >> --- a/contrib/completion/git-completion.bash
> >> +++ b/contrib/completion/git-completion.bash
> >> @@ -227,7 +227,11 @@ fi
> >>
> >> __gitcompadd ()
> >> {
> >> - COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
> >> + for x in $1; do
> >> + if [[ "$x" = "$3"* ]]; then
> >> + COMPREPLY+=("$2$x$4")
> >> + fi
> >> + done
> >
> > The whole point of creating __gitcomp_nl() back then was to fill
> > COMPREPLY without iterating through all words in the wordlist, making
> > completion faster for large number of words, e.g. a lot of refs, or
> > later a lot of symbols for 'git grep' in a larger project.
> >
> > The loop here kills that optimization.
>
> So your solution is to move the loop to awk? I fail to see how that
> could bring more optimization, specially since it includes an extra
> fork now.
This patch didn't aim for more optimization, but it was definitely a
goal not to waste what we gained by creating __gitcomp_nl() in
a31e6262 (completion: optimize refs completion, 2011-10-15). However,
as it turns out the new version with awk is actually faster than
current master with compgen:
Before:
$ refs="$(for i in {0..9999} ; do echo branch$i ; done)"
$ time __gitcomp_nl "$refs"
real 0m0.242s
user 0m0.220s
sys 0m0.028s
After:
$ time __gitcomp_nl "$refs"
real 0m0.109s
user 0m0.096s
sys 0m0.012s
^ permalink raw reply
* Re: [PATCH 6/7] completion: fix expansion issue in __gitcomp()
From: SZEDER Gábor @ 2012-11-17 14:09 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <CAMP44s3J3e_bcyoQmcdQno59dPJuJ4=7ej=-eseE5j2tteD=dA@mail.gmail.com>
On Sat, Nov 17, 2012 at 12:39:24PM +0100, Felipe Contreras wrote:
> On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>
> > -# Generates completion reply with compgen, appending a space to possible
> > -# completion words, if necessary.
> > +# Generates completion reply for the word in $cur, appending a space to
> > +# possible completion words, if necessary.
> > # It accepts 1 to 4 arguments:
> > # 1: List of possible completion words.
> > # 2: A prefix to be added to each possible completion word (optional).
> > -# 3: Generate possible completion matches for this word (optional).
> > +# 3: Generate possible completion matches for this word instead of $cur
> > +# (optional).
> > # 4: A suffix to be appended to each possible completion word (optional).
> > __gitcomp ()
> > {
> > @@ -241,10 +242,22 @@ __gitcomp ()
> > COMPREPLY=()
> > ;;
> > *)
> > - local IFS=$'\n'
> > - COMPREPLY=($(compgen -P "${2-}" \
> > - -W "$(__gitcomp_1 "${1-}" "${4-}")" \
> > - -- "$cur_"))
> > + local i=0 c IFS=$' \t\n'
> > + for c in $1; do
> > + case $c in
> > + "$cur_"*)
> > + c="$c${4-}"
> > + case $c in
> > + --*=*|*.) ;;
> > + *) c="$c " ;;
> > + esac
> > + COMPREPLY[$i]="${2-}$c"
> > + i=$((++i))
> > + ;;
> > + *)
> > + ;;
> > + esac
> > + done
>
> This is not quite the same as before, is it? Before the suffix would
> be taken into consideration for the comparison with $cur_, but not any
> more.
That's a good catch, thanks.
I remember it puzzled me that the suffix is considered in the
comparison (and that a trailing space would be appended even after a
given suffix, too, so there seems to be no way to disable the trailing
space). However, currently it doesn't make a difference for us,
because afaics we never specify a suffix for __gitcomp(). There were
two instances in _git_config() where we specified "." as suffix, but
those two were converted to __gitcomp_nl(). I changed those callsites
back to __gitcomp() and tried to provoke wrong behavior with the above
patch, but couldn't.
Anyway, it's better to err on the safe side, so here's the fixup.
-- >8 --
Subject: [PATCH] fixup! completion: fix expansion issue in __gitcomp()
---
contrib/completion/git-completion.bash | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a1bf732f..29818fb5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -231,9 +231,9 @@ __gitcomp ()
*)
local i=0 c IFS=$' \t\n'
for c in $1; do
+ c="$c${4-}"
case $c in
"$cur_"*)
- c="$c${4-}"
case $c in
--*=*|*.) ;;
*) c="$c " ;;
--
1.8.0.220.g4d14ece
^ permalink raw reply related
* git-credential-gnome-keyring fails at multilib
From: Peter Alfredsen @ 2012-11-17 14:05 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]
Downstream bug report: https://bugs.gentoo.org/443634
The git-credential-gnome-keyring Makefile doesn't allow overriding its
variables, making for spectacular link failure if you use CFLAGS for
aught but decoration.
gcc -g -O2 -Wall -I/usr/include/gnome-keyring-1
-I/usr/include/glib-2.0 -I/usr/lib32/glib-2.0/include -o
git-credential-gnome-keyring.o -c git-credential-gnome-keyring.c
gcc -o git-credential-gnome-keyring -Wl,--as-needed
-Wl,--hash-style=gnu -m32 git-credential-gnome-keyring.o
-lgnome-keyring -lglib-2.0
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.2/../../../../x86_64-pc-linux-gnu/bin/ld:
i386:x86-64 architecture of input file
`git-credential-gnome-keyring.o' is incompatible with i386 output
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.2/../../../../x86_64-pc-linux-gnu/bin/ld:
git-credential-gnome-keyring.o: file class ELFCLASS64 incompatible
with ELFCLASS32
/usr/lib/gcc/x86_64-pc-linux-gnu/4.7.2/../../../../x86_64-pc-linux-gnu/bin/ld:
final link failed: File in wrong format
collect2: error: ld returned 1 exit status
Attached patch fixes it.
/Peter
[-- Attachment #2: git-1.8.0-gnome-keyring-multilib.patch --]
[-- Type: application/octet-stream, Size: 597 bytes --]
diff -NrU5 git-1.8.0.orig/contrib/credential/gnome-keyring/Makefile git-1.8.0/contrib/credential/gnome-keyring/Makefile
--- git-1.8.0.orig/contrib/credential/gnome-keyring/Makefile 2012-11-17 14:45:06.536641381 +0100
+++ git-1.8.0/contrib/credential/gnome-keyring/Makefile 2012-11-17 14:45:40.883442939 +0100
@@ -1,11 +1,11 @@
MAIN:=git-credential-gnome-keyring
all:: $(MAIN)
-CC = gcc
-RM = rm -f
-CFLAGS = -g -O2 -Wall
+CC ?= gcc
+RM ?= rm -f
+CFLAGS ?= -g -O2 -Wall
-include ../../../config.mak.autogen
-include ../../../config.mak
INCS:=$(shell pkg-config --cflags gnome-keyring-1)
^ permalink raw reply
* Re: [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
From: Felipe Contreras @ 2012-11-17 11:50 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-6-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> __gitcomp_nl ()
> {
> local IFS=$'\n'
> - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
> + COMPREPLY=($(awk -v pfx="${2-}" -v sfx="${4- }" -v cur="${3-$cur}" '
> + BEGIN {
> + FS="\n";
> + len=length(cur);
> + }
> + {
> + if (cur == substr($1, 1, len))
> + print pfx$1sfx;
> + }' <<< "$1" ))
> }
Does this really perform better than my alternative?
+ for x in $1; do
+ if [[ "$x" = "$3"* ]]; then
+ COMPREPLY+=("$2$x$4")
+ fi
+ done
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] tcsh-completion re-using git-completion.bash
From: Felipe Contreras @ 2012-11-17 11:46 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Marc Khouzam, git
In-Reply-To: <20121117105605.GB12052@goldbirke>
On Sat, Nov 17, 2012 at 11:56 AM, SZEDER Gábor <szeder@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
^ permalink raw reply
* Re: [RFC/PATCH 4/5] completion: get rid of compgen
From: Felipe Contreras @ 2012-11-17 11:42 UTC (permalink / raw)
To: SZEDER Gábor
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <20121117110031.GE12052@goldbirke>
On Sat, Nov 17, 2012 at 12:00 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:
>> The functionality we use is very simple, plus, this fixes a known
>> breakage 'complete tree filename with metacharacters'.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>> contrib/completion/git-completion.bash | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 975ae13..ad3e1fe 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -227,7 +227,11 @@ fi
>>
>> __gitcompadd ()
>> {
>> - COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
>> + for x in $1; do
>> + if [[ "$x" = "$3"* ]]; then
>> + COMPREPLY+=("$2$x$4")
>> + fi
>> + done
>
> The whole point of creating __gitcomp_nl() back then was to fill
> COMPREPLY without iterating through all words in the wordlist, making
> completion faster for large number of words, e.g. a lot of refs, or
> later a lot of symbols for 'git grep' in a larger project.
>
> The loop here kills that optimization.
So your solution is to move the loop to awk? I fail to see how that
could bring more optimization, specially since it includes an extra
fork now.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 6/7] completion: fix expansion issue in __gitcomp()
From: Felipe Contreras @ 2012-11-17 11:39 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-7-git-send-email-szeder@ira.uka.de>
On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> -# Generates completion reply with compgen, appending a space to possible
> -# completion words, if necessary.
> +# Generates completion reply for the word in $cur, appending a space to
> +# possible completion words, if necessary.
> # It accepts 1 to 4 arguments:
> # 1: List of possible completion words.
> # 2: A prefix to be added to each possible completion word (optional).
> -# 3: Generate possible completion matches for this word (optional).
> +# 3: Generate possible completion matches for this word instead of $cur
> +# (optional).
> # 4: A suffix to be appended to each possible completion word (optional).
> __gitcomp ()
> {
> @@ -241,10 +242,22 @@ __gitcomp ()
> COMPREPLY=()
> ;;
> *)
> - local IFS=$'\n'
> - COMPREPLY=($(compgen -P "${2-}" \
> - -W "$(__gitcomp_1 "${1-}" "${4-}")" \
> - -- "$cur_"))
> + local i=0 c IFS=$' \t\n'
> + for c in $1; do
> + case $c in
> + "$cur_"*)
> + c="$c${4-}"
> + case $c in
> + --*=*|*.) ;;
> + *) c="$c " ;;
> + esac
> + COMPREPLY[$i]="${2-}$c"
> + i=$((++i))
> + ;;
> + *)
> + ;;
> + esac
> + done
This is not quite the same as before, is it? Before the suffix would
be taken into consideration for the comparison with $cur_, but not any
more.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [RFC/PATCH 5/5] completion: refactor __gitcomp_1
From: Felipe Contreras @ 2012-11-17 11:27 UTC (permalink / raw)
To: SZEDER Gábor
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <20121117105837.GC12052@goldbirke>
On Sat, Nov 17, 2012 at 11:58 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>> # The following function is based on code from:
>> @@ -249,10 +246,16 @@ __gitcomp ()
>> --*=)
>> ;;
>> *)
>> - local IFS=$'\n'
>> - __gitcompadd "$(__gitcomp_1 "${1-}" "${4-}")" "${2-}" "$cur_" ""
>> + local c IFS=$' \t\n'
>> + for c in ${1-}; do
>> + c=`__gitcomp_1 "$c${4-}"`
>
> 1. Backticks.
> 2. A subshell for every word in the wordlist?
Fine, lets make it hard for zsh then:
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -56,19 +56,6 @@ __gitdir ()
fi
}
-__gitcomp_1 ()
-{
- local c IFS=$' \t\n'
- for c in $1; do
- c="$c$2"
- case $c in
- --*=*|*.) ;;
- *) c="$c " ;;
- esac
- printf '%s\n' "$c"
- done
-}
-
# The following function is based on code from:
#
# bash_completion - programmable completion functions for bash 3.2+
@@ -241,12 +228,22 @@ __gitcomp ()
COMPREPLY=()
;;
*)
- local IFS=$'\n'
- COMPREPLY=($(compgen -P "${2-}" \
- -W "$(__gitcomp_1 "${1-}" "${4-}")" \
- -- "$cur_"))
+ local c i IFS=$' \t\n'
+ i=0
+ for c in ${1-}; do
+ c="$c${4-}"
+ case $c in
+ --*=*|*.) ;;
+ *) c="$c " ;;
+ esac
+ if [[ "$c" = "$cur_"* ]]; then
+ (( i++ ))
+ COMPREPLY[$i]="${2-}$c"
+ fi
+ done
;;
esac
+
}
# Generates completion reply with compgen from newline-separated possible
--
Felipe Contreras
^ permalink raw reply
* [PATCH 5/7] completion: fix expansion issues in __gitcomp_nl()
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
The compgen Bash-builtin performs expansion on all words in the
wordlist given to its -W option, breaking Git's completion script when
refs or filenames passed to __gitcomp_nl() contain expandable
substrings. At least one user can't use ref completion at all in a
repository, which contains tags with metacharacters like
Build%20V%20${bamboo.custom.jiraversion.name}%20Build%20721
Such a ref causes a failure in compgen as it tries to expand the
variable with invalid name.
Unfortunately, compgen has no option to turn off this expansion.
However, in __gitcomp_nl() we use only a small subset of compgen's
functionality, namely the filtering of matching possible completion
words and adding a prefix and/or suffix to each of those words. So,
to avoid compgen's problematic expansion we get rid of compgen, and
implement the equivalent functionality in a small awk script. The
reason for using awk instead of sed is that awk allows plain (i.e.
non-regexp) string comparison, making quoting of regexp characters in
the current word unnecessary.
Note, that such expandable words need quoting to be of any use on the
command line. This patch doesn't perform that quoting, but it could
be implemented later on top by extending the awk script to unquote
$cur and to quote matching words. Nonetheless, this patch still a
step forward, because at least refs can be completed even in the
presence of one with metacharacters.
Also update the function's description a bit.
Reported-by: Jeroen Meijer <jjgmeijer@hotmail.com>
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
awk doesn't have a prefixcmp(). I'm no awk wizard, so I'm not sure this
is the right way to do it.
contrib/completion/git-completion.bash | 17 +++++++++++++----
t/t9902-completion.sh | 4 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bc0657a2..65196ddd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -249,19 +249,28 @@ __gitcomp ()
esac
}
-# Generates completion reply with compgen from newline-separated possible
-# completion words by appending a space to all of them.
+# Generates completion reply for the word in $cur from newline-separated
+# possible completion words, appending a space to all of them.
# It accepts 1 to 4 arguments:
# 1: List of possible completion words, separated by a single newline.
# 2: A prefix to be added to each possible completion word (optional).
-# 3: Generate possible completion matches for this word (optional).
+# 3: Generate possible completion matches for this word instead of $cur
+# (optional).
# 4: A suffix to be appended to each possible completion word instead of
# the default space (optional). If specified but empty, nothing is
# appended.
__gitcomp_nl ()
{
local IFS=$'\n'
- COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
+ COMPREPLY=($(awk -v pfx="${2-}" -v sfx="${4- }" -v cur="${3-$cur}" '
+ BEGIN {
+ FS="\n";
+ len=length(cur);
+ }
+ {
+ if (cur == substr($1, 1, len))
+ print pfx$1sfx;
+ }' <<< "$1" ))
}
__git_heads ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a108ec1c..fa289324 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -246,7 +246,7 @@ test_expect_success '__gitcomp_nl - no suffix' '
test_cmp expected out
'
-test_expect_failure '__gitcomp_nl - doesnt fail because of invalid variable name' '
+test_expect_success '__gitcomp_nl - doesnt fail because of invalid variable name' '
(
__gitcomp_nl "$invalid_variable_name"
)
@@ -383,7 +383,7 @@ test_expect_success 'complete tree filename with spaces' '
EOF
'
-test_expect_failure 'complete tree filename with metacharacters' '
+test_expect_success 'complete tree filename with metacharacters' '
echo content >"name with \${meta}" &&
git add . &&
git commit -m meta &&
--
1.8.0.220.g4d14ece
^ permalink raw reply related
* [PATCH 7/7] completion: remove the now unused __gitcomp_1() internal helper function
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
Since the __gitcomp_1() helper is basically only an implementation
detail of __gitcomp() that exists solely for performance reasons (see
ab02dfe5 (bash completion: Improve responsiveness of git-log
completion, 2008-07-13)), it's quite unlikely that anyone uses or ever
used it besides __gitcomp(). And now __gitcomp() doesn't need it
anymore either, so delete it.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
contrib/completion/git-completion.bash | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 283ef99b..a281b28d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -56,19 +56,6 @@ __gitdir ()
fi
}
-__gitcomp_1 ()
-{
- local c IFS=$' \t\n'
- for c in $1; do
- c="$c$2"
- case $c in
- --*=*|*.) ;;
- *) c="$c " ;;
- esac
- printf '%s\n' "$c"
- done
-}
-
# The following function is based on code from:
#
# bash_completion - programmable completion functions for bash 3.2+
--
1.8.0.220.g4d14ece
^ permalink raw reply related
* [PATCH 6/7] completion: fix expansion issue in __gitcomp()
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
The compgen Bash-builtin performs expansion on all words in the
wordlist given to its -W option, breaking Git's completion script when
words passed to __gitcomp() contain expandable substrings.
In __gitcomp() we only use a small subset ot compgen's functionality,
namely the filtering of matching possible completion words and adding
a prefix to each of those words; suffix is added by the __gitcomp_1()
helper function instead. Now, since we already have to iterate over
all words in the wordlist to append a trailing space if necessary, we
can check in the same loop whether a word matches the word to be
completed and store matching words with possible prefix and/or suffix
in the COMPREPLY array. This way we achieve the same functionality
without the compgen builtin and, more importantly, without it's
problematic expansions.
An additional benefit, especially for Windows/MSysGit users, is the
loss of two subshells, one to run __gitcomp_1() and the other to run
the compgen builtin.
Note, that like the previous patch for __gitcomp_nl(), this patch
doesn't quote expandable words either, but that could be implemented
later on top by unquoting $cur and then quoting what get stored in
COMPREPLY.
Also update the function's description a bit.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
contrib/completion/git-completion.bash | 27 ++++++++++++++++++++-------
t/t9902-completion.sh | 2 +-
2 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 65196ddd..283ef99b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,12 +225,13 @@ _get_comp_words_by_ref ()
fi
fi
-# Generates completion reply with compgen, appending a space to possible
-# completion words, if necessary.
+# Generates completion reply for the word in $cur, appending a space to
+# possible completion words, if necessary.
# It accepts 1 to 4 arguments:
# 1: List of possible completion words.
# 2: A prefix to be added to each possible completion word (optional).
-# 3: Generate possible completion matches for this word (optional).
+# 3: Generate possible completion matches for this word instead of $cur
+# (optional).
# 4: A suffix to be appended to each possible completion word (optional).
__gitcomp ()
{
@@ -241,10 +242,22 @@ __gitcomp ()
COMPREPLY=()
;;
*)
- local IFS=$'\n'
- COMPREPLY=($(compgen -P "${2-}" \
- -W "$(__gitcomp_1 "${1-}" "${4-}")" \
- -- "$cur_"))
+ local i=0 c IFS=$' \t\n'
+ for c in $1; do
+ case $c in
+ "$cur_"*)
+ c="$c${4-}"
+ case $c in
+ --*=*|*.) ;;
+ *) c="$c " ;;
+ esac
+ COMPREPLY[$i]="${2-}$c"
+ i=$((++i))
+ ;;
+ *)
+ ;;
+ esac
+ done
;;
esac
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index fa289324..d08f4259 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -156,7 +156,7 @@ test_expect_success '__gitcomp - suffix' '
test_cmp expected out
'
-test_expect_failure '__gitcomp - doesnt fail because of invalid variable name' '
+test_expect_success '__gitcomp - doesnt fail because of invalid variable name' '
(
__gitcomp "$invalid_variable_name"
)
--
1.8.0.220.g4d14ece
^ permalink raw reply related
* [PATCH 4/7] completion: add tests for invalid variable name among completion words
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
The compgen Bash-builtin performs expansion on all words in the
wordlist given to its -W option, breaking Git's completion script in
various ways if one of those words can be expanded. The breakage can
be simply bogus possible completion words, but it can also be a
failure:
$ git branch '${foo.bar}'
$ git checkout <TAB>
bash: ${foo.bar}: bad substitution
${foo.bar} is an invalid variable name, which triggers the failure
when compgen attempts to expand it, completely breaking refs
completion. The same applies to e.g. completing the <tree>:<path>
notation when a filename contains a similar expandable substring.
Since both __gitcomp() and __gitcomp_nl() rely on compgen, both are
affected by this issue. So add a simple test for each of them to
check that such a word doesn't cause failures (but don't check the
resulting possible completion words for now, because that should be
quoted properly, and that's a separate topic).
Reported-by: Jeroen Meijer <jjgmeijer@hotmail.com>
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
t/t9902-completion.sh | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 32b3e8c4..a108ec1c 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -71,6 +71,7 @@ test_completion_long ()
}
newline=$'\n'
+invalid_variable_name="${foo.bar}"
test_expect_success '__gitcomp - trailing space - options' '
sed -e "s/Z$//" >expected <<-\EOF &&
@@ -155,6 +156,12 @@ test_expect_success '__gitcomp - suffix' '
test_cmp expected out
'
+test_expect_failure '__gitcomp - doesnt fail because of invalid variable name' '
+ (
+ __gitcomp "$invalid_variable_name"
+ )
+'
+
test_expect_success '__gitcomp_nl - trailing space' '
sed -e "s/Z$//" >expected <<-\EOF &&
maint Z
@@ -239,6 +246,12 @@ test_expect_success '__gitcomp_nl - no suffix' '
test_cmp expected out
'
+test_expect_failure '__gitcomp_nl - doesnt fail because of invalid variable name' '
+ (
+ __gitcomp_nl "$invalid_variable_name"
+ )
+'
+
test_expect_success 'basic' '
run_completion git "" &&
# built-in
--
1.8.0.220.g4d14ece
^ permalink raw reply related
* [PATCH 1/7] completion: make the 'basic' test more tester-friendly
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
The 'basic' test uses 'grep -q' to filter the resulting possible
completion words while looking for the presence or absence of certain
git commands, and relies on grep's exit status to indicate a failure.
This works fine as long as there are no errors. However, in case of a
failure it doesn't give any indication whatsoever about the reason of
the failure, i.e. which condition failed.
To make testers' life easier provide some output about the failed
condition: store the results of the filtering in a file and compare
its contents to the expected results by the good old test_cmp helper.
However, to actually get output from test_cmp in case of an error we
must make sure that test_cmp is always executed. Since in case of an
error grep's exit code aborts the test immediately, before running the
subsequent test_cmp, do the filtering using sed instead of grep.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
t/t9902-completion.sh | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 8fa025f9..b56759f7 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -158,14 +158,22 @@ test_expect_success '__gitcomp - suffix' '
test_expect_success 'basic' '
run_completion "git \"\"" &&
# built-in
- grep -q "^add \$" out &&
+ echo "add " >expected &&
+ sed -n "/^add \$/p" out >out2 &&
+ test_cmp expected out2 &&
# script
- grep -q "^filter-branch \$" out &&
+ echo "filter-branch " >expected &&
+ sed -n "/^filter-branch \$/p" out >out2 &&
+ test_cmp expected out2 &&
# plumbing
- ! grep -q "^ls-files \$" out &&
+ >expected &&
+ sed -n "/^ls-files \$/p" out >out2 &&
+ test_cmp expected out2 &&
run_completion "git f" &&
- ! grep -q -v "^f" out
+ >expected &&
+ sed -n "/^[^f]/p" out >out2 &&
+ test_cmp expected out2
'
test_expect_success 'double dash "git" itself' '
--
1.8.0.220.g4d14ece
^ permalink raw reply related
* [PATCH 3/7] completion: add tests for the __gitcomp_nl() completion helper function
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
Test __gitcomp_nl()'s basic functionality, i.e. that trailing space,
prefix, and suffix are added correctly.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
t/t9902-completion.sh | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 3af75872..32b3e8c4 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -155,6 +155,90 @@ test_expect_success '__gitcomp - suffix' '
test_cmp expected out
'
+test_expect_success '__gitcomp_nl - trailing space' '
+ sed -e "s/Z$//" >expected <<-\EOF &&
+ maint Z
+ master Z
+ EOF
+ (
+ declare -a COMPREPLY &&
+ refs="$(cat <<-\EOF
+ maint
+ master
+ next
+ pu
+ EOF
+ )" &&
+ cur="m" &&
+ __gitcomp_nl "$refs" &&
+ print_comp
+ ) &&
+ test_cmp expected out
+'
+
+test_expect_success '__gitcomp_nl - prefix' '
+ sed -e "s/Z$//" >expected <<-\EOF &&
+ --fixup=maint Z
+ --fixup=master Z
+ EOF
+ (
+ declare -a COMPREPLY &&
+ refs="$(cat <<-\EOF
+ maint
+ master
+ next
+ pu
+ EOF
+ )" &&
+ cur="--fixup=m" &&
+ __gitcomp_nl "$refs" "--fixup=" "m" &&
+ print_comp
+ ) &&
+ test_cmp expected out
+'
+
+test_expect_success '__gitcomp_nl - suffix' '
+ sed -e "s/Z$//" >expected <<-\EOF &&
+ branch.maint.Z
+ branch.master.Z
+ EOF
+ (
+ declare -a COMPREPLY &&
+ refs="$(cat <<-\EOF
+ maint
+ master
+ next
+ pu
+ EOF
+ )" &&
+ cur="branch.ma" &&
+ __gitcomp_nl "$refs" "branch." "ma" "." &&
+ print_comp
+ ) &&
+ test_cmp expected out
+'
+
+test_expect_success '__gitcomp_nl - no suffix' '
+ sed -e "s/Z$//" >expected <<-\EOF &&
+ maintZ
+ masterZ
+ EOF
+ (
+ declare -a COMPREPLY &&
+ refs="$(cat <<-\EOF
+ maint
+ master
+ next
+ pu
+ EOF
+ )" &&
+ cur="ma" &&
+ __gitcomp_nl "$refs" "" "ma" "" &&
+ print_comp
+ ) &&
+ test_cmp expected out
+'
+
test_expect_success 'basic' '
run_completion git "" &&
# built-in
--
1.8.0.220.g4d14ece
^ permalink raw reply related
* [PATCH 2/7] completion: fix args of run_completion() test helper
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
In-Reply-To: <1353150353-29874-1-git-send-email-szeder@ira.uka.de>
To simulate that the user hit 'git <TAB>, the 'basic' test sets up the
rather strange command line containing the two words
git ""
i.e. the second word on the command line consists of two double
quotes. This is not what happens for real, however, because after
'git <TAB>' the second word on the command line is just an empty
string. Luckily, the test works nevertheless.
Fix this by passing the command line to run_completion() as separate
words.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
t/t9902-completion.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index b56759f7..3af75872 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -49,7 +49,7 @@ run_completion ()
{
local -a COMPREPLY _words
local _cword
- _words=( $1 )
+ _words=( "$@" )
(( _cword = ${#_words[@]} - 1 ))
__git_wrap__git_main && print_comp
}
@@ -57,7 +57,7 @@ run_completion ()
test_completion ()
{
test $# -gt 1 && echo "$2" > expected
- run_completion "$@" &&
+ run_completion $1 &&
test_cmp expected out
}
@@ -156,7 +156,7 @@ test_expect_success '__gitcomp - suffix' '
'
test_expect_success 'basic' '
- run_completion "git \"\"" &&
+ run_completion git "" &&
# built-in
echo "add " >expected &&
sed -n "/^add \$/p" out >out2 &&
@@ -170,7 +170,7 @@ test_expect_success 'basic' '
sed -n "/^ls-files \$/p" out >out2 &&
test_cmp expected out2 &&
- run_completion "git f" &&
+ run_completion git f &&
>expected &&
sed -n "/^[^f]/p" out >out2 &&
test_cmp expected out2
--
1.8.0.220.g4d14ece
^ permalink raw reply related
* [PATCH 0/7] completion: fix expansion issues with compgen
From: SZEDER Gábor @ 2012-11-17 11:05 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Jeff King, Jonathan Nieder, Junio C Hamano,
SZEDER Gábor
This series fixes the expansion issues with compgen reported by Jeroen
Meijer a while ago in
http://article.gmane.org/gmane.comp.version-control.git/201596
The problem is that the compgen Bash-builtin performs expansion on all
words in the wordlist given to its -W option, breaking Git's completion
script when e.g. refs or filenames contain expandable substrings. Since
compgen has no option to turn off this expansion and we only use a small
subset of compgen's functionality, this series replaces compgen in
__gitcomp() and __gitcomp_nl() by some shell logic and a small awk script,
respectively.
Patches 1 and 2 are test enhancements and fixes, while 3 and 4 add new
tests to make sure I don't break anything and to demonstrate the issues,
respectively. The actual bugfixes are in patches 5 and 6. Finally, patch
7 is a cleanup made possible by patch 6 (could be squashed into 6).
In the future we might want/need to fill COMPREPLY directly when
completing a path in the <tree>:<path> notation instead using
__gitcomp_nl() there, because paths can contain newlines, too.
Compared to Felipe's series from yesterday, this series has more tests,
more descriptive commit messages, and it's faster. However, it doesn't
include his 1/5 (completion: get rid of empty COMPREPLY assignments).
Footnote: check the patchlist below, and notice how format-patch put
patches 4 and 5 into the same line.
SZEDER Gábor (7):
completion: make the 'basic' test more tester-friendly
completion: fix args of run_completion() test helper
completion: add tests for the __gitcomp_nl() completion helper
function
completion: add tests for invalid variable name among completion words completion: fix expansion issues in __gitcomp_nl()
completion: fix expansion issue in __gitcomp()
completion: remove the now unused __gitcomp_1() internal helper
function
contrib/completion/git-completion.bash | 57 ++++++++-------
t/t9902-completion.sh | 123 ++++++++++++++++++++++++++++++---
2 files changed, 147 insertions(+), 33 deletions(-)
--
1.8.0.220.g4d14ece
^ permalink raw reply
* Re: [RFC/PATCH 4/5] completion: get rid of compgen
From: SZEDER Gábor @ 2012-11-17 11:00 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353116298-11798-5-git-send-email-felipe.contreras@gmail.com>
On Sat, Nov 17, 2012 at 02:38:17AM +0100, Felipe Contreras wrote:
> The functionality we use is very simple, plus, this fixes a known
> breakage 'complete tree filename with metacharacters'.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> contrib/completion/git-completion.bash | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 975ae13..ad3e1fe 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -227,7 +227,11 @@ fi
>
> __gitcompadd ()
> {
> - COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
> + for x in $1; do
> + if [[ "$x" = "$3"* ]]; then
> + COMPREPLY+=("$2$x$4")
> + fi
> + done
The whole point of creating __gitcomp_nl() back then was to fill
COMPREPLY without iterating through all words in the wordlist, making
completion faster for large number of words, e.g. a lot of refs, or
later a lot of symbols for 'git grep' in a larger project.
The loop here kills that optimization.
> }
>
> # Generates completion reply with compgen, appending a space to possible
> --
> 1.8.0
^ permalink raw reply
* Re: [RFC/PATCH 3/5] completion: trivial test improvement
From: SZEDER Gábor @ 2012-11-17 10:59 UTC (permalink / raw)
To: Felipe Contreras
Cc: git, Junio C Hamano, Björn Gustavsson, Shawn O. Pearce,
Robert Zeh, Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353116298-11798-4-git-send-email-felipe.contreras@gmail.com>
On Sat, Nov 17, 2012 at 02:38:16AM +0100, Felipe Contreras wrote:
> Instead of passing a dummy "", let's check if the last character is a
> space, and then move the _cword accordingly.
>
> Apparently we were passing "" all the way to compgen, which fortunately
> expanded it to nothing.
Glad you noticed it, too.
I posted an alternative fix (without any new conditions in the code
path) a while ago:
http://article.gmane.org/gmane.comp.version-control.git/206525
Will repost it as part of my series shortly.
> Lets do the right thing though.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> t/t9902-completion.sh | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index cbd0fb6..0b5e1f5 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -51,6 +51,7 @@ run_completion ()
> local _cword
> _words=( $1 )
> (( _cword = ${#_words[@]} - 1 ))
> + test "${1: -1}" == ' ' && (( _cword += 1 ))
> __git_wrap__git_main && print_comp
> }
>
> @@ -156,7 +157,7 @@ test_expect_success '__gitcomp - suffix' '
> '
>
> test_expect_success 'basic' '
> - run_completion "git \"\"" &&
> + run_completion "git " &&
> # built-in
> grep -q "^add \$" out &&
> # script
> --
> 1.8.0
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox