Git development
 help / color / mirror / Atom feed
* [PATCH 4/4] tree_entry_interesting: do basedir compare on wildcard patterns when possible
From: Nguyễn Thái Ngọc Duy @ 2012-11-18  9:13 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1353229989-13075-1-git-send-email-pclouds@gmail.com>

Currently we treat "*.c" and "path/to/*.c" the same way. Which means
we check all possible paths in repo against "path/to/*.c". One could
see that "path/elsewhere/foo.c" obviously cannot match "path/to/*.c"
and we only need to check all paths _inside_ "path/to/" against that
pattern.

This patch checks the leading fixed part of a pathspec against base
directory and exit early if possible. We could even optimize further
in "path/to/something*.c" case (i.e. check the fixed part against
name_entry as well) but that's more complicated and probably does not
gain us much.

-O2 build on linux-2.6, without and with this patch respectively:

$ time git rev-list --quiet HEAD -- 'drivers/*.c'

real    1m9.484s
user    1m9.128s
sys     0m0.181s

$ time ~/w/git/git rev-list --quiet HEAD -- 'drivers/*.c'

real    0m15.710s
user    0m15.564s
sys     0m0.107s

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 tree-walk.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+), 1 deletion(-)

diff --git a/tree-walk.c b/tree-walk.c
index 42fe610..dcc1015 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -573,6 +573,52 @@ static int match_dir_prefix(const char *base,
 }
 
 /*
+ * Perform matching on the leading non-wildcard part of
+ * pathspec. item->nowildcard_len must be greater than zero. Return
+ * non-zero if base is matched.
+ */
+static int match_wildcard_base(const struct pathspec_item *item,
+			       const char *base, int baselen,
+			       int *matched)
+{
+	const char *match = item->match;
+	/* the wildcard part is not considered in this function */
+	int matchlen = item->nowildcard_len;
+
+	if (baselen) {
+		int dirlen;
+		/*
+		 * Return early if base is longer than the
+		 * non-wildcard part but it does not match.
+		 */
+		if (baselen >= matchlen) {
+			*matched = matchlen;
+			return !strncmp(base, match, matchlen);
+		}
+
+		dirlen = matchlen;
+		while (dirlen && match[dirlen - 1] != '/')
+			dirlen--;
+
+		/* Return early if base is shorter than the
+		   non-wildcard part but it does not match. Note that
+		   base ends with '/' so we are sure it really matches
+		   directory */
+		if (strncmp(base, match, baselen))
+			return 0;
+		*matched = baselen;
+	} else
+		*matched = 0;
+	/*
+	 * we could have checked entry against the non-wildcard part
+	 * that is not in base and does similar never_interesting
+	 * optimization as in match_entry. For now just be happy with
+	 * base comparison.
+	 */
+	return entry_interesting;
+}
+
+/*
  * Is a tree entry interesting given the pathspec we have?
  *
  * Pre-condition: either baselen == base_offset (i.e. empty path)
@@ -602,7 +648,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
 		const struct pathspec_item *item = ps->items+i;
 		const char *match = item->match;
 		const char *base_str = base->buf + base_offset;
-		int matchlen = item->len;
+		int matchlen = item->len, matched = 0;
 
 		if (baselen >= matchlen) {
 			/* If it doesn't match, move along... */
@@ -647,9 +693,24 @@ match_wildcards:
 		if (item->nowildcard_len == item->len)
 			continue;
 
+		if (item->nowildcard_len &&
+		    !match_wildcard_base(item, base_str, baselen, &matched))
+			return entry_not_interesting;
+
 		/*
 		 * Concatenate base and entry->path into one and do
 		 * fnmatch() on it.
+		 *
+		 * While we could avoid concatenation in certain cases
+		 * [1], which saves a memcpy and potentially a
+		 * realloc, it turns out not worth it. Measurement on
+		 * linux-2.6 does not show any clear improvements,
+		 * partly because of the nowildcard_len optimization
+		 * in git_fnmatch(). Avoid micro-optimizations here.
+		 *
+		 * [1] if match_wildcard_base() says the base
+		 * directory is already matched, we only need to match
+		 * the rest, which is shorter so _in theory_ faster.
 		 */
 
 		strbuf_add(base, entry->path, pathlen);
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* Re: [PATCH 1/7] completion: make the 'basic' test more tester-friendly
From: Felipe Contreras @ 2012-11-18  9:38 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: SZEDER Gábor, git, Jeff King, Junio C Hamano
In-Reply-To: <20121117230022.GA3815@elie.Belkin>

On Sun, Nov 18, 2012 at 12:00 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> SZEDER Gábor wrote:
>
>> 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.
> [...]
>> 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.
>
> Looks good.  I wonder if this points to the need for a test_grep
> helper more generally.

It does.

-- 
Felipe Contreras

^ permalink raw reply

* Re: [PATCH 1/7] completion: make the 'basic' test more tester-friendly
From: Felipe Contreras @ 2012-11-18  9:39 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: git, Jeff King, Jonathan Nieder, Junio C Hamano
In-Reply-To: <1353150353-29874-2-git-send-email-szeder@ira.uka.de>

On Sat, Nov 17, 2012 at 12:05 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> 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' '

It's not very useful to see a single failure without context. Maybe this:

--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -191,18 +191,20 @@ test_expect_success '__gitcomp_nl - doesnt fail
because of invalid variable name

 test_expect_success 'basic' '
        run_completion "git " &&
+       (
        # built-in
-       echo "add " >expected &&
-       sed -n "/^add \$/p" out >out2 &&
-       test_cmp expected out2 &&
+       sed -n "/^add $/p" out &&
        # script
-       echo "filter-branch " >expected &&
-       sed -n "/^filter-branch \$/p" out >out2 &&
-       test_cmp expected out2 &&
+       sed -n "/^filter-branch $/p" out &&
        # plumbing
-       >expected &&
-       sed -n "/^ls-files \$/p" out >out2 &&
-       test_cmp expected out2 &&
+       sed -n "/^ls-files $/p" out
+       ) > actual &&
+
+       sed -e 's/Z$//' > expected <<-EOF &&
+       add Z
+       filter-branch Z
+       EOF
+       test_cmp expected actual &&

        run_completion "git f" &&
        >expected &&

