Re: [PATCH] git clean: Don't automatically remove directories when run within subdirectory

[Junio C Hamano <gitster@pobox.com>]

Shawn Bohrer <shawn.bohrer@gmail.com> writes:

> diff --git a/builtin-clean.c b/builtin-clean.c
> index fefec30..5c5ec98 100644
> --- a/builtin-clean.c
> +++ b/builtin-clean.c
> @@ -95,7 +95,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>  
>  	for (i = 0; i < dir.nr; i++) {
>  		struct dir_entry *ent = dir.entries[i];
> -		int len, pos, matches;
> +		int len, pos;
> +		int matches = 0;
>  		struct cache_entry *ce;
>  		struct stat st;

Initialization of "matches" seems to be an independent clean-up.  Although
it forces the initialization in the codepath that do not need the value of
matches, that is not a big deal --- right?

> @@ -127,18 +128,16 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>  
>  		if (pathspec) {
>  			memset(seen, 0, argc > 0 ? argc : 1);
> -			matches = match_pathspec(pathspec, ent->name, ent->len,
> +			matches = match_pathspec(pathspec, ent->name, len,
>  						 baselen, seen);
> -		} else {
> -			matches = 0;
>  		}

And the essential change (fix) is to send len which could be shorter than
ent->len because we have stripped '/' here, plus the one in match_one()
that now allows name[] that is not NUL terminated.

>  		if (S_ISDIR(st.st_mode)) {
>  			strbuf_addstr(&directory, ent->name);
>  			qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
> -			if (show_only && (remove_directories || matches)) {
> +			if (show_only && (remove_directories || (matches >= 2))) {
>  				printf("Would remove %s\n", qname);
> -			} else if (remove_directories || matches) {
> +			} else if (remove_directories || (matches >= 2)) {

These magic numbers are bad.  Please update it to use symbolic constants.

>  				if (!quiet)
>  					printf("Removing %s\n", qname);
>  				if (remove_dir_recursively(&directory, 0) != 0) {
> diff --git a/dir.c b/dir.c
> index b5bfbca..63715c9 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -80,7 +80,7 @@ static int match_one(const char *match, const char *name, int namelen)
>  	if (strncmp(match, name, matchlen))
>  		return !fnmatch(match, name, 0) ? MATCHED_FNMATCH : 0;
>  
> -	if (!name[matchlen])
> +	if (namelen == matchlen)
>  		return MATCHED_EXACTLY;
>  	if (match[matchlen-1] == '/' || name[matchlen] == '/')
>  		return MATCHED_RECURSIVELY;