Re: [PATCH] add: don't complain when adding empty project root

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Dec 26, 2013, 17:25 UTC
Message-ID: <20131226172542.GS20443@google.com>
In-Reply-To: <1387789361-29036-1-git-send-email-pclouds@gmail.com>

Hi,

Nguyễn Thái Ngọc Duy wrote:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Thanks.

[...]

Show 11 quoted lines

> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -544,7 +544,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  
>  		for (i = 0; i < pathspec.nr; i++) {
>  			const char *path = pathspec.items[i].match;
> -			if (!seen[i] &&
> +			if (!seen[i] && pathspec.items[i].match[0] &&
>  			    ((pathspec.items[i].magic &
>  			      (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
>  			     !file_exists(path))) {

Nit: in this loop there's already the synonym 'path' for item.match,
so perhaps

			if (!seen[i] && path[0] && ...)

would be clearer.

Should "git add --refresh ." get the same treatment?

Show 9 quoted lines

> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -307,4 +307,8 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out
>  	test_i18ncmp expect.err actual.err
>  '
>  
> +test_expect_success 'git add -A on empty repo does not error out' '
> +	git init empty && ( cd empty && git add -A . )
> +'

Adding a test at the end like this means the tests come in chronological order instead of logical order and simultaneous patches to the same test script become more likely to conflict.

How about something like the following, for squashing in?

With or without the tweaks below,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

diff --git i/builtin/add.c w/builtin/add.c
index fbd3f3a..d7e3e44 100644
--- i/builtin/add.c
+++ w/builtin/add.c
@@ -544,7 +544,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 		for (i = 0; i < pathspec.nr; i++) {
 			const char *path = pathspec.items[i].match;
-			if (!seen[i] && pathspec.items[i].match[0] &&
+			if (!seen[i] && path[0] &&
 			    ((pathspec.items[i].magic &
 			      (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
 			     !file_exists(path))) {
diff --git i/t/t3700-add.sh w/t/t3700-add.sh
index 1535d8f..fe274e2 100755
--- i/t/t3700-add.sh
+++ w/t/t3700-add.sh
@@ -272,6 +272,25 @@ test_expect_success '"add non-existent" should fail' '
 	! (git ls-files | grep "non-existent")
 '
 
+test_expect_success 'git add -A on empty repo does not error out' '
+	rm -fr empty &&
+	git init empty &&
+	(
+		cd empty &&
+		git add -A . &&
+		git add -A
+	)
+'
+
+test_expect_success '"git add ." in empty repo' '
+	rm -fr empty &&
+	git init empty &&
+	(
+		cd empty &&
+		git add .
+	)
+'
+
 test_expect_success 'git add --dry-run of existing changed file' "
 	echo new >>track-this &&
 	git add --dry-run track-this >actual 2>&1 &&
@@ -307,8 +326,4 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out
 	test_i18ncmp expect.err actual.err
 '
 
-test_expect_success 'git add -A on empty repo does not error out' '
-	git init empty && ( cd empty && git add -A . )
-'
-
 test_done