-- 
Felipe Contreras

^ permalink raw reply

* Re: Local clones aka forks disk size optimization
From: Sitaram Chamarty @ 2012-11-18 10:42 UTC (permalink / raw)
  To: Enrico Weigelt; +Cc: Michael J Gruber, Andrew Ardill, Javier Domingo, git
In-Reply-To: <ccb8680a-97ad-4cf8-95d0-5e21f60494f4@zcs>

On Fri, Nov 16, 2012 at 11:34 PM, Enrico Weigelt <enrico.weigelt@vnc.biz> wrote:
>
>> Provide one "main" clone which is bare, pulls automatically, and is
>> there to stay (no pruning), so that all others can use that as a
>> reliable alternates source.
>
> The problem here, IMHO, is the assumption, that the main repo will
> never be cleaned up. But what to do if you dont wanna let it grow
> forever ?

That's not the only problem.  I believe you only get the savings when
the main repo gets the commits first.  Which is probably ok most of
the time but it's worth mentioning.

>
> hmm, distributed GC is a tricky problem.

Except for one little issue (see other thread, subject line "cloning a
namespace downloads all the objects"), namespaces appear to do
everything we want in terms of the typical use cases for alternates,
and/or 'git clone -l', at least on the server side.

^ permalink raw reply

* [PATCH v3 0/6] completion: test consolidations
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Felipe Contreras

These started from a discussion with SZEDER, but then I realized there were
many improvements possible.

Changes since v2:

 * Updated comments and commit messages

Changes since v1:

 * A lot more cleanups

Felipe Contreras (6):
  completion: add comment for test_completion()
  completion: standardize final space marker in tests
  completion: simplify tests using test_completion_long()
  completion: consolidate test_completion*() tests
  completion: refactor __gitcomp related tests
  completion: simplify __gitcomp() test helper

 t/t9902-completion.sh | 133 +++++++++++++++++++-------------------------------
 1 file changed, 51 insertions(+), 82 deletions(-)

-- 
1.8.0

^ permalink raw reply

* [PATCH v3 1/6] completion: add comment for test_completion()
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>

So that it's easier to understand what it does.

Also, make sure we pass only the first argument for completion.
Shouldn't cause any functional changes because run_completion only
checks $1.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 8fa025f..2276a37 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -54,10 +54,14 @@ run_completion ()
 	__git_wrap__git_main && print_comp
 }
 
+# Test high-level completion
+# Arguments are:
+# 1: current command line
+# 2: expected completion
 test_completion ()
 {
 	test $# -gt 1 && echo "$2" > expected
-	run_completion "$@" &&
+	run_completion "$1" &&
 	test_cmp expected out
 }
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH v3 2/6] completion: standardize final space marker in tests
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>

The rest of the code uses ' Z$'. Lets use that for
test_completion_long() as well.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 2276a37..f4c7342 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -66,11 +66,10 @@ test_completion ()
 }
 
 # Like test_completion, but reads expectation from stdin,
-# which is convenient when it is multiline. We also process "_" into
-# spaces to make test vectors more readable.
+# which is convenient when it is multiline.
 test_completion_long ()
 {
-	tr _ " " >expected &&
+	sed -e 's/Z$//' > expected &&
 	test_completion "$1"
 }
 
@@ -252,24 +251,24 @@ test_expect_success 'setup for ref completion' '
 
 test_expect_success 'checkout completes ref names' '
 	test_completion_long "git checkout m" <<-\EOF
-	master_
-	mybranch_
-	mytag_
+	master Z
+	mybranch Z
+	mytag Z
 	EOF
 '
 
 test_expect_success 'show completes all refs' '
 	test_completion_long "git show m" <<-\EOF
-	master_
-	mybranch_
-	mytag_
+	master Z
+	mybranch Z
+	mytag Z
 	EOF
 '
 
 test_expect_success '<ref>: completes paths' '
 	test_completion_long "git show mytag:f" <<-\EOF
-	file1_
-	file2_
+	file1 Z
+	file2 Z
 	EOF
 '
 
@@ -278,7 +277,7 @@ test_expect_success 'complete tree filename with spaces' '
 	git add . &&
 	git commit -m spaces &&
 	test_completion_long "git show HEAD:nam" <<-\EOF
-	name with spaces_
+	name with spaces Z
 	EOF
 '
 
@@ -287,8 +286,8 @@ test_expect_failure 'complete tree filename with metacharacters' '
 	git add . &&
 	git commit -m meta &&
 	test_completion_long "git show HEAD:nam" <<-\EOF
-	name with ${meta}_
-	name with spaces_
+	name with ${meta} Z
+	name with spaces Z
 	EOF
 '
 
-- 
1.8.0

^ permalink raw reply related

* [PATCH v3 3/6] completion: simplify tests using test_completion_long()
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>

No need to duplicate that functionality.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index f4c7342..6a250ad 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -172,7 +172,7 @@ test_expect_success 'basic' '
 '
 
 test_expect_success 'double dash "git" itself' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_completion_long "git --" <<-\EOF
 	--paginate Z
 	--no-pager Z
 	--git-dir=
@@ -187,11 +187,10 @@ test_expect_success 'double dash "git" itself' '
 	--no-replace-objects Z
 	--help Z
 	EOF
-	test_completion "git --"
 '
 
 test_expect_success 'double dash "git checkout"' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_completion_long "git checkout --" <<-\EOF
 	--quiet Z
 	--ours Z
 	--theirs Z
@@ -202,17 +201,15 @@ test_expect_success 'double dash "git checkout"' '
 	--orphan Z
 	--patch Z
 	EOF
