Re: [PATCH 1/5] port --ignore-unmatch to "git add"

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 13, 2009, 19:36 UTC
Message-ID: <7vy6pna4lu.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1250133624-2272-1-git-send-email-luke-jr+git@utopios.org>

Luke Dashjr <luke-jr+git@utopios.org> writes:

Show 5 quoted lines

> "git rm" has a --ignore-unmatch option that is also applicable to "git add"
> and may be useful for persons wanting to ignore unmatched arguments, but not
> all errors.
>
> Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>

Chould you refresh my memory a bit?

In what circumstance is "rm --ignore-unmatch" useful to begin with? A similar question for "add --ignore-unmatch".

Now the obligatory design level question is behind us, let's take a brief look at the codde.

> +static int ignore_unmatch = 0;

Drop " = 0" and let the language initialize this to zero.

Show 20 quoted lines

>  static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
>  {
> @@ -63,7 +64,7 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
>  	fill_pathspec_matches(pathspec, seen, specs);
>  
>  	for (i = 0; i < specs; i++) {
> -		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
> +		if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]) && !ignore_unmatch)
>  			die("pathspec '%s' did not match any files",
>  					pathspec[i]);
>  	}
> @@ -108,7 +109,7 @@ static void refresh(int verbose, const char **pathspec)
>  	refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
>  		      pathspec, seen);
>  	for (i = 0; i < specs; i++) {
> -		if (!seen[i])
> +		if (!seen[i] && !ignore_unmatch)
>  			die("pathspec '%s' did not match any files", pathspec[i]);
>  	}
>          free(seen);

What's the point of these two loops if under ignore_unmatch everything becomes no-op?

That is, wouldn't it be much more clear if you wrote like this?

 builtin-add.c |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 581a2a1..49576b4 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -41,16 +41,25 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 	}
 }
 
+static char *alloc_seen(const char **pathspec, int *specs_)
+{
+	int specs;
+
+	if (ignore_unmatch)
+		return NULL;
+	for (specs = 0; pathspec[specs];  specs++)
+		; /* nothing */
+	*specs_ = specs;
+	return xcalloc(specs, 1);
+}
+
 static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
 {
 	char *seen;
 	int i, specs;
 	struct dir_entry **src, **dst;
 
-	for (specs = 0; pathspec[specs];  specs++)
-		/* nothing */;
-	seen = xcalloc(specs, 1);
-
+	seen = alloc_seen(pathspec, &specs);
 	src = dst = dir->entries;
 	i = dir->nr;
 	while (--i >= 0) {
@@ -60,6 +69,8 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
 			*dst++ = entry;
 	}
 	dir->nr = dst - dir->entries;
+	if (!seen)
+		return;
 	fill_pathspec_matches(pathspec, seen, specs);
 
 	for (i = 0; i < specs; i++) {
@@ -102,11 +113,11 @@ static void refresh(int verbose, const char **pathspec)
 	char *seen;
 	int i, specs;
 
-	for (specs = 0; pathspec[specs];  specs++)
-		/* nothing */;
-	seen = xcalloc(specs, 1);
+	seen = alloc_seen(pathspec, &specs);
 	refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
 		      pathspec, seen);
+	if (!seen)
+		return;
 	for (i = 0; i < specs; i++) {
 		if (!seen[i])
 			die("pathspec '%s' did not match any files", pathspec[i]);