Re: [PATCH v4] git-clean: Display more accurate delete messages

> Consider the output of the improved version:
>
>   $ git clean -fd
>   Removing tracked_dir/some_untracked_file
>   Removing untracked_file
>   warning: ignoring untracked git repository untracked_foo/frotz.git
>   Removing untracked_foo/bar
>   Removing untracked_foo/emptydir
>   warning: ignoring untracked git repository untracked_some.git/
>
> Now it displays only the file and directory names that got actually
> deleted and shows warnings about ignored untracked git repositories.
>
> Reported-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
>
> Signed-off-by: Zoltan Klinger <zoltan.klinger@gmail.com>
> ---

>  builtin/clean.c |  158 +++++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 129 insertions(+), 29 deletions(-)
>
> diff --git a/builtin/clean.c b/builtin/clean.c
> index 69c1cda..1714546 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -10,6 +10,7 @@
>  #include "cache.h"
>  #include "dir.h"
>  #include "parse-options.h"
> +#include "refs.h"
>  #include "string-list.h"
>  #include "quote.h"
>  
> @@ -20,6 +21,12 @@ static const char *const builtin_clean_usage[] = {
>  	NULL
>  };
>  
> +static const char *msg_remove = N_("Removing %s\n");
> +static const char *msg_would_remove = N_("Would remove %s\n");
> +static const char *msg_would_ignore_git_dir = N_("Would ignore untracked git repository %s\n");
> +static const char *msg_warn_ignore_git_dir = N_("ignoring untracked git repository %s");
> +static const char *msg_warn_remove_failed = N_("failed to remove %s");
> +
>  static int git_clean_config(const char *var, const char *value, void *cb)
>  {
>  	if (!strcmp(var, "clean.requireforce"))
> @@ -34,11 +41,116 @@ static int exclude_cb(const struct option *opt, const char *arg, int unset)
>  	return 0;
>  }
>  
> +static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
> +		int dry_run, int quiet, int *dir_gone)
> +{
> +	DIR *dir;
> +	struct strbuf quoted = STRBUF_INIT;
> +	struct dirent *e;
> +	int res = 0, ret = 0, gone = 1, original_len = path->len, len, i;
> +	unsigned char submodule_head[20];
> +	struct string_list dels = STRING_LIST_INIT_DUP;
> +
> +	*dir_gone = 1;
> +
> +	if ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
> +	    !resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) {
> +		if (dry_run && !quiet) {
> +			quote_path_relative(path->buf, strlen(path->buf), &quoted, prefix);
> +			printf(_(msg_would_ignore_git_dir), quoted.buf);
> +		} else if (!dry_run) {
> +			quote_path_relative(path->buf, strlen(path->buf), &quoted, prefix);
> +			warning(_(msg_warn_ignore_git_dir), quoted.buf);
> +		}
> +
> +		*dir_gone = 0;
> +		return 0;
> +	}
> +
> +	dir = opendir(path->buf);
> +	if (!dir) {
> +		/* an empty dir could be removed even if it is unreadble */
> +		res = dry_run ? 0 : rmdir(path->buf);
> +		if (res) {
> +			quote_path_relative(path->buf, strlen(path->buf), &quoted, prefix);
> +			warning(_(msg_warn_remove_failed), quoted.buf);
> +			*dir_gone = 0;
> +		}
> +		return res;
> +	}
> +
> +	if (path->buf[original_len - 1] != '/')
> +		strbuf_addch(path, '/');
> +
> +	len = path->len;
> +	while ((e = readdir(dir)) != NULL) {
> +		struct stat st;
> +		if (is_dot_or_dotdot(e->d_name))
> +			continue;
> +
> +		strbuf_setlen(path, len);
> +		strbuf_addstr(path, e->d_name);
> +		if (lstat(path->buf, &st))
> +			; /* fall thru */
> +		else if (S_ISDIR(st.st_mode)) {
> +			if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
> +				ret = 1;
> +			if (gone) {
> +				quote_path_relative(path->buf, strlen(path->buf), &quoted, prefix);
> +				string_list_append(&dels, quoted.buf);
> +			}
> +			else
> +				*dir_gone = 0;
> +			continue;
> +		} else {
> +			res = dry_run ? 0 : unlink(path->buf);
> +			if (!res) {
> +				quote_path_relative(path->buf, strlen(path->buf), &quoted, prefix);
> +				string_list_append(&dels, quoted.buf);
> +			}
> +			else {
> +				quote_path_relative(path->buf, strlen(path->buf), &quoted, prefix);
> +				warning(_(msg_warn_remove_failed), quoted.buf);
> +				*dir_gone = 0;
> +				ret = 1;
> +			}
> +			continue;
> +		}
> +
> +		/* path too long, stat fails, or non-directory still exists */
> +		*dir_gone = 0;
> +		ret = 1;
> +		break;
> +	}
> +	closedir(dir);
> +
> +	strbuf_setlen(path, original_len);
> +
> +	if (*dir_gone) {
> +		res = dry_run ? 0 : rmdir(path->buf);
> +		if (!res)
> +			*dir_gone = 1;
> +		else {
> +			quote_path_relative(path->buf, strlen(path->buf), &quoted, prefix);
> +			warning(_(msg_warn_remove_failed), quoted.buf);
> +			*dir_gone = 0;
> +			ret = 1;
> +		}
> +	}
> +
> +	if (!*dir_gone && !quiet) {
> +		for (i = 0; i < dels.nr; i++)
> +			printf(dry_run ?  _(msg_would_remove) : _(msg_remove), dels.items[i].string);
> +	}
> +	string_list_clear(&dels, 0);
> +	return ret;
> +}
> +
>  int cmd_clean(int argc, const char **argv, const char *prefix)
>  {
> -	int i;
> -	int show_only = 0, remove_directories = 0, quiet = 0, ignored = 0;
> -	int ignored_only = 0, config_set = 0, errors = 0;
> +	int i, res;
> +	int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
> +	int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
>  	int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
>  	struct strbuf directory = STRBUF_INIT;
>  	struct dir_struct dir;
> @@ -49,7 +161,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>  	char *seen = NULL;
>  	struct option options[] = {
>  		OPT__QUIET(&quiet, N_("do not print names of files removed")),
> -		OPT__DRY_RUN(&show_only, N_("dry run")),
> +		OPT__DRY_RUN(&dry_run, N_("dry run")),
>  		OPT__FORCE(&force, N_("force")),
>  		OPT_BOOLEAN('d', NULL, &remove_directories,
>  				N_("remove whole directories")),
> @@ -77,7 +189,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>  	if (ignored && ignored_only)
>  		die(_("-x and -X cannot be used together"));
>  
> -	if (!show_only && !force) {
> +	if (!dry_run && !force) {
>  		if (config_set)
>  			die(_("clean.requireForce set to true and neither -n nor -f given; "
>  				  "refusing to clean"));
> @@ -149,38 +261,26 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>  
>  		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 == MATCHED_EXACTLY))) {
> -				printf(_("Would remove %s\n"), qname);
> -			} else if (remove_directories ||
> -				   (matches == MATCHED_EXACTLY)) {
> -				if (!quiet)
> -					printf(_("Removing %s\n"), qname);
> -				if (remove_dir_recursively(&directory,
> -							   rm_flags) != 0) {
> -					warning(_("failed to remove %s"), qname);
> +			if (remove_directories || (matches == MATCHED_EXACTLY)) {
> +				if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))
>  					errors++;
> +				if (gone && !quiet) {
> +					qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
> +					printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
>  				}
> -			} else if (show_only) {
> -				printf(_("Would not remove %s\n"), qname);
> -			} else {
> -				printf(_("Not removing %s\n"), qname);
>  			}
>  			strbuf_reset(&directory);
>  		} else {
>  			if (pathspec && !matches)
>  				continue;
> -			qname = quote_path_relative(ent->name, -1, &buf, prefix);
> -			if (show_only) {
> -				printf(_("Would remove %s\n"), qname);
> -				continue;
> -			} else if (!quiet) {
> -				printf(_("Removing %s\n"), qname);
> -			}
> -			if (unlink(ent->name) != 0) {
> -				warning(_("failed to remove %s"), qname);
> +			res = dry_run ? 0 : unlink(ent->name);
> +			if (res) {
> +				qname = quote_path_relative(ent->name, -1, &buf, prefix);
> +				warning(_(msg_warn_remove_failed), qname);
>  				errors++;
> +			} else if (!quiet) {
> +				qname = quote_path_relative(ent->name, -1, &buf, prefix);
> +				printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
>  			}
>  		}
>  	}