-	test_completion "git checkout --"
 '
 
 test_expect_success 'general options' '
 	test_completion "git --ver" "--version " &&
 	test_completion "git --hel" "--help " &&
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_completion_long "git --exe" <<-\EOF &&
 	--exec-path Z
 	--exec-path=
 	EOF
-	test_completion "git --exe" &&
 	test_completion "git --htm" "--html-path " &&
 	test_completion "git --pag" "--paginate " &&
 	test_completion "git --no-p" "--no-pager " &&
-- 
1.8.0

^ permalink raw reply related

* [PATCH v3 4/6] completion: consolidate test_completion*() tests
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>

No need to have two versions; if a second argument is specified, use
that, otherwise use stdin.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 6a250ad..90a9a91 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -60,19 +60,15 @@ run_completion ()
 # 2: expected completion
 test_completion ()
 {
-	test $# -gt 1 && echo "$2" > expected
+	if [ $# -gt 1 ]; then
+		echo "$2" > expected
+	else
+		sed -e 's/Z$//' > expected
+	fi &&
 	run_completion "$1" &&
 	test_cmp expected out
 }
 
-# Like test_completion, but reads expectation from stdin,
-# which is convenient when it is multiline.
-test_completion_long ()
-{
-	sed -e 's/Z$//' > expected &&
-	test_completion "$1"
-}
-
 newline=$'\n'
 
 test_expect_success '__gitcomp - trailing space - options' '
@@ -172,7 +168,7 @@ test_expect_success 'basic' '
 '
 
 test_expect_success 'double dash "git" itself' '
-	test_completion_long "git --" <<-\EOF
+	test_completion "git --" <<-\EOF
 	--paginate Z
 	--no-pager Z
 	--git-dir=
@@ -190,7 +186,7 @@ test_expect_success 'double dash "git" itself' '
 '
 
 test_expect_success 'double dash "git checkout"' '
-	test_completion_long "git checkout --" <<-\EOF
+	test_completion "git checkout --" <<-\EOF
 	--quiet Z
 	--ours Z
 	--theirs Z
@@ -206,7 +202,7 @@ test_expect_success 'double dash "git checkout"' '
 test_expect_success 'general options' '
 	test_completion "git --ver" "--version " &&
 	test_completion "git --hel" "--help " &&
-	test_completion_long "git --exe" <<-\EOF &&
+	test_completion "git --exe" <<-\EOF &&
 	--exec-path Z
 	--exec-path=
 	EOF
@@ -247,7 +243,7 @@ test_expect_success 'setup for ref completion' '
 '
 
 test_expect_success 'checkout completes ref names' '
-	test_completion_long "git checkout m" <<-\EOF
+	test_completion "git checkout m" <<-\EOF
 	master Z
 	mybranch Z
 	mytag Z
@@ -255,7 +251,7 @@ test_expect_success 'checkout completes ref names' '
 '
 
 test_expect_success 'show completes all refs' '
-	test_completion_long "git show m" <<-\EOF
+	test_completion "git show m" <<-\EOF
 	master Z
 	mybranch Z
 	mytag Z
@@ -263,7 +259,7 @@ test_expect_success 'show completes all refs' '
 '
 
 test_expect_success '<ref>: completes paths' '
-	test_completion_long "git show mytag:f" <<-\EOF
+	test_completion "git show mytag:f" <<-\EOF
 	file1 Z
 	file2 Z
 	EOF
@@ -273,7 +269,7 @@ test_expect_success 'complete tree filename with spaces' '
 	echo content >"name with spaces" &&
 	git add . &&
 	git commit -m spaces &&
-	test_completion_long "git show HEAD:nam" <<-\EOF
+	test_completion "git show HEAD:nam" <<-\EOF
 	name with spaces Z
 	EOF
 '
@@ -282,7 +278,7 @@ test_expect_failure 'complete tree filename with metacharacters' '
 	echo content >"name with \${meta}" &&
 	git add . &&
 	git commit -m meta &&
-	test_completion_long "git show HEAD:nam" <<-\EOF
+	test_completion "git show HEAD:nam" <<-\EOF
 	name with ${meta} Z
 	name with spaces Z
 	EOF
-- 
1.8.0

^ permalink raw reply related

* [PATCH v3 5/6] completion: refactor __gitcomp related tests
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>

Lots of duplicated code removed!

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 76 ++++++++++++++++++---------------------------------
 1 file changed, 27 insertions(+), 49 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 90a9a91..fba632f 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -71,87 +71,65 @@ test_completion ()
 
 newline=$'\n'
 
-test_expect_success '__gitcomp - trailing space - options' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
-	--reuse-message=Z
-	--reedit-message=Z
-	--reset-author Z
-	EOF
+# Test __gitcomp
+# Arguments are:
+# 1: current word (cur)
+# -: the rest are passed to __gitcomp
+test_gitcomp ()
+{
+	sed -e 's/Z$//' > expected &&
 	(
 		local -a COMPREPLY &&
-		cur="--re" &&
-		__gitcomp "--dry-run --reuse-message= --reedit-message=
-				--reset-author" &&
+		cur="$1" &&
+		shift &&
+		__gitcomp "$@" &&
 		IFS="$newline" &&
 		echo "${COMPREPLY[*]}" > out
 	) &&
 	test_cmp expected out
+}
+
+test_expect_success '__gitcomp - trailing space - options' '
+	test_gitcomp "--re" "--dry-run --reuse-message= --reedit-message=
+		--reset-author" <<-EOF
+	--reuse-message=Z
+	--reedit-message=Z
+	--reset-author Z
+	EOF
 '
 
 test_expect_success '__gitcomp - trailing space - config keys' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_gitcomp "br" "branch. branch.autosetupmerge
+		branch.autosetuprebase browser." <<-\EOF
 	branch.Z
 	branch.autosetupmerge Z
 	branch.autosetuprebase Z
 	browser.Z
 	EOF
-	(
-		local -a COMPREPLY &&
-		cur="br" &&
-		__gitcomp "branch. branch.autosetupmerge
-				branch.autosetuprebase browser." &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
-	test_cmp expected out
 '
 
 test_expect_success '__gitcomp - option parameter' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_gitcomp "--strategy=re" "octopus ours recursive resolve subtree" \
+		"" "re" <<-\EOF
 	recursive Z
 	resolve Z
 	EOF
-	(
-		local -a COMPREPLY &&
-		cur="--strategy=re" &&
-		__gitcomp "octopus ours recursive resolve subtree
-			" "" "re" &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
-	test_cmp expected out
 '
 
 test_expect_success '__gitcomp - prefix' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_gitcomp "branch.me" "remote merge mergeoptions rebase" \
+		"branch.maint." "me" <<-\EOF
 	branch.maint.merge Z
 	branch.maint.mergeoptions Z
 	EOF
-	(
-		local -a COMPREPLY &&
-		cur="branch.me" &&
-		__gitcomp "remote merge mergeoptions rebase
-			" "branch.maint." "me" &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
-	test_cmp expected out
 '
 
 test_expect_success '__gitcomp - suffix' '
-	sed -e "s/Z$//" >expected <<-\EOF &&
+	test_gitcomp "branch.me" "master maint next pu" "branch." \
+		"ma" "." <<-\EOF
 	branch.master.Z
 	branch.maint.Z
 	EOF
-	(
-		local -a COMPREPLY &&
-		cur="branch.me" &&
-		__gitcomp "master maint next pu
-			" "branch." "ma" "." &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
-	test_cmp expected out
 '
 
 test_expect_success 'basic' '
-- 
1.8.0

^ permalink raw reply related

* [PATCH v3 6/6] completion: simplify __gitcomp() test helper
From: Felipe Contreras @ 2012-11-18 10:51 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Felipe Contreras
In-Reply-To: <1353235917-13059-1-git-send-email-felipe.contreras@gmail.com>

By using print_comp as suggested by SZEDER Gábor.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index fba632f..42db3da 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -69,23 +69,18 @@ test_completion ()
 	test_cmp expected out
 }
 
-newline=$'\n'
-
 # Test __gitcomp
 # Arguments are:
 # 1: current word (cur)
 # -: the rest are passed to __gitcomp
 test_gitcomp ()
 {
+	local -a COMPREPLY &&
 	sed -e 's/Z$//' > expected &&
-	(
-		local -a COMPREPLY &&
-		cur="$1" &&
-		shift &&
-		__gitcomp "$@" &&
-		IFS="$newline" &&
-		echo "${COMPREPLY[*]}" > out
-	) &&
+	cur="$1" &&
+	shift &&
+	__gitcomp "$@" &&
+	print_comp &&
 	test_cmp expected out
 }
 
-- 
1.8.0

^ permalink raw reply related

* [RFC/PATCH v2 1/8] completion: trivial test improvement
From: Felipe Contreras @ 2012-11-18 10:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Felipe Contreras,
	Björn Gustavsson, Shawn O. Pearce, Robert Zeh,
	Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353236213-13471-1-git-send-email-felipe.contreras@gmail.com>

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.

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 8fa025f..79a2453 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -50,6 +50,7 @@ run_completion ()
 	local -a COMPREPLY _words
 	local _cword
 	_words=( $1 )
+	test "${1: -1}" == ' ' && _words+=('')
 	(( _cword = ${#_words[@]} - 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 related

* [RFC/PATCH v2 0/8] completion: compgen/compadd cleanups
From: Felipe Contreras @ 2012-11-18 10:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Felipe Contreras,
	Björn Gustavsson, Shawn O. Pearce, Robert Zeh,
	Peter van der Does, Jonathan Nieder, Jeff King

Hi,

These were hinted by SZEDER, so I went ahead and implemented the changes trying
to keep in mind the new zsh completion werapper. The resulting code should be
more efficient, and a known breakage is fixed.

The first two patches are transplanted from another patch series which most
likely is not going to use them.

I also borrowed the tests from another patch series from SZEDER, but they are
not meant to be applied through this series.

SZEDER implemented some of this, but I prefer my version of the patches.

Felipe Contreras (8):
  completion: trivial test improvement
  completion: get rid of empty COMPREPLY assignments
  completion: add new __gitcompadd helper
  tmp: completion: add SZEDER's tests
  completion: get rid of compgen
  tmp: completion: mark SZEDER's tests as fixed
  completion: get rid of __gitcomp_1
  completion: small optimization

 contrib/completion/git-completion.bash | 77 ++++++++++++----------------------
 t/t9902-completion.sh                  | 66 ++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 52 deletions(-)

-- 
1.8.0

^ permalink raw reply

* [RFC/PATCH v2 3/8] completion: add new __gitcompadd helper
From: Felipe Contreras @ 2012-11-18 10:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Felipe Contreras,
	Björn Gustavsson, Shawn O. Pearce, Robert Zeh,
	Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353236213-13471-1-git-send-email-felipe.contreras@gmail.com>

The idea is to never touch the COMPREPLY variable directly.

This allows other completion systems override __gitcompadd, and do
something different instead.

Also, this allows the simplification of the completion tests (separate
patch).

There should be no functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c2fb061..19e18a8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,6 +225,11 @@ _get_comp_words_by_ref ()
 fi
 fi
 
+__gitcompadd ()
+{
+	COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
+}
+
 # Generates completion reply with compgen, appending a space to possible
 # completion words, if necessary.
 # It accepts 1 to 4 arguments:
@@ -241,9 +246,7 @@ __gitcomp ()
 		;;
 	*)
 		local IFS=$'\n'
-		COMPREPLY=($(compgen -P "${2-}" \
-			-W "$(__gitcomp_1 "${1-}" "${4-}")" \
-			-- "$cur_"))
+		__gitcompadd "$(__gitcomp_1 "${1-}" "${4-}")" "${2-}" "$cur_" ""
 		;;
 	esac
 }
@@ -260,7 +263,7 @@ __gitcomp ()
 __gitcomp_nl ()
 {
 	local IFS=$'\n'
-	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
+	__gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
 }
 
 __git_heads ()
@@ -1605,7 +1608,7 @@ _git_config ()
 		local remote="${prev#remote.}"
 		remote="${remote%.fetch}"
 		if [ -z "$cur" ]; then
-			COMPREPLY=("refs/heads/")
+			__gitcompadd "refs/heads/"
 			return
 		fi
 		__gitcomp_nl "$(__git_refs_remotes "$remote")"
-- 
1.8.0

^ permalink raw reply related

* [RFC/PATCH v2 2/8] completion: get rid of empty COMPREPLY assignments
From: Felipe Contreras @ 2012-11-18 10:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Felipe Contreras,
	Björn Gustavsson, Shawn O. Pearce, Robert Zeh,
	Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353236213-13471-1-git-send-email-felipe.contreras@gmail.com>

There's no functional reason for those, the only purpose they are
supposed to serve is to say "we don't provide any words here", but even
for that it's not used consitently.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bc0657a..c2fb061 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -238,7 +238,6 @@ __gitcomp ()
 
 	case "$cur_" in
 	--*=)
-		COMPREPLY=()
 		;;
 	*)
 		local IFS=$'\n'
@@ -486,7 +485,6 @@ __git_complete_remote_or_refspec ()
 			case "$cmd" in
 			push) no_complete_refspec=1 ;;
 			fetch)
-				COMPREPLY=()
 				return
 				;;
 			*) ;;
@@ -502,7 +500,6 @@ __git_complete_remote_or_refspec ()
 		return
 	fi
 	if [ $no_complete_refspec = 1 ]; then
-		COMPREPLY=()
 		return
 	fi
 	[ "$remote" = "." ] && remote=
@@ -776,7 +773,6 @@ _git_am ()
 			"
 		return
 	esac
-	COMPREPLY=()
 }
 
 _git_apply ()
@@ -796,7 +792,6 @@ _git_apply ()
 			"
 		return
 	esac
-	COMPREPLY=()
 }
 
 _git_add ()
@@ -811,7 +806,6 @@ _git_add ()
 			"
 		return
 	esac
-	COMPREPLY=()
 }
 
 _git_archive ()
@@ -856,7 +850,6 @@ _git_bisect ()
 		__gitcomp_nl "$(__git_refs)"
 		;;
 	*)
-		COMPREPLY=()
 		;;
 	esac
 }
@@ -969,7 +962,6 @@ _git_clean ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_clone ()
@@ -993,7 +985,6 @@ _git_clone ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_commit ()
@@ -1027,7 +1018,6 @@ _git_commit ()
 			"
 		return
 	esac
-	COMPREPLY=()
 }
 
 _git_describe ()
@@ -1152,7 +1142,6 @@ _git_fsck ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_gc ()
@@ -1163,7 +1152,6 @@ _git_gc ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_gitk ()
@@ -1240,7 +1228,6 @@ _git_init ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_ls_files ()
@@ -1259,7 +1246,6 @@ _git_ls_files ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_ls_remote ()
@@ -1375,7 +1361,6 @@ _git_mergetool ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_merge_base ()
@@ -1391,7 +1376,6 @@ _git_mv ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_name_rev ()
@@ -1681,7 +1665,6 @@ _git_config ()
 		return
 		;;
 	*.*)
-		COMPREPLY=()
 		return
 		;;
 	esac
@@ -2061,7 +2044,6 @@ _git_remote ()
 		__gitcomp "$c"
 		;;
 	*)
-		COMPREPLY=()
 		;;
 	esac
 }
@@ -2105,7 +2087,6 @@ _git_rm ()
 		return
 		;;
 	esac
-	COMPREPLY=()
 }
 
 _git_shortlog ()
@@ -2174,8 +2155,6 @@ _git_stash ()
 		*)
 			if [ -z "$(__git_find_on_cmdline "$save_opts")" ]; then
 				__gitcomp "$subcommands"
-			else
-				COMPREPLY=()
 			fi
 			;;
 		esac
@@ -2188,14 +2167,12 @@ _git_stash ()
 			__gitcomp "--index --quiet"
 			;;
 		show,--*|drop,--*|branch,--*)
-			COMPREPLY=()
 			;;
 		show,*|apply,*|drop,*|pop,*|branch,*)
 			__gitcomp_nl "$(git --git-dir="$(__gitdir)" stash list \
 					| sed -n -e 's/:.*//p')"
 			;;
 		*)
-			COMPREPLY=()
 			;;
 		esac
 	fi
@@ -2312,7 +2289,6 @@ _git_svn ()
 			__gitcomp "--revision= --parent"
 			;;
 		*)
-			COMPREPLY=()
 			;;
 		esac
 	fi
@@ -2337,13 +2313,10 @@ _git_tag ()
 
 	case "$prev" in
 	-m|-F)
-		COMPREPLY=()
 		;;
 	-*|tag)
 		if [ $f = 1 ]; then
 			__gitcomp_nl "$(__git_tags)"
-		else
-			COMPREPLY=()
 		fi
 		;;
 	*)
-- 
1.8.0

^ permalink raw reply related

* [RFC/PATCH v2 4/8] tmp: completion: add SZEDER's tests
From: Felipe Contreras @ 2012-11-18 10:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Felipe Contreras,
	Björn Gustavsson, Shawn O. Pearce, Robert Zeh,
	Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353236213-13471-1-git-send-email-felipe.contreras@gmail.com>

Separate patch series, but the results are relevant.

+ modifications from me.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 79a2453..35fc31b 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -73,6 +73,24 @@ test_completion_long ()
 
 newline=$'\n'
 
+# Test __gitcomp_nl
+# Arguments are:
+# 1: current word (cur)
+# -: the rest are passed to __gitcomp_nl
+test_gitcomp_nl ()
+{
+	local -a COMPREPLY &&
+	sed -e 's/Z$//' > expected &&
+	cur="$1" &&
+	shift &&
+	__gitcomp_nl "$@" &&
+	print_comp &&
+	cp expected out /tmp
+	test_cmp expected out
+}
+
+invalid_variable_name="${foo.bar}"
+
 test_expect_success '__gitcomp - trailing space - options' '
 	sed -e "s/Z$//" >expected <<-\EOF &&
 	--reuse-message=Z
@@ -156,6 +174,49 @@ test_expect_success '__gitcomp - suffix' '
 	test_cmp expected out
 '
 
+test_expect_failure '__gitcomp - doesnt fail because of invalid variable name' '
+	__gitcomp "$invalid_variable_name"
+'
+
+read -r -d "" refs <<-\EOF
+maint
+master
+next
+pu
+EOF
+
+test_expect_success '__gitcomp_nl - trailing space' '
+	test_gitcomp_nl "m" "$refs" <<-EOF
+	maint Z
+	master Z
+	EOF
+'
+
+test_expect_success '__gitcomp_nl - prefix' '
+	test_gitcomp_nl "--fixup=m" "$refs" "--fixup=" "m" <<-EOF
+	--fixup=maint Z
+	--fixup=master Z
+	EOF
+'
+
+test_expect_success '__gitcomp_nl - suffix' '
+	test_gitcomp_nl "branch.ma" "$refs" "branch." "ma" "." <<-\EOF
+	branch.maint.Z
+	branch.master.Z
+	EOF
+'
+
+test_expect_success '__gitcomp_nl - no suffix' '
+	test_gitcomp_nl "ma" "$refs" "" "ma" "" <<-\EOF
+	maintZ
+	masterZ
+	EOF
+'
+
+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

^ permalink raw reply related

* [RFC/PATCH v2 5/8] completion: get rid of compgen
From: Felipe Contreras @ 2012-11-18 10:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Felipe Contreras,
	Björn Gustavsson, Shawn O. Pearce, Robert Zeh,
	Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353236213-13471-1-git-send-email-felipe.contreras@gmail.com>

The functionality we use from compgen is not much, we can do the same
manually, with drastical improvements in speed, specially when dealing
with only a few rods.

This patch also has the sideffect that brekage reported by Jeroen Meijer
and SZEDER Gábor gets fixed because we no longer expand the resulting
words.

Other solutions with awk and sed are possible and more efficient when
dealing with tens of thousands of words. But is rarely the case. For the
common cases, this solution is faster.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 15 ++++++++++-----
 t/t9902-completion.sh                  |  2 +-
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 19e18a8..0b72f24 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -227,11 +227,16 @@ fi
 
 __gitcompadd ()
 {
-	COMPREPLY=($(compgen -W "$1" -P "$2" -S "$4" -- "$3"))
+	local i=0
+	for x in $1; do
+		if [[ "$x" == "$3"* ]]; then
+			COMPREPLY[i++]="$2$x$4"
+		fi
+	done
 }
 
-# Generates completion reply with compgen, appending a space to possible
-# completion words, if necessary.
+# Generates completion reply, 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).
@@ -251,8 +256,8 @@ __gitcomp ()
 	esac
 }
 
-# Generates completion reply with compgen from newline-separated possible
-# completion words by appending a space to all of them.
+# Generates completion reply from newline-separated possible completion words
+# by 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).
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 35fc31b..7fb5b50 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -340,7 +340,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

^ permalink raw reply related

* [RFC/PATCH v2 6/8] tmp: completion: mark SZEDER's tests as fixed
From: Felipe Contreras @ 2012-11-18 10:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Felipe Contreras,
	Björn Gustavsson, Shawn O. Pearce, Robert Zeh,
	Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353236213-13471-1-git-send-email-felipe.contreras@gmail.com>

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7fb5b50..4e2f4fb 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -174,7 +174,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"
 '
 
@@ -213,7 +213,7 @@ test_expect_success '__gitcomp_nl - no suffix' '
 	EOF
 '
 
-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"
 '
 
-- 
1.8.0

^ permalink raw reply related

* [RFC/PATCH v2 7/8] completion: get rid of __gitcomp_1
From: Felipe Contreras @ 2012-11-18 10:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Felipe Contreras,
	Björn Gustavsson, Shawn O. Pearce, Robert Zeh,
	Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353236213-13471-1-git-send-email-felipe.contreras@gmail.com>

There's no point in calling a separate function that is only used in one
place. Specially considering that there's no need to call compgen, and
we traverse the words ourselves both in __gitcompadd, and __gitcomp_1.

So lets squash the functions together, and traverse only once.

This improves performance.

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0b72f24..82ea7b1 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+
@@ -250,8 +237,17 @@ __gitcomp ()
 	--*=)
 		;;
 	*)
-		local IFS=$'\n'
-		__gitcompadd "$(__gitcomp_1 "${1-}" "${4-}")" "${2-}" "$cur_" ""
+		local c i=0 IFS=$' \t\n'
+		for c in $1; do
+			c="$c${4-}"
+			case $c in
+			--*=*|*.) ;;
+			*) c="$c " ;;
+			esac
+			if [[ $c == "$cur_"* ]]; then
+				COMPREPLY[i++]="${2-}$c"
+			fi
+		done
 		;;
 	esac
 }
-- 
1.8.0

^ permalink raw reply related

* [RFC/PATCH v2 8/8] completion: small optimization
From: Felipe Contreras @ 2012-11-18 10:56 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor, Felipe Contreras,
	Björn Gustavsson, Shawn O. Pearce, Robert Zeh,
	Peter van der Does, Jonathan Nieder, Jeff King
In-Reply-To: <1353236213-13471-1-git-send-email-felipe.contreras@gmail.com>

No need to calculate a new $c with a space if we are not going to do
anything it with it.

There should be no functional changes, except that a word "foo " with no
suffixes can't be matched. But $cur cannot have a space at the end
anyway. So it's safe.

Based on the code from SZEDER Gábor.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 82ea7b1..e5f862c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -240,11 +240,11 @@ __gitcomp ()
 		local c i=0 IFS=$' \t\n'
 		for c in $1; do
 			c="$c${4-}"
-			case $c in
-			--*=*|*.) ;;
-			*) c="$c " ;;
-			esac
 			if [[ $c == "$cur_"* ]]; then
+				case $c in
+				--*=*|*.) ;;
+				*) c="$c " ;;
+				esac
 				COMPREPLY[i++]="${2-}$c"
 			fi
 		done
-- 
1.8.0

^ permalink raw reply related

* Re: using multiple version of git simultaneously
From: arif @ 2012-11-18 11:04 UTC (permalink / raw)
  To: Jeff King; +Cc: Tomas Carnecky, git
In-Reply-To: <20121117161631.GA18844@sigill.intra.peff.net>

On 11/17/2012 10:16 PM, Jeff King wrote:
> On Sat, Nov 17, 2012 at 02:50:31PM +0000, Tomas Carnecky wrote:
> 
>> On Sat, 17 Nov 2012 20:25:21 +0600, arif <aftnix@gmail.com> wrote:
<snip>
>
>> Install each version into its own prefix (~/git/1.8.0/, ~/git/1.7.0/ etc).
> 
> Once you have done that, you can also symlink the binary from each into
> your regular PATH (e.g., ln -s ~/git/1.8.0/bin/git ~/bin/git.v1.8) to
> make it easy to switch between them. The installed exec-path is baked in
> at compile-time, so it finds the correct git sub-programs properly.
> 
> I keep a couple dozen built versions of git around like this for quick
> regression testing of bugs we see on the list.
> 
> -Peff
> 

So what you are saying that, making a symlink for "git" is sufficient. I
don't need to make symlinks for ever git subbinaries.

Is that correct?


-- 
Cheers
arif

^ permalink raw reply

* [PATCH v6 0/2] New zsh wrapper
From: Felipe Contreras @ 2012-11-18 11:08 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Marc Khouzam, Felipe Contreras,
	Marius Storm-Olsen, Marius Storm-Olsen, Jonathan Nieder,
	Peter van der Does, SZEDER Gábor, Mark Lodato

Hi,

It looks like there's some resistance with the other patches this series was
depending upon, so lets get rid of them, they are not really needed.

This version should be ready to merge.

The second patch is new, in order for users to get the same features when
sourcing the bash script (they don't need to change anything). They'll get a
warning suggesting to check the new script git-completion.zsh. Eventually, that
support would be dropped from the bash script.

Some people were suggesting something like this, so here it is.

Can we merge the zsh wrapper now?

Felipe Contreras (2):
  completion: add new zsh completion
  completion: start moving to the new zsh completion

 contrib/completion/git-completion.bash | 104 +++++++++++++++++++--------------
 contrib/completion/git-completion.zsh  |  78 +++++++++++++++++++++++++
 2 files changed, 139 insertions(+), 43 deletions(-)
 create mode 100644 contrib/completion/git-completion.zsh

-- 
1.8.0

^ permalink raw reply

* [PATCH v6 1/2] completion: add new zsh completion
From: Felipe Contreras @ 2012-11-18 11:08 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Marc Khouzam, Felipe Contreras,
	Marius Storm-Olsen, Marius Storm-Olsen, Jonathan Nieder,
	Peter van der Does, SZEDER Gábor, Mark Lodato
In-Reply-To: <1353236889-15052-1-git-send-email-felipe.contreras@gmail.com>

It seems there's always issues with zsh's bash completion emulation.
I've tried to fix as many as I could[1], and most of the fixes are already
in the latest version of zsh, but still, there are issues.

There is no point going through all that pain; the emulation is easy to
achieve, and this patch works better than zsh's bash completion
emulation.

[1] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=commitdiff;h=23907bb840c80eef99eabba17e086e44c9b2d3fc

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.zsh | 78 +++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 contrib/completion/git-completion.zsh

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
new file mode 100644
index 0000000..4577502
--- /dev/null
+++ b/contrib/completion/git-completion.zsh
@@ -0,0 +1,78 @@
+#compdef git gitk
+
+# zsh completion wrapper for git
+#
+# You need git's bash completion script installed somewhere, by default on the
+# same directory as this script.
+#
+# If your script is on ~/.git-completion.sh instead, you can configure it on
+# your ~/.zshrc:
+#
+#  zstyle ':completion:*:*:git:*' script ~/.git-completion.sh
+#
+# The recommended way to install this script is to copy to
+# '~/.zsh/completion/_git', and then add the following to your ~/.zshrc file:
+#
+#  fpath=(~/.zsh/completion $fpath)
+
+complete ()
+{
+	# do nothing
+	return 0
+}
+
+zstyle -s ":completion:*:*:git:*" script script
+test -z "$script" && script="$(dirname ${funcsourcetrace[1]%:*})"/git-completion.bash
+ZSH_VERSION='' . "$script"
+
+__gitcomp ()
+{
+	emulate -L zsh
+
+	local cur_="${3-$cur}"
+
+	case "$cur_" in
+	--*=)
+		;;
+	*)
+		local c IFS=$' \t\n'
+		local -a array
+		for c in ${=1}; do
+			c="$c${4-}"
+			case $c in
+			--*=*|*.) ;;
+			*) c="$c " ;;
+			esac
+			array+=("$c")
+		done
+		compset -P '*[=:]'
+		compadd -Q -S '' -p "${2-}" -a -- array && _ret=0
+		;;
+	esac
+}
+
+__gitcomp_nl ()
+{
+	emulate -L zsh
+
+	local IFS=$'\n'
+	compset -P '*[=:]'
+	compadd -Q -S "${4- }" -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
+}
+
+_git
-- 
1.8.0

^ permalink raw reply related

* [PATCH v6 2/2] completion: start moving to the new zsh completion
From: Felipe Contreras @ 2012-11-18 11:08 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Marc Khouzam, Felipe Contreras,
	Marius Storm-Olsen, Marius Storm-Olsen, Jonathan Nieder,
	Peter van der Does, SZEDER Gábor, Mark Lodato
In-Reply-To: <1353236889-15052-1-git-send-email-felipe.contreras@gmail.com>

Zsh's bash completion emulation is buggy, not properly maintained, and
we have some workarounds in place for different bugs that appeared in
various versions.

Since I'm the only one that has worked on that code lately[1], it might make
snese to use the code I wrote specifically for git.

The advantages are:

 1) Less workarounds

   * No need to hack __get_comp_words_by_ref
   * No need to hack IFS or words

 2) Improved features

   * 'git show master' now properly adds a space at the end (IFS bug)
   * 'git checkout --conflict=' now properly returns the sub-items
     (missing feature)

 3) Consolidated code

   * It's all now in a single chunk, and it's basically the same as
     git-completion.zsh

Since there's some interest in moving the zsh-specific code out of this
script, lets go ahead and warn the users that they should be using
git-completion.zsh.

[1] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=history;f=Completion/bashcompinit

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 104 +++++++++++++++++++--------------
 1 file changed, 61 insertions(+), 43 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bc0657a..9cd58ca 100644
--- 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:"
@@ -169,7 +165,6 @@ __git_reassemble_comp_words_by_ref()
 }
 
 if ! type _get_comp_words_by_ref >/dev/null 2>&1; then
-if [[ -z ${ZSH_VERSION:+set} ]]; then
 _get_comp_words_by_ref ()
 {
 	local exclude cur_ words_ cword_
@@ -197,32 +192,6 @@ _get_comp_words_by_ref ()
 		shift
 	done
 }
-else
-_get_comp_words_by_ref ()
-{
-	while [ $# -gt 0 ]; do
-		case "$1" in
-		cur)
-			cur=${COMP_WORDS[COMP_CWORD]}
-			;;
-		prev)
-			prev=${COMP_WORDS[COMP_CWORD-1]}
-			;;
-		words)
-			words=("${COMP_WORDS[@]}")
-			;;
-		cword)
-			cword=$COMP_CWORD
-			;;
-		-n)
-			# assume COMP_WORDBREAKS is already set sanely
-			shift
-			;;
-		esac
-		shift
-	done
-}
-fi
 fi
 
 # Generates completion reply with compgen, appending a space to possible
@@ -2430,20 +2399,69 @@ __gitk_main ()
 	__git_complete_revlist
 }
 
-__git_func_wrap ()
-{
-	if [[ -n ${ZSH_VERSION-} ]]; then
-		emulate -L bash
-		setopt KSH_TYPESET
+if [[ -n ${ZSH_VERSION-} ]]; then
+	echo "WARNING: this script is deprecated, please see git-completion.zsh" 1>&2
 
-		# workaround zsh's bug that leaves 'words' as a special
-		# variable in versions < 4.3.12
-		typeset -h words
+	__gitcomp ()
+	{
+		emulate -L zsh
 
-		# workaround zsh's bug that quotes spaces in the COMPREPLY
-		# array if IFS doesn't contain spaces.
-		typeset -h IFS
-	fi
+		local cur_="${3-$cur}"
+
+		case "$cur_" in
+		--*=)
+			;;
+		*)
+			local c IFS=$' \t\n'
+			local -a array
+			for c in ${=1}; do
+				c="$c${4-}"
+				case $c in
+				--*=*|*.) ;;
+				*) c="$c " ;;
+				esac
+				array+=("$c")
+			done
+			compset -P '*[=:]'
+			compadd -Q -S '' -p "${2-}" -a -- array && _ret=0
+			;;
+		esac
+	}
+
+	__gitcomp_nl ()
+	{
+		emulate -L zsh
+
+		local IFS=$'\n'
+		compset -P '*[=:]'
+		compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
+	}
+
+	__git_zsh_helper ()
+	{
+		emulate -L ksh
+		local cur cword prev
+		cur=${words[CURRENT-1]}
+		prev=${words[CURRENT-2]}
+		let cword=CURRENT-1
+		__${service}_main
+	}
+
+	_git ()
+	{
+		emulate -L zsh
+		local _ret=1
+		__git_zsh_helper
+		let _ret && _default -S '' && _ret=0
+		return _ret
+	}
+
+	compdef _git git gitk
+	return
+fi
+
+__git_func_wrap ()
+{
 	local cur words cword prev
 	_get_comp_words_by_ref -n =: cur words cword prev
 	$1
-- 
1.8.0

^ permalink raw reply related

* [ANNOUNCE] namespace support in gitolite
From: Sitaram Chamarty @ 2012-11-18 11:41 UTC (permalink / raw)
  To: Git Mailing List

Hello all,

Gitolite now supports namespaces on the server side (invisible
to the client side of course).  See [1] for details.

A simple way to describe it would be that all the repos in the
top row of the picture in the "integration manager workflow" at
[2] are stored in one physical repo on the server, which saves a
lot of disk space as well as network traffic for pushes to a
"new" repo.

This is all on the server side.  On the client side they
continue to look like separate repos, so nothing changes.

In addition, gitolite's 'upstream' trigger can be used to keep
the main repo in sync with the real upstream (somewhere on the
internet) if you wish.

A bit more detail and a small example is at [3].

[1]: http://sitaramc.github.com/gitolite/namespaces.html
[2]: http://git-scm.com/book/en/Distributed-Git-Distributed-Workflows#Integration-Manager-Workflow
[3]: http://groups.google.com/group/gitolite/browse_thread/thread/5efcbfb5ff3ebe88

-- 
Sitaram

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox