[PATCH v5 0/3] interactive git clean

[PATCH v5 0/3] interactive git clean

[Jiang Xin]

The interactive git clean combines `git clean -n` and `git clean -f`
together to do safe cleaning, and has more features.

First it displays what would be removed in columns (so that you can
see them all in one screen). The user must confirm before actually
cleaning.

    WARNING: The following items will be removed permanently. Press "y"
    WARNING: to start cleaning, and press "n" to abort the cleaning.
    WARNING: You can also enter the "edit" mode, and select items
    WARNING: to be excluded from the cleaning.

      What would be removed...    What would be removed...
      What would be removed...    What would be removed...
    
    Remove (yes/no/Edit) ? 

In this confirmation dialog, the user has three choices:

 * Yes: Start to do cleaning.
 * No:  Nothing will be deleted.
 * Edit (default for the first time): Enter the edit mode.

When the user chooses the edit mode, it would look like this:


    NOTE: Will remove the following items. You can input space-seperated
    NOTE: patterns (just like .gitignore) to exclude items from deletion,
    NOTE: or press ENTER to continue.
    
      What would be removed...    What would be removed...
      What would be removed...    What would be removed...
    
    Input ignore patterns> 


The user can input space-separated patterns (the same syntax as gitignore),
and each clean candidate that matches with one of the patterns will be
excluded from cleaning.

When the user feels it's OK, presses ENTER and back to the confirmation dialog.

    WARNING: The following items will be removed permanently. Press "y"
    WARNING: to start cleaning, and press "n" to abort the cleaning.
    WARNING: You can also enter the "edit" mode, and select items
    WARNING: to be excluded from the cleaning.

      What would be removed...
    
    Remove (Yes/no/edit) ? 

This time the default choice of the confirmation dialog is "YES".
So when user press ENTER, start cleaning.

Jiang Xin (3):
  Add support for -i/--interactive to git-clean
  Show items of interactive git-clean in columns
  Add colors to interactive git-clean

 Documentation/git-clean.txt |  15 ++-
 builtin/clean.c             | 295 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 291 insertions(+), 19 deletions(-)

-- 
1.8.3.rc0.364.gc6aefbf

[PATCH v5 1/3] Add support for -i/--interactive to git-clean

[Jiang Xin]

Show what would be done and the user must confirm before actually
cleaning. In the confirmation dialog, the user has three choices:

 * Yes: Start to do cleaning.
 * No:  Nothing will be deleted.
 * Edit (default for the first time): Enter the edit mode.

When the user chooses the edit mode, the user can input space-
separated patterns (the same syntax as gitignore), and each clean
candidate that matches with one of the patterns will be excluded
from cleaning. When the user feels it's OK, presses ENTER and back
to the confirmation dialog.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Spelling-check-by: Eric Sunshine <sunshine@sunshineco.com>
Comments-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-clean.txt |  15 +++-
 builtin/clean.c             | 183 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 181 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index bdc3a..f5572 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree
 SYNOPSIS
 --------
 [verse]
-'git clean' [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X] [--] <path>...
+'git clean' [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <path>...
 
 DESCRIPTION
 -----------
@@ -34,7 +34,18 @@ OPTIONS
 -f::
 --force::
 	If the Git configuration variable clean.requireForce is not set
-	to false, 'git clean' will refuse to run unless given -f or -n.
+	to false, 'git clean' will refuse to run unless given -f, -n or
+	-i.
+
+-i::
+--interactive::
+	Show what would be done and the user must confirm before actually
+	cleaning. In the confirmation dialog, the user can choose to abort
+	the cleaning, or enter into an edit mode. In the edit mode, the
+	user can input space-separated patterns (the same syntax as
+	gitignore), and each clean candidate that matches with one of the
+	patterns will be excluded from cleaning. When the user feels it's
+	OK, presses ENTER and back to the confirmation dialog.
 
 -n::
 --dry-run::
diff --git a/builtin/clean.c b/builtin/clean.c
index 04e39..12489 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -15,9 +15,10 @@
 #include "quote.h"
 
 static int force = -1; /* unset */
+static int interactive;
 
 static const char *const builtin_clean_usage[] = {
-	N_("git clean [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>..."),
+	N_("git clean [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>..."),
 	NULL
 };
 
@@ -142,6 +143,138 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 	return ret;
 }
 
+void interactive_clean_edit(struct string_list *dels, const char *prefix)
+{
+	struct dir_struct dir;
+	struct strbuf confirm = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf **ignore_list;
+	struct string_list_item *item;
+	struct exclude_list *el;
+	const char *qname;
+	int changed = -1, i;
+
+	putchar('\n');
+	printf_ln(_(
+		    "NOTE: Will remove the following items. You can input space-seperated\n"
+		    "NOTE: patterns (just like .gitignore) to exclude items from deletion,\n"
+		    "NOTE: or press ENTER to continue."
+		   ));
+
+	while (1) {
+		/* dels list may become empty when we run string_list_remove_empty_items later */
+		if (!dels->nr) {
+			printf_ln(_("No more files to clean, exiting."));
+			break;
+		}
+
+		if (changed) {
+			putchar('\n');
+
+			/* Display dels in "Would remove ..." format */
+			for_each_string_list_item(item, dels) {
+				qname = quote_path_relative(item->string, -1, &buf, prefix);
+				printf(_(msg_would_remove), qname);
+			}
+			putchar('\n');
+		}
+
+		printf(_("Input ignore patterns> "));
+		strbuf_getline(&confirm, stdin, '\n');
+		strbuf_trim(&confirm);
+
+		/* Quit edit mode */
+		if (!confirm.len)
+			break;
+
+		memset(&dir, 0, sizeof(dir));
+		el = add_exclude_list(&dir, EXC_CMDL, "manual exclude");
+		ignore_list = strbuf_split_buf(confirm.buf, confirm.len, ' ', 0);
+
+		for (i = 0; ignore_list[i]; i++) {
+			strbuf_trim(*ignore_list);
+			if (!(*ignore_list)->len)
+				continue;
+
+			add_exclude(ignore_list[i]->buf, "", 0, el, -(i+1));
+		}
+
+		changed = 0;
+		for_each_string_list_item(item, dels) {
+			int dtype = DT_UNKNOWN;
+			const char *qname;
+
+			qname = quote_path_relative(item->string, -1, &buf, prefix);
+
+			if (is_excluded(&dir, qname, &dtype)) {
+				*item->string = '\0';
+				changed++;
+			}
+		}
+
+		if (changed) {
+			string_list_remove_empty_items(dels, 0);
+		} else {
+			printf_ln(_("WARNING: Cannot find items matched by: %s"), confirm.buf);
+		}
+
+		strbuf_list_free(ignore_list);
+		clear_directory(&dir);
+	}
+
+	strbuf_release(&buf);
+	strbuf_release(&confirm);
+}
+
+void interactive_clean(struct string_list *dels, const char *prefix)
+{
+	struct strbuf confirm = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	struct string_list_item *item;
+	const char *qname;
+	int count = 0;
+
+	/* dels list may become empty after return back from edit mode */
+	while (dels->nr) {
+		/* Display dels in "Would remove ..." format */
+		putchar('\n');
+		for_each_string_list_item(item, dels) {
+			qname = quote_path_relative(item->string, -1, &buf, prefix);
+			printf(_(msg_would_remove), qname);
+		}
+		putchar('\n');
+
+		/* Confirmation dialog */
+		printf(count > 0 ? _("Remove (Yes/no/edit) ? ") : _("Remove (yes/no/Edit) ? "));
+		strbuf_getline(&confirm, stdin, '\n');
+		strbuf_trim(&confirm);
+
+		if (confirm.len) {
+			if (!strncasecmp(confirm.buf, "yes", confirm.len)) {
+				break;
+			} else if (!strncasecmp(confirm.buf, "no", confirm.len) ||
+				   !strncasecmp(confirm.buf, "quit", confirm.len)) {
+				string_list_clear(dels, 0);
+				break;
+			} else if (!strncasecmp(confirm.buf, "edit", confirm.len)) {
+				interactive_clean_edit(dels, prefix);
+			} else {
+				continue;
+			}
+		} else if (count > 0) {
+			/* If back from edit_mode, confirmation dialog defaults to "yes" */
+			break;
+		} else {
+			/* For the first time, confirmation dialog defaults to "edit" */
+			interactive_clean_edit(dels, prefix);
+		}
+		count++;
+	}
+
+	strbuf_release(&buf);
+	strbuf_release(&confirm);
+}
+
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
 	int i, res;
@@ -154,12 +287,15 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	struct strbuf buf = STRBUF_INIT;
 	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 	struct exclude_list *el;
+	struct string_list dels = STRING_LIST_INIT_DUP;
+	struct string_list_item *item;
 	const char *qname;
 	char *seen = NULL;
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("do not print names of files removed")),
 		OPT__DRY_RUN(&dry_run, N_("dry run")),
 		OPT__FORCE(&force, N_("force")),
+		OPT_BOOL('i', "interactive", &interactive, N_("interactive cleaning")),
 		OPT_BOOLEAN('d', NULL, &remove_directories,
 				N_("remove whole directories")),
 		{ OPTION_CALLBACK, 'e', "exclude", &exclude_list, N_("pattern"),
@@ -186,12 +322,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	if (ignored && ignored_only)
 		die(_("-x and -X cannot be used together"));
 
-	if (!dry_run && !force) {
+	if (!dry_run && !force && !interactive) {
 		if (config_set)
-			die(_("clean.requireForce set to true and neither -n nor -f given; "
+			die(_("clean.requireForce set to true and neither -i, -n nor -f given; "
 				  "refusing to clean"));
 		else
-			die(_("clean.requireForce defaults to true and neither -n nor -f given; "
+			die(_("clean.requireForce defaults to true and neither -i, -n nor -f given; "
 				  "refusing to clean"));
 	}
 
@@ -257,26 +393,42 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		}
 
 		if (S_ISDIR(st.st_mode)) {
-			strbuf_addstr(&directory, ent->name);
 			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);
-				}
+				string_list_append(&dels, ent->name);
 			}
-			strbuf_reset(&directory);
 		} else {
 			if (pathspec && !matches)
 				continue;
-			res = dry_run ? 0 : unlink(ent->name);
+			string_list_append(&dels, ent->name);
+		}
+	}
+
+	if (interactive && dels.nr > 0 && !dry_run && isatty(0) && isatty(1))
+		interactive_clean(&dels, prefix);
+
+	for_each_string_list_item(item, &dels) {
+		struct stat st;
+
+		if (lstat(item->string, &st))
+			continue;
+
+		if (S_ISDIR(st.st_mode)) {
+			strbuf_addstr(&directory, item->string);
+			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);
+			}
+			strbuf_reset(&directory);
+		} else {
+			res = dry_run ? 0 : unlink(item->string);
 			if (res) {
-				qname = quote_path_relative(ent->name, -1, &buf, prefix);
+				qname = quote_path_relative(item->string, -1, &buf, prefix);
 				warning(_(msg_warn_remove_failed), qname);
 				errors++;
 			} else if (!quiet) {
-				qname = quote_path_relative(ent->name, -1, &buf, prefix);
+				qname = quote_path_relative(item->string, -1, &buf, prefix);
 				printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
 			}
 		}
@@ -285,5 +437,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	strbuf_release(&directory);
 	string_list_clear(&exclude_list, 0);
+	string_list_clear(&dels, 0);
 	return (errors != 0);
 }
-- 
1.8.3.rc0.364.gc6aefbf

[PATCH v5 2/3] Show items of interactive git-clean in columns

[Jiang Xin]

When there are lots of items to be cleaned, it is hard to see them all
in one screen. Show them in columns instead of in one column will solve
this problem.

Since no longer show items to be cleaned using the "Would remove ..."
format (only plain filenames) in interactive mode, we add instructions
and warnings as header before them.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Comments-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/clean.c | 64 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 17 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 12489..6ee7 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -13,9 +13,11 @@
 #include "refs.h"
 #include "string-list.h"
 #include "quote.h"
+#include "column.h"
 
 static int force = -1; /* unset */
 static int interactive;
+static unsigned int colopts;
 
 static const char *const builtin_clean_usage[] = {
 	N_("git clean [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>..."),
@@ -30,8 +32,14 @@ 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"))
+	/* honors the column.ui config variable only */
+	if (!prefixcmp(var, "column."))
+		return git_column_config(var, value, NULL, &colopts);
+
+	if (!strcmp(var, "clean.requireforce")) {
 		force = !git_config_bool(var, value);
+		return 0;
+	}
 	return git_default_config(var, value, cb);
 }
 
@@ -143,6 +151,33 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 	return ret;
 }
 
+void pretty_print_dels(struct string_list *dels, const char *prefix)
+{
+	struct string_list list = STRING_LIST_INIT_DUP;
+	struct string_list_item *item;
+	struct strbuf buf = STRBUF_INIT;
+	const char *qname;
+	struct column_options copts;
+
+	for_each_string_list_item(item, dels) {
+		qname = quote_path_relative(item->string, -1, &buf, prefix);
+		string_list_append(&list, qname);
+	}
+
+	/*
+	 * always enable column display, we only consult column.*
+	 * about layout strategy and stuff
+	 */
+	colopts = (colopts & ~COL_ENABLE_MASK) | COL_ENABLED;
+	memset(&copts, 0, sizeof(copts));
+	copts.indent = "  ";
+	copts.padding = 2;
+	print_columns(&list, colopts, &copts);
+	putchar('\n');
+	strbuf_release(&buf);
+	string_list_clear(&list, 0);
+}
+
 void interactive_clean_edit(struct string_list *dels, const char *prefix)
 {
 	struct dir_struct dir;
@@ -151,7 +186,6 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)
 	struct strbuf **ignore_list;
 	struct string_list_item *item;
 	struct exclude_list *el;
-	const char *qname;
 	int changed = -1, i;
 
 	putchar('\n');
@@ -171,12 +205,8 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)
 		if (changed) {
 			putchar('\n');
 
-			/* Display dels in "Would remove ..." format */
-			for_each_string_list_item(item, dels) {
-				qname = quote_path_relative(item->string, -1, &buf, prefix);
-				printf(_(msg_would_remove), qname);
-			}
-			putchar('\n');
+			/* Display dels in columns */
+			pretty_print_dels(dels, prefix);
 		}
 
 		printf(_("Input ignore patterns> "));
@@ -229,21 +259,22 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)
 void interactive_clean(struct string_list *dels, const char *prefix)
 {
 	struct strbuf confirm = STRBUF_INIT;
-	struct strbuf buf = STRBUF_INIT;
-	struct string_list_item *item;
-	const char *qname;
 	int count = 0;
 
 	/* dels list may become empty after return back from edit mode */
 	while (dels->nr) {
-		/* Display dels in "Would remove ..." format */
 		putchar('\n');
-		for_each_string_list_item(item, dels) {
-			qname = quote_path_relative(item->string, -1, &buf, prefix);
-			printf(_(msg_would_remove), qname);
-		}
+		printf_ln(_(
+			    "WARNING: The following items will be removed permanently. Press \"y\"\n"
+			    "WARNING: to start cleaning, and press \"n\" to abort the cleaning.\n"
+			    "WARNING: You can also enter the \"edit\" mode, and select items\n"
+			    "WARNING: to be excluded from the cleaning."
+			   ));
 		putchar('\n');
 
+		/* Display dels in columns */
+		pretty_print_dels(dels, prefix);
+
 		/* Confirmation dialog */
 		printf(count > 0 ? _("Remove (Yes/no/edit) ? ") : _("Remove (yes/no/Edit) ? "));
 		strbuf_getline(&confirm, stdin, '\n');
@@ -271,7 +302,6 @@ void interactive_clean(struct string_list *dels, const char *prefix)
 		count++;
 	}
 
-	strbuf_release(&buf);
 	strbuf_release(&confirm);
 }
 
-- 
1.8.3.rc0.364.gc6aefbf

[PATCH v5 3/3] Add colors to interactive git-clean

[Jiang Xin]

Show header, help, error messages, and prompt in colors for interactive
git-clean. Re-use config variables for other git commands, such as
git-add--interactive and git-stash:

 * color.interactive: When set to always, always use colors for
   interactive prompts and displays. When false (or never),
   never. When set to true or auto, use colors only when the
   output is to the terminal.

 * color.interactive.<slot>: Use customized color for interactive
   git-clean output (like git add --interactive). <slot> may be
   prompt, header, help or error.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Comments-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/clean.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 6ee7..1692b 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -14,6 +14,7 @@
 #include "string-list.h"
 #include "quote.h"
 #include "column.h"
+#include "color.h"
 
 static int force = -1; /* unset */
 static int interactive;
@@ -30,17 +31,82 @@ static const char *msg_skip_git_dir = N_("Skipping repository %s\n");
 static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
 static const char *msg_warn_remove_failed = N_("failed to remove %s");
 
+static int clean_use_color = -1;
+static char clean_colors[][COLOR_MAXLEN] = {
+	GIT_COLOR_RESET,
+	GIT_COLOR_NORMAL,	/* PLAIN */
+	GIT_COLOR_BOLD_BLUE,	/* PROMPT */
+	GIT_COLOR_BOLD,		/* HEADER */
+	GIT_COLOR_BOLD_RED,	/* HELP */
+	GIT_COLOR_BOLD_RED,	/* ERROR */
+};
+enum color_clean {
+	CLEAN_COLOR_RESET = 0,
+	CLEAN_COLOR_PLAIN = 1,
+	CLEAN_COLOR_PROMPT = 2,
+	CLEAN_COLOR_HEADER = 3,
+	CLEAN_COLOR_HELP = 4,
+	CLEAN_COLOR_ERROR = 5,
+};
+
+static int parse_clean_color_slot(const char *var, int ofs)
+{
+	if (!strcasecmp(var+ofs, "reset"))
+		return CLEAN_COLOR_RESET;
+	if (!strcasecmp(var+ofs, "plain"))
+		return CLEAN_COLOR_PLAIN;
+	if (!strcasecmp(var+ofs, "prompt"))
+		return CLEAN_COLOR_PROMPT;
+	if (!strcasecmp(var+ofs, "header"))
+		return CLEAN_COLOR_HEADER;
+	if (!strcasecmp(var+ofs, "help"))
+		return CLEAN_COLOR_HELP;
+	if (!strcasecmp(var+ofs, "error"))
+		return CLEAN_COLOR_ERROR;
+	return -1;
+}
+
 static int git_clean_config(const char *var, const char *value, void *cb)
 {
 	/* honors the column.ui config variable only */
 	if (!prefixcmp(var, "column."))
 		return git_column_config(var, value, NULL, &colopts);
 
+	/* honors the color.interactive* config variables which also
+	   applied in git-add--interactive and git-stash */
+	if (!strcmp(var, "color.interactive")) {
+		clean_use_color = git_config_colorbool(var, value);
+		return 0;
+	}
+	if (!prefixcmp(var, "color.interactive.")) {
+		int slot = parse_clean_color_slot(var, 18);
+		if (slot < 0)
+			return 0;
+		if (!value)
+			return config_error_nonbool(var);
+		color_parse(value, var, clean_colors[slot]);
+		return 0;
+	}
+
 	if (!strcmp(var, "clean.requireforce")) {
 		force = !git_config_bool(var, value);
 		return 0;
 	}
-	return git_default_config(var, value, cb);
+
+	/* inspect the color.ui config variable and others */
+	return git_color_default_config(var, value, cb);
+}
+
+static const char *clean_get_color(enum color_clean ix)
+{
+	if (want_color(clean_use_color))
+		return clean_colors[ix];
+	return "";
+}
+
+static void clean_print_color(enum color_clean ix)
+{
+	printf("%s", clean_get_color(ix));
 }
 
 static int exclude_cb(const struct option *opt, const char *arg, int unset)
@@ -189,16 +255,20 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)
 	int changed = -1, i;
 
 	putchar('\n');
+	clean_print_color(CLEAN_COLOR_HELP);
 	printf_ln(_(
 		    "NOTE: Will remove the following items. You can input space-seperated\n"
 		    "NOTE: patterns (just like .gitignore) to exclude items from deletion,\n"
 		    "NOTE: or press ENTER to continue."
 		   ));
+	clean_print_color(CLEAN_COLOR_RESET);
 
 	while (1) {
 		/* dels list may become empty when we run string_list_remove_empty_items later */
 		if (!dels->nr) {
+			clean_print_color(CLEAN_COLOR_ERROR);
 			printf_ln(_("No more files to clean, exiting."));
+			clean_print_color(CLEAN_COLOR_RESET);
 			break;
 		}
 
@@ -209,7 +279,9 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)
 			pretty_print_dels(dels, prefix);
 		}
 
+		clean_print_color(CLEAN_COLOR_PROMPT);
 		printf(_("Input ignore patterns> "));
+		clean_print_color(CLEAN_COLOR_RESET);
 		strbuf_getline(&confirm, stdin, '\n');
 		strbuf_trim(&confirm);
 
@@ -245,7 +317,9 @@ void interactive_clean_edit(struct string_list *dels, const char *prefix)
 		if (changed) {
 			string_list_remove_empty_items(dels, 0);
 		} else {
+			clean_print_color(CLEAN_COLOR_ERROR);
 			printf_ln(_("WARNING: Cannot find items matched by: %s"), confirm.buf);
+			clean_print_color(CLEAN_COLOR_RESET);
 		}
 
 		strbuf_list_free(ignore_list);
@@ -264,19 +338,23 @@ void interactive_clean(struct string_list *dels, const char *prefix)
 	/* dels list may become empty after return back from edit mode */
 	while (dels->nr) {
 		putchar('\n');
+		clean_print_color(CLEAN_COLOR_HEADER);
 		printf_ln(_(
 			    "WARNING: The following items will be removed permanently. Press \"y\"\n"
 			    "WARNING: to start cleaning, and press \"n\" to abort the cleaning.\n"
 			    "WARNING: You can also enter the \"edit\" mode, and select items\n"
 			    "WARNING: to be excluded from the cleaning."
 			   ));
+		clean_print_color(CLEAN_COLOR_RESET);
 		putchar('\n');
 
 		/* Display dels in columns */
 		pretty_print_dels(dels, prefix);
 
 		/* Confirmation dialog */
+		clean_print_color(CLEAN_COLOR_PROMPT);
 		printf(count > 0 ? _("Remove (Yes/no/edit) ? ") : _("Remove (yes/no/Edit) ? "));
+		clean_print_color(CLEAN_COLOR_RESET);
 		strbuf_getline(&confirm, stdin, '\n');
 		strbuf_trim(&confirm);
 
-- 
1.8.3.rc0.364.gc6aefbf

Re: [PATCH v5 0/3] interactive git clean

[Eric Sunshine]

Usability observations below...

On Thu, May 2, 2013 at 11:49 PM, Jiang Xin <worldhello.net@gmail.com> wrote:
> The interactive git clean combines `git clean -n` and `git clean -f`
> together to do safe cleaning, and has more features.
>
> First it displays what would be removed in columns (so that you can
> see them all in one screen). The user must confirm before actually
> cleaning.
>
>     WARNING: The following items will be removed permanently. Press "y"
>     WARNING: to start cleaning, and press "n" to abort the cleaning.
>     WARNING: You can also enter the "edit" mode, and select items
>     WARNING: to be excluded from the cleaning.

The user intended for files to be removed when invoking git-clean,
therefore WARNING that git-clean will do what was requested explicitly
seems overkill. Along the same lines, the user asked explicitly for an
interactive session (via --interactive), hence the above paragraph is
effectively redundant since it does little more than tell the user (in
a lengthy fashion) what he already knows (that the session is
interactive). The short prompt printed after the listed files says the
same thing (more succinctly), thus this warning paragraph is
essentially superfluous.

>       What would be removed...    What would be removed...
>       What would be removed...    What would be removed...
>
>     Remove (yes/no/Edit) ?

For convenience, implementations traditionally allow single letter
responses (y/n/e), but this one does not. Should it?

> In this confirmation dialog, the user has three choices:
>
>  * Yes: Start to do cleaning.
>  * No:  Nothing will be deleted.
>  * Edit (default for the first time): Enter the edit mode.

What about the user who desires more traditional "rm -i" behavior in
which he is prompted for each file? Should that be supported with a
"Prompt [each]" option in the above menu?

> When the user chooses the edit mode, it would look like this:
>
>     NOTE: Will remove the following items. You can input space-seperated
>     NOTE: patterns (just like .gitignore) to exclude items from deletion,
>     NOTE: or press ENTER to continue.

As earlier, this (lengthy) paragraph says little more than what could
be said in a more succinct prompt printed after the file list, thus is
probably superfluous.

>       What would be removed...    What would be removed...
>       What would be removed...    What would be removed...
>
>     Input ignore patterns>

The list of files to be removed was already shown directly above.
Dumping the entire list to the user's screen a second time upon
entering edit mode seems unnecessary. If you drop the WARNING and NOTE
paragraphs as suggested, then the session becomes much less verbose,
thus there is little reason to re-display the file list upon entering
edit mode. For instance:

  % git clean -i
  file1 file2 file3
  file4 file5 file6
  Remove (yes/no/edit)? e
  Exclude (space-separated gitignore patterns): file[4-6]
  file1 file2 file3
  Exclude (space-separated gitignore patterns): [enter]
  Remove (yes/no/edit)?

> The user can input space-separated patterns (the same syntax as gitignore),
> and each clean candidate that matches with one of the patterns will be
> excluded from cleaning.
>
> When the user feels it's OK, presses ENTER and back to the confirmation dialog.
>
>     WARNING: The following items will be removed permanently. Press "y"
>     WARNING: to start cleaning, and press "n" to abort the cleaning.
>     WARNING: You can also enter the "edit" mode, and select items
>     WARNING: to be excluded from the cleaning.
>
>       What would be removed...
>
>     Remove (Yes/no/edit) ?
>
> This time the default choice of the confirmation dialog is "YES".
> So when user press ENTER, start cleaning.

Is there precedent for this sort of self-mutating default action in
other utilities? Wouldn't this lead to high "surprise factor" for
users and potential for lost files? For instance:

  % git clean -i
  file1 file2 file3
  file4 file5 file6
  Remove (yes/no/Edit)? [enter]
    {user presses ENTER for edit mode}
  Exclude (space-separated gitignore patterns): file[3-6]
    {user mistakenly types "3" rather than "4"}
  file1 file2
  Exclude (space-separated gitignore patterns): [enter]
  Remove (Yes/no/edit)? [enter]
    {user notices mistake and presses ENTER expecting edit mode}
  Removing file3
  Removing file4
  Removing file5
  Removing file6

Oh no! The user didn't notice the subtle change of default from
"yes/no/Edit" to "Yes/no/edit",  thus he pressed ENTER thinking it
would take him to edit mode as it did initially, but instead git-clean
proceeded with the removals and file3 is lost.

Other considerations:

Is it necessary to force the user to escape from edit mode by pressing
ENTER (i.e. empty input)? Wouldn't you achieve the same level of
functionality by exiting back to the (yes/no/edit) prompt
automatically after the user enters his gitignore pattern(s)? For
instance:

  % git clean -i
  file1 file2 file3
  file4 file5 file6
  Remove (yes/no/edit)? e
  Exclude (space-separated gitignore patterns): file[4-6]
  file1 file2 file3
  Remove (yes/no/edit)?

More generally, is this sort of modal edit mode desirable and
convenient? Can the edit operation be combined with the top-level
prompt? For example:

  % git clean -i
  file1 file2 file3
  file4 file5 file6
  Remove ([y]es, [n]o, [p]rompt, exclusion-list)? file[4-6]
  file1 file2 file3
  Remove ([y]es, [n]o, [p]rompt, exclusion-list)? p
  file1 (y/n/q/!)? y
  file2 (y/n/q/!)? n
  file3 (y/n/q/!)? y

-- ES

Re: [PATCH v5 0/3] interactive git clean

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> The interactive git clean combines `git clean -n` and `git clean -f`
> together to do safe cleaning, and has more features.
>
> First it displays what would be removed in columns (so that you can
> see them all in one screen). The user must confirm before actually
> cleaning.
>
>     WARNING: The following items will be removed permanently. Press "y"
>     WARNING: to start cleaning, and press "n" to abort the cleaning.
>     WARNING: You can also enter the "edit" mode, and select items
>     WARNING: to be excluded from the cleaning.
>
>       What would be removed...    What would be removed...
>       What would be removed...    What would be removed...

To a user who explicitly _asked_ to run the interactive mode, I find
these four warning lines that shout at the user in all caps way
overkill.  I would have expected the output to begin with a line to
explain what it is listing (e.g. "Cleaning the following files:"),
the list and then

    Remove (Yes/no/edit/?) ?

The existing "add -p" prompt uses this trick to hint the user that a
further help is available by typing "?", and it is a good example to
follow.

Re: [PATCH v5 0/3] interactive git clean

[Jiang Xin]

2013/5/3 Eric Sunshine <sunshine@sunshineco.com>:
>>     WARNING: The following items will be removed permanently. Press "y"
>>     WARNING: to start cleaning, and press "n" to abort the cleaning.
>>     WARNING: You can also enter the "edit" mode, and select items
>>     WARNING: to be excluded from the cleaning.
>
> The user intended for files to be removed when invoking git-clean,
> therefore WARNING that git-clean will do what was requested explicitly
> seems overkill. Along the same lines, the user asked explicitly for an
> interactive session (via --interactive), hence the above paragraph is
> effectively redundant since it does little more than tell the user (in
> a lengthy fashion) what he already knows (that the session is
> interactive). The short prompt printed after the listed files says the
> same thing (more succinctly), thus this warning paragraph is
> essentially superfluous.

I will try to make the header short, and not too scary.

>> In this confirmation dialog, the user has three choices:
>>
>>  * Yes: Start to do cleaning.
>>  * No:  Nothing will be deleted.
>>  * Edit (default for the first time): Enter the edit mode.
>
> What about the user who desires more traditional "rm -i" behavior in
> which he is prompted for each file? Should that be supported with a
> "Prompt [each]" option in the above menu?

I'd like to have a try. Maybe I can borrow code/interface from
git-add--interactive.perl to support both (batch exclude and confirm
one by one). For example:

*** Would remove the following item(s) ***

    files to be removed...     files to be removed...
    files to be removed...     files to be removed...
    files to be removed...     files to be removed...

*** Commands ***
    1. [y]es, clean    2. [n]o, quit    3. batch [e]xclude
4.[c]onfirm one by one
What now> e

input ignore patterns>> * ![a-c]*
    files to be removed...     files to be removed...
    files to be removed...     files to be removed...
Input ignore patterns>> ENTER

*** Would remove the following item(s) ***

    files to be removed...     files to be removed...
    files to be removed...     files to be removed...

*** Commands ***
    1. [y]es, clean    2. [n]o, quit    3. batch [e]xclude
4.[c]onfirm one by one
What now> y

Removing ...
Removing ...

> More generally, is this sort of modal edit mode desirable and
> convenient? Can the edit operation be combined with the top-level
> prompt? For example:
>
>   % git clean -i
>   file1 file2 file3
>   file4 file5 file6
>   Remove ([y]es, [n]o, [p]rompt, exclusion-list)? file[4-6]
>   file1 file2 file3
>   Remove ([y]es, [n]o, [p]rompt, exclusion-list)? p
>   file1 (y/n/q/!)? y
>   file2 (y/n/q/!)? n
>   file3 (y/n/q/!)? y

What If there is a file named 'y', and the user want to exclude it,
and press 'y' as a pattern.

--
Jiang Xin

Re: [PATCH v5 0/3] interactive git clean

[Eric Sunshine]

On Fri, May 3, 2013 at 9:06 PM, Jiang Xin <worldhello.net@gmail.com> wrote:
> 2013/5/3 Eric Sunshine <sunshine@sunshineco.com>:
>> More generally, is this sort of modal edit mode desirable and
>> convenient? Can the edit operation be combined with the top-level
>> prompt? For example:
>>
>>   % git clean -i
>>   file1 file2 file3
>>   file4 file5 file6
>>   Remove ([y]es, [n]o, [p]rompt, exclusion-list)? file[4-6]
>>   file1 file2 file3
>>   Remove ([y]es, [n]o, [p]rompt, exclusion-list)? p
>>   file1 (y/n/q/!)? y
>>   file2 (y/n/q/!)? n
>>   file3 (y/n/q/!)? y
>
> What If there is a file named 'y', and the user want to exclude it,
> and press 'y' as a pattern.

The pattern [y] will match file named 'y'. It probably is unusual for
files named 'y', 'n', etc. to exist in the top-level directory, but
the gitignore patterns already provide an escape hatch for these
unusual cases. (That is not to say that this is the perfect example or
solution, but only that it may be worth considering such options when
designing the user-interface for convenience.)

Re: [PATCH v5 0/3] interactive git clean

[Matthieu Moy]

Eric Sunshine <sunshine@sunshineco.com> writes:

> The pattern [y] will match file named 'y'. It probably is unusual for
> files named 'y', 'n', etc. to exist in the top-level directory, but
> the gitignore patterns already provide an escape hatch for these
> unusual cases.

But how does the user know that?

I'd rather stay away from dwim that works in 99% of cases but do
something dangerous in the 1% remaining, and complex un-guessable escape
scheme to solve these few cases. The two stages (yes/no/edit, and then
escape patterns) is clear, and does not require so many additional
keystrokes.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH v5 0/3] interactive git clean

[Eric Sunshine]

Hi Matthieu,

On Mon, May 6, 2013 at 3:58 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> The pattern [y] will match file named 'y'. It probably is unusual for
>> files named 'y', 'n', etc. to exist in the top-level directory, but
>> the gitignore patterns already provide an escape hatch for these
>> unusual cases.
>
> But how does the user know that?

Documentation. Junio also mentioned having a '?' in each prompt
explaining the options at that point.

> I'd rather stay away from dwim that works in 99% of cases but do
> something dangerous in the 1% remaining, and complex un-guessable escape
> scheme to solve these few cases. The two stages (yes/no/edit, and then
> escape patterns) is clear, and does not require so many additional
> keystrokes.

The scheme doesn't have to be unsafe. git-clean *knows* which files
are up for deletion. If one of those filenames conflicts with one of
the prompt options, --interactive mode can warn about the ambiguity at
the point the user types that particular ambiguous response, and ask
for clarification ("did you mean 'yes, delete everything' or 'exclude
filename y'?"). You get DWIM for 99.9% of the cases, and an extra
prompt for the other 0.1%. Nothing unsafe. Is such an implementation
more complicated or ugly than the separate 'edit' mode? Is the user
experience better or worse (more or less convenient)? Is it easier or
harder to document and explain?

Another observation which came to mind after my original email:

Would it make sense for --interactive mode to launch an editor with
the list of files scheduled for deletion and allow the user to remove
lines he does not want deleted (similar to "git-rebase
--interactive"). Depending upon case, this could be more or less
convenient than the proposed gitignore-patterned 'edit' mode. It could
be yet another option in the yes/no/prompt/edit menu or it could be
the default and only behavior of "git-clean --interactive" (again
similar to "git-rebase --interactive).

I'm not advocating any particular user interface. The observations and
questions about convenience and user experience both here and in my
original email were made with the hope of promoting discussion before
the interface gets locked in. How clunky or fluid should it be? How
verbose or terse?

-- ES

[PATCH v6 0/7] interactive git clean

[Jiang Xin]

Implement a 'git add --interactive' style of interactive git-clean.
It will show what would be done before start to clean.  See
``Interactive mode`` for details.

Interactive mode
----------------
When the command enters the interactive mode, it shows the
files and directories to be cleaned, and goes into its
interactive command loop.

The command loop shows the list of subcommands available, and
gives a prompt "What now> ".  In general, when the prompt ends
with a single '>', you can pick only one of the choices given
and type return, like this:

------------
    *** Commands ***
      1: clean             2: edit by patterns   3: edit by numbers
      4. rm -i             5. quit               6. help
    What now> 2
------------

You also could say `c` or `clean` above as long as the choice is unique.

The main command loop has 6 subcommands.

clean::

   Start cleaning files and directories, and then quit.

edit by patterns::

   This shows the files and directories to be deleted and issues an
   "Input ignore patterns>>" prompt. You can input a space-seperated
   patterns to exclude files and directories from deletion.
   E.g. "*.c *.h" will excludes files end with ".c" and ".h" from
   deletion. When you are satisfied with the filtered result, press
   ENTER (empty) back to the main menu.

edit by numbers::

   This shows the files and directories to be deleted and issues an
   "Select items to delete>>" prompt. When the prompt ends with double
   '>>' like this, you can make more than one selection, concatenated
   with whitespace or comma.  Also you can say ranges.  E.g. "2-5 7,9"
   to choose 2,3,4,5,7,9 from the list.  If the second number in a
   range is omitted, all remaining patches are taken.  E.g. "7-" to
   choose 7,8,9 from the list.  You can say '*' to choose everything.
   Also when you are satisfied with the filtered result, press ENTER
   (empty) back to the main menu.

rm -i::

  This will show a "rm -i" style cleaning, that you must confirm one
  by one in order to delete items. This action is not as efficient
  as the above two actions.

quit::

  This lets you quit without do cleaning.

help::

  Show brief usage of interactive git-clean.



Jiang Xin (7):
  Add support for -i/--interactive to git-clean
  Show items of interactive git-clean in columns
  Add colors to interactive git-clean
  git-clean: use a git-add-interactive compatible UI
  git-clean: interactive cleaning by select numbers
  git-clean: rm -i style interactive cleaning
  git-clean: update document for interactive git-clean

 Documentation/config.txt    |   4 +
 Documentation/git-clean.txt |  71 ++++-
 builtin/clean.c             | 700 ++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 752 insertions(+), 23 deletions(-)

-- 
1.8.3.rc1.338.gb35aa5d

[PATCH v6 1/7] Add support for -i/--interactive to git-clean

[Jiang Xin]

Show what would be done and the user must confirm before actually
cleaning. In the confirmation dialog, the user has three choices:

 * y/yes:  Start to do cleaning.
 * n/no:   Nothing will be deleted.
 * e/edit: Exclude items from deletion using ignore patterns.

When the user chooses the edit mode, the user can input space-
separated patterns (the same syntax as gitignore), and each clean
candidate that matches with one of the patterns will be excluded
from cleaning. When the user feels it's OK, presses ENTER and back
to the confirmation dialog.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Suggested-by: Junio C Hamano <gitster@pobox.com>
Spelling-checked-by: Eric Sunshine <sunshine@sunshineco.com>
Comments-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Suggested-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/git-clean.txt |  15 +++-
 builtin/clean.c             | 195 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 191 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index bdc3a..f5572 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree
 SYNOPSIS
 --------
 [verse]
-'git clean' [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X] [--] <path>...
+'git clean' [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <path>...
 
 DESCRIPTION
 -----------
@@ -34,7 +34,18 @@ OPTIONS
 -f::
 --force::
 	If the Git configuration variable clean.requireForce is not set
-	to false, 'git clean' will refuse to run unless given -f or -n.
+	to false, 'git clean' will refuse to run unless given -f, -n or
+	-i.
+
+-i::
+--interactive::
+	Show what would be done and the user must confirm before actually
+	cleaning. In the confirmation dialog, the user can choose to abort
+	the cleaning, or enter into an edit mode. In the edit mode, the
+	user can input space-separated patterns (the same syntax as
+	gitignore), and each clean candidate that matches with one of the
+	patterns will be excluded from cleaning. When the user feels it's
+	OK, presses ENTER and back to the confirmation dialog.
 
 -n::
 --dry-run::
diff --git a/builtin/clean.c b/builtin/clean.c
index 04e39..29fbf 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -15,9 +15,12 @@
 #include "quote.h"
 
 static int force = -1; /* unset */
+static int interactive;
+static struct string_list del_list = STRING_LIST_INIT_DUP;
+static const char **the_prefix;
 
 static const char *const builtin_clean_usage[] = {
-	N_("git clean [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>..."),
+	N_("git clean [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>..."),
 	NULL
 };
 
@@ -142,6 +145,139 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 	return ret;
 }
 
+void edit_by_patterns_cmd()
+{
+	struct dir_struct dir;
+	struct strbuf confirm = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf **ignore_list;
+	struct string_list_item *item;
+	struct exclude_list *el;
+	const char *qname;
+	int changed = -1, i;
+
+	while (1) {
+		/* dels list may become empty when we run string_list_remove_empty_items later */
+		if (!del_list.nr) {
+			printf_ln(_("No more files to clean, exiting."));
+			break;
+		}
+
+		if (changed) {
+			putchar('\n');
+
+			/* Display dels in "Would remove ..." format */
+			for_each_string_list_item(item, &del_list) {
+				qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
+				printf(_(msg_would_remove), qname);
+			}
+			putchar('\n');
+		}
+
+		printf(_("Input ignore patterns>> "));
+		if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
+			strbuf_trim(&confirm);
+		} else {
+			putchar('\n');
+			break;
+		}
+
+		/* Quit edit mode */
+		if (!confirm.len)
+			break;
+
+		memset(&dir, 0, sizeof(dir));
+		el = add_exclude_list(&dir, EXC_CMDL, "manual exclude");
+		ignore_list = strbuf_split_max(&confirm, ' ', 0);
+
+		for (i = 0; ignore_list[i]; i++) {
+			strbuf_trim(ignore_list[i]);
+			if (!ignore_list[i]->len)
+				continue;
+
+			add_exclude(ignore_list[i]->buf, "", 0, el, -(i+1));
+		}
+
+		changed = 0;
+		for_each_string_list_item(item, &del_list) {
+			int dtype = DT_UNKNOWN;
+			const char *qname;
+
+			qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
+
+			if (is_excluded(&dir, qname, &dtype)) {
+				*item->string = '\0';
+				changed++;
+			}
+		}
+
+		if (changed) {
+			string_list_remove_empty_items(&del_list, 0);
+		} else {
+			printf_ln(_("WARNING: Cannot find items matched by: %s"), confirm.buf);
+		}
+
+		strbuf_list_free(ignore_list);
+		clear_directory(&dir);
+	}
+
+	strbuf_release(&buf);
+	strbuf_release(&confirm);
+}
+
+void interactive_main_loop()
+{
+	struct strbuf confirm = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	struct string_list_item *item;
+	const char *qname;
+
+	/* dels list may become empty after return back from edit mode */
+	while (del_list.nr) {
+		printf_ln(Q_("Would remove the following item:",
+			     "Would remove the following items:",
+			     del_list.nr));
+		putchar('\n');
+
+		/* Display dels in "Would remove ..." format */
+		for_each_string_list_item(item, &del_list) {
+			qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
+			printf(_(msg_would_remove), qname);
+		}
+		putchar('\n');
+
+		/* Confirmation dialog */
+		printf(_("Remove ([y]es/[n]o/[e]dit) ? "));
+		if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
+			strbuf_trim(&confirm);
+		} else {
+			/* Ctrl-D is the same as "quit" */
+			string_list_clear(&del_list, 0);
+			putchar('\n');
+			printf_ln("Bye.");
+			break;
+		}
+
+		if (confirm.len) {
+			if (!strncasecmp(confirm.buf, "yes", confirm.len)) {
+				break;
+			} else if (!strncasecmp(confirm.buf, "no", confirm.len) ||
+				   !strncasecmp(confirm.buf, "quit", confirm.len)) {
+				string_list_clear(&del_list, 0);
+				printf_ln("Bye.");
+				break;
+			} else if (!strncasecmp(confirm.buf, "edit", confirm.len)) {
+				edit_by_patterns_cmd();
+			} else {
+				continue;
+			}
+		}
+	}
+
+	strbuf_release(&buf);
+	strbuf_release(&confirm);
+}
+
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
 	int i, res;
@@ -154,12 +290,14 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	struct strbuf buf = STRBUF_INIT;
 	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 	struct exclude_list *el;
+	struct string_list_item *item;
 	const char *qname;
 	char *seen = NULL;
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("do not print names of files removed")),
 		OPT__DRY_RUN(&dry_run, N_("dry run")),
 		OPT__FORCE(&force, N_("force")),
+		OPT_BOOL('i', "interactive", &interactive, N_("interactive cleaning")),
 		OPT_BOOLEAN('d', NULL, &remove_directories,
 				N_("remove whole directories")),
 		{ OPTION_CALLBACK, 'e', "exclude", &exclude_list, N_("pattern"),
@@ -176,7 +314,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	else
 		config_set = 1;
 
-	argc = parse_options(argc, argv, prefix, options, builtin_clean_usage,
+	the_prefix = &prefix;
+
+	argc = parse_options(argc, argv, *the_prefix, options, builtin_clean_usage,
 			     0);
 
 	memset(&dir, 0, sizeof(dir));
@@ -186,12 +326,16 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	if (ignored && ignored_only)
 		die(_("-x and -X cannot be used together"));
 
-	if (!dry_run && !force) {
+	if (interactive) {
+		if (!isatty(0) || !isatty(1))
+			die(_("interactive clean can not run without a valid tty; "
+				  "refusing to clean"));
+	} else if (!dry_run && !force) {
 		if (config_set)
-			die(_("clean.requireForce set to true and neither -n nor -f given; "
+			die(_("clean.requireForce set to true and neither -i, -n nor -f given; "
 				  "refusing to clean"));
 		else
-			die(_("clean.requireForce defaults to true and neither -n nor -f given; "
+			die(_("clean.requireForce defaults to true and neither -i, -n nor -f given; "
 				  "refusing to clean"));
 	}
 
@@ -210,7 +354,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	for (i = 0; i < exclude_list.nr; i++)
 		add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
 
-	pathspec = get_pathspec(prefix, argv);
+	pathspec = get_pathspec(*the_prefix, argv);
 
 	fill_directory(&dir, pathspec);
 
@@ -257,26 +401,42 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		}
 
 		if (S_ISDIR(st.st_mode)) {
-			strbuf_addstr(&directory, ent->name);
 			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);
-				}
+				string_list_append(&del_list, ent->name);
 			}
-			strbuf_reset(&directory);
 		} else {
 			if (pathspec && !matches)
 				continue;
-			res = dry_run ? 0 : unlink(ent->name);
+			string_list_append(&del_list, ent->name);
+		}
+	}
+
+	if (interactive && del_list.nr > 0 && !dry_run && isatty(0) && isatty(1))
+		interactive_main_loop();
+
+	for_each_string_list_item(item, &del_list) {
+		struct stat st;
+
+		if (lstat(item->string, &st))
+			continue;
+
+		if (S_ISDIR(st.st_mode)) {
+			strbuf_addstr(&directory, item->string);
+			if (remove_dirs(&directory, *the_prefix, rm_flags, dry_run, quiet, &gone))
+				errors++;
+			if (gone && !quiet) {
+				qname = quote_path_relative(directory.buf, directory.len, &buf, *the_prefix);
+				printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
+			}
+			strbuf_reset(&directory);
+		} else {
+			res = dry_run ? 0 : unlink(item->string);
 			if (res) {
-				qname = quote_path_relative(ent->name, -1, &buf, prefix);
+				qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
 				warning(_(msg_warn_remove_failed), qname);
 				errors++;
 			} else if (!quiet) {
-				qname = quote_path_relative(ent->name, -1, &buf, prefix);
+				qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
 				printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
 			}
 		}
@@ -285,5 +445,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	strbuf_release(&directory);
 	string_list_clear(&exclude_list, 0);
+	string_list_clear(&del_list, 0);
 	return (errors != 0);
 }
-- 
1.8.3.rc1.338.gb35aa5d

[PATCH v6 2/7] Show items of interactive git-clean in columns

[Jiang Xin]

When there are lots of items to be cleaned, it is hard to see them all
in one screen. Show them in columns instead of in one column will solve
this problem.

Since no longer show items to be cleaned using the "Would remove ..."
format (only plain filenames) in interactive mode, we add instructions
and warnings as header before them.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Comments-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |  4 ++++
 builtin/clean.c          | 58 +++++++++++++++++++++++++++++++++---------------
 2 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e53f..98bfa 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -955,6 +955,10 @@ column.branch::
 	Specify whether to output branch listing in `git branch` in columns.
 	See `column.ui` for details.
 
+column.clean::
+	Specify whether to output cleaning files in `git clean -i` in columns.
+	See `column.ui` for details.
+
 column.status::
 	Specify whether to output untracked files in `git status` in columns.
 	See `column.ui` for details.
diff --git a/builtin/clean.c b/builtin/clean.c
index 29fbf..43383 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -13,11 +13,13 @@
 #include "refs.h"
 #include "string-list.h"
 #include "quote.h"
+#include "column.h"
 
 static int force = -1; /* unset */
 static int interactive;
 static struct string_list del_list = STRING_LIST_INIT_DUP;
 static const char **the_prefix;
+static unsigned int colopts;
 
 static const char *const builtin_clean_usage[] = {
 	N_("git clean [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>..."),
@@ -32,8 +34,13 @@ 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"))
+	if (!prefixcmp(var, "column."))
+		return git_column_config(var, value, "clean", &colopts);
+
+	if (!strcmp(var, "clean.requireforce")) {
 		force = !git_config_bool(var, value);
+		return 0;
+	}
 	return git_default_config(var, value, cb);
 }
 
@@ -145,6 +152,33 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 	return ret;
 }
 
+void pretty_print_dels()
+{
+	struct string_list list = STRING_LIST_INIT_DUP;
+	struct string_list_item *item;
+	struct strbuf buf = STRBUF_INIT;
+	const char *qname;
+	struct column_options copts;
+
+	for_each_string_list_item(item, &del_list) {
+		qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
+		string_list_append(&list, qname);
+	}
+
+	/*
+	 * always enable column display, we only consult column.*
+	 * about layout strategy and stuff
+	 */
+	colopts = (colopts & ~COL_ENABLE_MASK) | COL_ENABLED;
+	memset(&copts, 0, sizeof(copts));
+	copts.indent = "  ";
+	copts.padding = 2;
+	print_columns(&list, colopts, &copts);
+	putchar('\n');
+	strbuf_release(&buf);
+	string_list_clear(&list, 0);
+}
+
 void edit_by_patterns_cmd()
 {
 	struct dir_struct dir;
@@ -153,7 +187,6 @@ void edit_by_patterns_cmd()
 	struct strbuf **ignore_list;
 	struct string_list_item *item;
 	struct exclude_list *el;
-	const char *qname;
 	int changed = -1, i;
 
 	while (1) {
@@ -166,12 +199,8 @@ void edit_by_patterns_cmd()
 		if (changed) {
 			putchar('\n');
 
-			/* Display dels in "Would remove ..." format */
-			for_each_string_list_item(item, &del_list) {
-				qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
-				printf(_(msg_would_remove), qname);
-			}
-			putchar('\n');
+			/* Display dels in columns */
+			pretty_print_dels();
 		}
 
 		printf(_("Input ignore patterns>> "));
@@ -228,23 +257,17 @@ void edit_by_patterns_cmd()
 void interactive_main_loop()
 {
 	struct strbuf confirm = STRBUF_INIT;
-	struct strbuf buf = STRBUF_INIT;
-	struct string_list_item *item;
-	const char *qname;
 
 	/* dels list may become empty after return back from edit mode */
 	while (del_list.nr) {
+		putchar('\n');
 		printf_ln(Q_("Would remove the following item:",
 			     "Would remove the following items:",
 			     del_list.nr));
 		putchar('\n');
 
-		/* Display dels in "Would remove ..." format */
-		for_each_string_list_item(item, &del_list) {
-			qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
-			printf(_(msg_would_remove), qname);
-		}
-		putchar('\n');
+		/* Display dels in columns */
+		pretty_print_dels();
 
 		/* Confirmation dialog */
 		printf(_("Remove ([y]es/[n]o/[e]dit) ? "));
@@ -274,7 +297,6 @@ void interactive_main_loop()
 		}
 	}
 
-	strbuf_release(&buf);
 	strbuf_release(&confirm);
 }
 
-- 
1.8.3.rc1.338.gb35aa5d

[PATCH v6 3/7] Add colors to interactive git-clean

[Jiang Xin]

Show header, help, error messages, and prompt in colors for interactive
git-clean. Re-use config variables for other git commands, such as
git-add--interactive and git-stash:

 * color.interactive: When set to always, always use colors for
   interactive prompts and displays. When false (or never),
   never. When set to true or auto, use colors only when the
   output is to the terminal.

 * color.interactive.<slot>: Use customized color for interactive
   git-clean output (like git add --interactive). <slot> may be
   prompt, header, help or error.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Comments-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin/clean.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 1 deletion(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 43383..6bda3 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -14,6 +14,7 @@
 #include "string-list.h"
 #include "quote.h"
 #include "column.h"
+#include "color.h"
 
 static int force = -1; /* unset */
 static int interactive;
@@ -32,16 +33,81 @@ static const char *msg_skip_git_dir = N_("Skipping repository %s\n");
 static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
 static const char *msg_warn_remove_failed = N_("failed to remove %s");
 
+static int clean_use_color = -1;
+static char clean_colors[][COLOR_MAXLEN] = {
+	GIT_COLOR_RESET,
+	GIT_COLOR_NORMAL,	/* PLAIN */
+	GIT_COLOR_BOLD_BLUE,	/* PROMPT */
+	GIT_COLOR_BOLD,		/* HEADER */
+	GIT_COLOR_BOLD_RED,	/* HELP */
+	GIT_COLOR_BOLD_RED,	/* ERROR */
+};
+enum color_clean {
+	CLEAN_COLOR_RESET = 0,
+	CLEAN_COLOR_PLAIN = 1,
+	CLEAN_COLOR_PROMPT = 2,
+	CLEAN_COLOR_HEADER = 3,
+	CLEAN_COLOR_HELP = 4,
+	CLEAN_COLOR_ERROR = 5,
+};
+
+static int parse_clean_color_slot(const char *var, int ofs)
+{
+	if (!strcasecmp(var+ofs, "reset"))
+		return CLEAN_COLOR_RESET;
+	if (!strcasecmp(var+ofs, "plain"))
+		return CLEAN_COLOR_PLAIN;
+	if (!strcasecmp(var+ofs, "prompt"))
+		return CLEAN_COLOR_PROMPT;
+	if (!strcasecmp(var+ofs, "header"))
+		return CLEAN_COLOR_HEADER;
+	if (!strcasecmp(var+ofs, "help"))
+		return CLEAN_COLOR_HELP;
+	if (!strcasecmp(var+ofs, "error"))
+		return CLEAN_COLOR_ERROR;
+	return -1;
+}
+
 static int git_clean_config(const char *var, const char *value, void *cb)
 {
 	if (!prefixcmp(var, "column."))
 		return git_column_config(var, value, "clean", &colopts);
 
+	/* honors the color.interactive* config variables which also
+	   applied in git-add--interactive and git-stash */
+	if (!strcmp(var, "color.interactive")) {
+		clean_use_color = git_config_colorbool(var, value);
+		return 0;
+	}
+	if (!prefixcmp(var, "color.interactive.")) {
+		int slot = parse_clean_color_slot(var, 18);
+		if (slot < 0)
+			return 0;
+		if (!value)
+			return config_error_nonbool(var);
+		color_parse(value, var, clean_colors[slot]);
+		return 0;
+	}
+
 	if (!strcmp(var, "clean.requireforce")) {
 		force = !git_config_bool(var, value);
 		return 0;
 	}
-	return git_default_config(var, value, cb);
+
+	/* inspect the color.ui config variable and others */
+	return git_color_default_config(var, value, cb);
+}
+
+static const char *clean_get_color(enum color_clean ix)
+{
+	if (want_color(clean_use_color))
+		return clean_colors[ix];
+	return "";
+}
+
+static void clean_print_color(enum color_clean ix)
+{
+	printf("%s", clean_get_color(ix));
 }
 
 static int exclude_cb(const struct option *opt, const char *arg, int unset)
@@ -192,7 +258,9 @@ void edit_by_patterns_cmd()
 	while (1) {
 		/* dels list may become empty when we run string_list_remove_empty_items later */
 		if (!del_list.nr) {
+			clean_print_color(CLEAN_COLOR_ERROR);
 			printf_ln(_("No more files to clean, exiting."));
+			clean_print_color(CLEAN_COLOR_RESET);
 			break;
 		}
 
@@ -203,7 +271,9 @@ void edit_by_patterns_cmd()
 			pretty_print_dels();
 		}
 
+		clean_print_color(CLEAN_COLOR_PROMPT);
 		printf(_("Input ignore patterns>> "));
+		clean_print_color(CLEAN_COLOR_RESET);
 		if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
 			strbuf_trim(&confirm);
 		} else {
@@ -243,7 +313,9 @@ void edit_by_patterns_cmd()
 		if (changed) {
 			string_list_remove_empty_items(&del_list, 0);
 		} else {
+			clean_print_color(CLEAN_COLOR_ERROR);
 			printf_ln(_("WARNING: Cannot find items matched by: %s"), confirm.buf);
+			clean_print_color(CLEAN_COLOR_RESET);
 		}
 
 		strbuf_list_free(ignore_list);
@@ -261,16 +333,20 @@ void interactive_main_loop()
 	/* dels list may become empty after return back from edit mode */
 	while (del_list.nr) {
 		putchar('\n');
+		clean_print_color(CLEAN_COLOR_HEADER);
 		printf_ln(Q_("Would remove the following item:",
 			     "Would remove the following items:",
 			     del_list.nr));
+		clean_print_color(CLEAN_COLOR_RESET);
 		putchar('\n');
 
 		/* Display dels in columns */
 		pretty_print_dels();
 
 		/* Confirmation dialog */
+		clean_print_color(CLEAN_COLOR_PROMPT);
 		printf(_("Remove ([y]es/[n]o/[e]dit) ? "));
+		clean_print_color(CLEAN_COLOR_RESET);
 		if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
 			strbuf_trim(&confirm);
 		} else {
-- 
1.8.3.rc1.338.gb35aa5d

[PATCH v6 4/7] git-clean: use a git-add-interactive compatible UI

[Jiang Xin]

Rewrite menu using a new method `list_and_choose`, which is borrowed
from `git-add--interactive.perl`. We can reused this method later for
more actions.

Please NOTE:

 * Method `list_and_choose` return an array of integers, and
 * it is up to you to free the allocated memory of the array.
 * The array ends with EOF.
 * If user pressed CTRL-D (i.e. EOF), no selection returned.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 builtin/clean.c | 410 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 367 insertions(+), 43 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 6bda3..3b9f3 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -16,6 +16,35 @@
 #include "column.h"
 #include "color.h"
 
+#define MENU_OPTS_SINGLETON		01
+#define MENU_OPTS_IMMEDIATE		02
+#define MENU_OPTS_LIST_ONLY		04
+
+#define MENU_RETURN_NO_LOOP		10
+
+struct menu_opts {
+	const char *header;
+	const char *prompt;
+	int flag;
+};
+
+enum menu_stuff_type {
+	MENU_STUFF_TYPE_STRING_LIST = 1,
+	MENU_STUFF_TYPE_MENU_ITEM
+};
+
+struct menu_stuff {
+	enum menu_stuff_type type;
+	int nr;
+	void *stuff;
+};
+
+struct menu_item {
+	char hotkey;
+	char *title;
+	int (*fn)();
+};
+
 static int force = -1; /* unset */
 static int interactive;
 static struct string_list del_list = STRING_LIST_INIT_DUP;
@@ -240,12 +269,284 @@ void pretty_print_dels()
 	copts.indent = "  ";
 	copts.padding = 2;
 	print_columns(&list, colopts, &copts);
-	putchar('\n');
 	strbuf_release(&buf);
 	string_list_clear(&list, 0);
 }
 
-void edit_by_patterns_cmd()
+void pretty_print_menus(struct string_list *menu_list)
+{
+	struct strbuf buf = STRBUF_INIT;
+	unsigned int local_colopts = 0;
+	struct column_options copts;
+
+	/*
+	 * always enable column display, we only consult column.*
+	 * about layout strategy and stuff
+	 */
+	local_colopts = COL_ENABLED | COL_ROW;
+	memset(&copts, 0, sizeof(copts));
+	copts.indent = "  ";
+	copts.padding = 2;
+	print_columns(menu_list, local_colopts, &copts);
+	strbuf_release(&buf);
+}
+
+void prompt_help_cmd(int singleton)
+{
+	clean_print_color(CLEAN_COLOR_HELP);
+	printf_ln(singleton ?
+		  _("Prompt help:\n"
+		    "1          - select a numbered item\n"
+		    "foo        - select item based on unique prefix\n"
+		    "           - (empty) select nothing") :
+		  _("Prompt help:\n"
+		    "1          - select a single item\n"
+		    "3-5        - select a range of items\n"
+		    "2-3,6-9    - select multiple ranges\n"
+		    "foo        - select item based on unique prefix\n"
+		    "-...       - unselect specified items\n"
+		    "*          - choose all items\n"
+		    "           - (empty) finish selecting"));
+	clean_print_color(CLEAN_COLOR_RESET);
+}
+
+/*
+ * Implement a git-add-interactive compatible UI, which is borrowed
+ * from git-add--interactive.perl.
+ *
+ * Return value:
+ *
+ *   - Return an array of integers
+ *   - , and it is up to you to free the allocated memory.
+ *   - The array ends with EOF.
+ *   - If user pressed CTRL-D (i.e. EOF), no selection returned.
+ */
+int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff)
+{
+	static struct string_list menu_list = STRING_LIST_INIT_DUP;
+	struct strbuf menu = STRBUF_INIT;
+	struct strbuf choice = STRBUF_INIT;
+	struct strbuf **choice_list;
+	int *chosen, *result;
+	char *p;
+	int nr = 0;
+	int i, j;
+	int eof = 0;
+
+	chosen = xmalloc(sizeof(int) * stuff->nr);
+	memset(chosen, 0, sizeof(int) * stuff->nr);
+
+	while (1) {
+		int i = 0, j = 0;
+		string_list_clear(&menu_list, 0);
+
+		if (opts->header) {
+			printf_ln("%s%s%s",
+				  clean_get_color(CLEAN_COLOR_HEADER),
+				  opts->header,
+				  clean_get_color(CLEAN_COLOR_RESET));
+		}
+
+		/* highlight hotkey in menu */
+		if (MENU_STUFF_TYPE_MENU_ITEM == stuff->type) {
+			struct menu_item *item;
+
+			item = (struct menu_item *)stuff->stuff;
+			for (i = 0; i < stuff->nr; i++, item++) {
+				p = item->title;
+				strbuf_addf(&menu, "%s%2d: ", chosen[i] ? "*" : " ", i+1);
+				for (; *p; p++) {
+					if (*p == item->hotkey) {
+						strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_PROMPT));
+						strbuf_addch(&menu, *p);
+						strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_RESET));
+					} else {
+						strbuf_addch(&menu, *p);
+					}
+				}
+				string_list_append(&menu_list, menu.buf);
+				strbuf_reset(&menu);
+			}
+		} else if (MENU_STUFF_TYPE_STRING_LIST == stuff->type) {
+			struct string_list_item *item;
+			struct strbuf buf = STRBUF_INIT;
+			i = 0;
+
+			for_each_string_list_item(item, (struct string_list *)stuff->stuff) {
+				const char *qname;
+
+				qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
+				strbuf_addf(&menu, "%s%2d: %s", chosen[i] ? "*" : " ", ++i, qname);
+				string_list_append(&menu_list, menu.buf);
+				strbuf_reset(&menu);
+			}
+		}
+
+		pretty_print_menus(&menu_list);
+
+		if (opts->flag & MENU_OPTS_LIST_ONLY)
+			break;
+
+		if (opts->prompt) {
+			printf("%s%s%s%s",
+			       clean_get_color(CLEAN_COLOR_PROMPT),
+			       opts->prompt,
+			       opts->flag & MENU_OPTS_SINGLETON ? "> " : ">> ",
+			       clean_get_color(CLEAN_COLOR_RESET));
+		}
+
+		if (strbuf_getline(&choice, stdin, '\n') != EOF) {
+			if (!(opts->flag & MENU_OPTS_SINGLETON)) {
+				char *p = choice.buf;
+				do {
+					if (*p == ',')
+						*p = ' ';
+				} while (*p++);
+			}
+			strbuf_trim(&choice);
+		} else {
+			eof = 1;
+			break;
+		}
+
+		/* help for prompt */
+		if (!strcmp(choice.buf, "?")) {
+			prompt_help_cmd(opts->flag & MENU_OPTS_SINGLETON);
+			continue;
+		}
+
+		if (!(opts->flag & MENU_OPTS_SINGLETON) && !choice.len)
+			break;
+
+		choice_list = strbuf_split_max(&choice, ' ', 0);
+		for (i = 0; choice_list[i]; i++) {
+			int choose = 1;
+			int bottom = 0, top = 0;
+			char *p;
+			int is_range = 0;
+			int is_number = 1;
+
+			strbuf_trim(choice_list[i]);
+			if (!choice_list[i]->len)
+				continue;
+
+			/* Input that begins with '-'; unchoose */
+			if (*choice_list[i]->buf == '-') {
+				choose = 0;
+				strbuf_remove(choice_list[i], 0, 1);
+			}
+
+			p = choice_list[i]->buf;
+			for(; *p; p++) {
+				if ('-' == *p) {
+					if (!is_range) {
+						is_range = 1;
+						is_number = 0;
+					} else {
+						is_number = 0;
+						is_range = 0;
+						break;
+					}
+				} else if (!isdigit(*p)) {
+					is_number = 0;
+					is_range = 0;
+					break;
+				}
+			}
+
+			if (is_number) {
+				bottom = atoi(choice_list[i]->buf);
+				top = bottom;
+			} else if (is_range) {
+				bottom = atoi(choice_list[i]->buf);
+				if (!*(strchr(choice_list[i]->buf, '-') + 1)) {
+					top = stuff->nr - 1;
+				} else {
+					top = atoi(strchr(choice_list[i]->buf, '-') + 1);
+				}
+			} else if (!strcmp(choice_list[i]->buf, "*")) {
+				bottom = 1;
+				top = stuff->nr;
+			} else {
+				if (MENU_STUFF_TYPE_MENU_ITEM == stuff->type) {
+					struct menu_item *item;
+
+					item = (struct menu_item *)stuff->stuff;
+					for (j = 0; j < stuff->nr; j++, item++) {
+						if ((choice_list[i]->len == 1 &&
+						     *choice_list[i]->buf == item->hotkey) ||
+						    !strcasecmp(choice_list[i]->buf, item->title)) {
+							bottom = j + 1;
+							top = bottom;
+							break;
+						}
+					}
+				} else if (MENU_STUFF_TYPE_STRING_LIST == stuff->type) {
+					struct string_list_item *item;
+
+					item = ((struct string_list *)stuff->stuff)->items;
+					for (j = 0; j < stuff->nr; j++, item++) {
+						if (!strcasecmp(choice_list[i]->buf, item->string)) {
+							bottom = j + 1;
+							top = bottom;
+							break;
+						}
+					}
+				}
+			}
+
+			if (top <= 0 || bottom <= 0 || top > stuff-> nr || bottom > top ||
+			    (opts->flag & MENU_OPTS_SINGLETON && bottom != top)) {
+				printf_ln("%sHuh (%s)?%s",
+					  clean_get_color(CLEAN_COLOR_ERROR),
+					  choice_list[i]->buf,
+					  clean_get_color(CLEAN_COLOR_RESET));
+				continue;
+			}
+
+			/* A range can be specified like 5-7 or 5-. */
+			for (j = bottom; j <= top; j++) {
+				chosen[j-1] = choose;
+				nr++;
+			}
+		}
+
+		if (opts->flag & MENU_OPTS_SINGLETON) {
+			if (nr)
+				break;
+		} else if (opts->flag & MENU_OPTS_IMMEDIATE) {
+			break;
+		}
+	}
+
+
+	if (eof) {
+		result = xmalloc(sizeof(int) * 2);
+		result[0] = EOF;
+		result[1] = 0;
+	} else {
+		result = xmalloc(sizeof(int) * (nr + 1));
+		memset(result, 0, sizeof(int) * (nr + 1));
+		for (i = 0, j = 0; i < stuff->nr && j < nr; i++) {
+			if (chosen[i])
+				result[j++] = i;
+		}
+		result[j] = EOF;
+	}
+
+	free(chosen);
+	string_list_clear(&menu_list, 0);
+	strbuf_release(&menu);
+	strbuf_release(&choice);
+	return result;
+}
+
+int clean_cmd()
+{
+	return MENU_RETURN_NO_LOOP;
+}
+
+int edit_by_patterns_cmd()
 {
 	struct dir_struct dir;
 	struct strbuf confirm = STRBUF_INIT;
@@ -257,16 +558,10 @@ void edit_by_patterns_cmd()
 
 	while (1) {
 		/* dels list may become empty when we run string_list_remove_empty_items later */
-		if (!del_list.nr) {
-			clean_print_color(CLEAN_COLOR_ERROR);
-			printf_ln(_("No more files to clean, exiting."));
-			clean_print_color(CLEAN_COLOR_RESET);
+		if (!del_list.nr)
 			break;
-		}
 
 		if (changed) {
-			putchar('\n');
-
 			/* Display dels in columns */
 			pretty_print_dels();
 		}
@@ -324,56 +619,86 @@ void edit_by_patterns_cmd()
 
 	strbuf_release(&buf);
 	strbuf_release(&confirm);
+	return 0;
 }
 
-void interactive_main_loop()
+int quit_cmd()
 {
-	struct strbuf confirm = STRBUF_INIT;
+	string_list_clear(&del_list, 0);
+	printf_ln(_("Bye."));
+	return MENU_RETURN_NO_LOOP;
+}
 
+int help_cmd(int x)
+{
+	clean_print_color(CLEAN_COLOR_HELP);
+	printf_ln(_(
+		    "clean            - start cleaning\n"
+		    "edit by patterns - exclude items from deletion\n"
+		    "quit             - stop cleaning\n"
+		    "help             - this screen\n"
+		    "?                - help for prompt selection"
+		   ));
+	clean_print_color(CLEAN_COLOR_RESET);
+	return 0;
+}
+
+void interactive_main_loop()
+{
 	/* dels list may become empty after return back from edit mode */
 	while (del_list.nr) {
-		putchar('\n');
+		struct menu_opts menu_opts;
+		struct menu_stuff menu_stuff;
+		struct menu_item menus[] = {
+			{'c', "clean",			clean_cmd},
+			{'p', "edit by patterns",	edit_by_patterns_cmd},
+			{'q', "quit",			quit_cmd},
+			{'h', "help",			help_cmd},
+		};
+		int *chosen;
+
+		menu_opts.header = _("*** Commands ***");
+		menu_opts.prompt = "What now";
+		menu_opts.flag = MENU_OPTS_SINGLETON;
+
+		menu_stuff.type = MENU_STUFF_TYPE_MENU_ITEM;
+		menu_stuff.stuff = menus;
+		menu_stuff.nr = sizeof(menus) / sizeof(struct menu_item);
+
 		clean_print_color(CLEAN_COLOR_HEADER);
 		printf_ln(Q_("Would remove the following item:",
 			     "Would remove the following items:",
 			     del_list.nr));
 		clean_print_color(CLEAN_COLOR_RESET);
-		putchar('\n');
 
-		/* Display dels in columns */
+		/* display dels in columns */
 		pretty_print_dels();
 
-		/* Confirmation dialog */
-		clean_print_color(CLEAN_COLOR_PROMPT);
-		printf(_("Remove ([y]es/[n]o/[e]dit) ? "));
-		clean_print_color(CLEAN_COLOR_RESET);
-		if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
-			strbuf_trim(&confirm);
-		} else {
-			/* Ctrl-D is the same as "quit" */
-			string_list_clear(&del_list, 0);
-			putchar('\n');
-			printf_ln("Bye.");
-			break;
-		}
-
-		if (confirm.len) {
-			if (!strncasecmp(confirm.buf, "yes", confirm.len)) {
-				break;
-			} else if (!strncasecmp(confirm.buf, "no", confirm.len) ||
-				   !strncasecmp(confirm.buf, "quit", confirm.len)) {
-				string_list_clear(&del_list, 0);
-				printf_ln("Bye.");
-				break;
-			} else if (!strncasecmp(confirm.buf, "edit", confirm.len)) {
-				edit_by_patterns_cmd();
-			} else {
+		/* main menu */
+		chosen = list_and_choose(&menu_opts, &menu_stuff);
+
+		if (*chosen != EOF) {
+			int ret;
+			ret = menus[*chosen].fn(1);
+			if (ret != MENU_RETURN_NO_LOOP) {
+				free(chosen);
+				chosen = NULL;
+				if (!del_list.nr) {
+					clean_print_color(CLEAN_COLOR_ERROR);
+					printf_ln(_("No more files to clean, exiting."));
+					clean_print_color(CLEAN_COLOR_RESET);
+					break;
+				}
 				continue;
 			}
+		} else {
+			quit_cmd();
 		}
-	}
 
-	strbuf_release(&confirm);
+		free(chosen);
+		chosen = NULL;
+		break;
+	}
 }
 
 int cmd_clean(int argc, const char **argv, const char *prefix)
@@ -499,9 +824,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		}
 
 		if (S_ISDIR(st.st_mode)) {
-			if (remove_directories || (matches == MATCHED_EXACTLY)) {
+			if (remove_directories || (matches == MATCHED_EXACTLY))
 				string_list_append(&del_list, ent->name);
-			}
 		} else {
 			if (pathspec && !matches)
 				continue;
-- 
1.8.3.rc1.338.gb35aa5d

[PATCH v6 5/7] git-clean: interactive cleaning by select numbers

[Jiang Xin]

Draw a multiple choice menu using `list_and_choose` to select items
to be deleted by numbers.

User can input:

 *  1,5-7 : select 1,5,6,7 items to be deleted
 *  *     : select all items to be deleted
 *  -*    : unselect all, nothing will be deleted
 *        : (empty) finish selecting, and return back to main menu

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 builtin/clean.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/builtin/clean.c b/builtin/clean.c
index 3b9f3..3b07f 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -622,6 +622,43 @@ int edit_by_patterns_cmd()
 	return 0;
 }
 
+int edit_by_numbers_cmd()
+{
+	struct menu_opts menu_opts;
+	struct menu_stuff menu_stuff;
+	struct string_list_item *items;
+	int *chosen;
+	int i, j;
+
+	menu_opts.header = NULL;
+	menu_opts.prompt = "Select items to delete";
+	menu_opts.flag = 0;
+
+	menu_stuff.type = MENU_STUFF_TYPE_STRING_LIST;
+	menu_stuff.stuff = &del_list;
+	menu_stuff.nr = del_list.nr;
+
+	chosen = list_and_choose(&menu_opts, &menu_stuff);
+	items = del_list.items;
+	for(i = 0, j = 0; i < del_list.nr; i++) {
+		if (i < chosen[j]) {
+			*(items[i].string) = '\0';
+		} else if (i == chosen[j]) {
+			/* delete selected item */
+			j++;
+			continue;
+		} else {
+			/* end of chosen (EOF), won't delete */
+			*(items[i].string) = '\0';
+		}
+	}
+
+	string_list_remove_empty_items(&del_list, 0);
+
+	free(chosen);
+	return 0;
+}
+
 int quit_cmd()
 {
 	string_list_clear(&del_list, 0);
@@ -635,6 +672,7 @@ int help_cmd(int x)
 	printf_ln(_(
 		    "clean            - start cleaning\n"
 		    "edit by patterns - exclude items from deletion\n"
+		    "edit by numbers  - select items to be deleted by numbers\n"
 		    "quit             - stop cleaning\n"
 		    "help             - this screen\n"
 		    "?                - help for prompt selection"
@@ -652,6 +690,7 @@ void interactive_main_loop()
 		struct menu_item menus[] = {
 			{'c', "clean",			clean_cmd},
 			{'p', "edit by patterns",	edit_by_patterns_cmd},
+			{'n', "edit by numbers",	edit_by_numbers_cmd},
 			{'q', "quit",			quit_cmd},
 			{'h', "help",			help_cmd},
 		};
-- 
1.8.3.rc1.338.gb35aa5d

[PATCH v6 6/7] git-clean: rm -i style interactive cleaning

[Jiang Xin]

Add a "rm -i" style interactive cleaning method. User must confirm one
by one before starting to delete.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 builtin/clean.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/builtin/clean.c b/builtin/clean.c
index 3b07f..f36ad 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -659,6 +659,40 @@ int edit_by_numbers_cmd()
 	return 0;
 }
 
+int rm_i_cmd()
+{
+	struct strbuf confirm = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	struct string_list_item *item;
+	const char *qname;
+	int changed = 0, eof = 0;
+
+	for_each_string_list_item(item, &del_list) {
+		/* Ctrl-D should stop removing files */
+		if (!eof) {
+			qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
+			printf(_("remove %s ? "), qname);
+			if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
+				strbuf_trim(&confirm);
+			} else {
+				putchar('\n');
+				eof = 1;
+			}
+		}
+		if (!confirm.len || !strncasecmp(confirm.buf, "no", confirm.len)) {
+			*item->string = '\0';
+			changed++;
+		}
+	}
+
+	if (changed)
+		string_list_remove_empty_items(&del_list, 0);
+
+	strbuf_release(&buf);
+	strbuf_release(&confirm);
+	return MENU_RETURN_NO_LOOP;
+}
+
 int quit_cmd()
 {
 	string_list_clear(&del_list, 0);
@@ -673,6 +707,7 @@ int help_cmd(int x)
 		    "clean            - start cleaning\n"
 		    "edit by patterns - exclude items from deletion\n"
 		    "edit by numbers  - select items to be deleted by numbers\n"
+		    "rm -i            - delete items one by one, like \"rm -i\"\n"
 		    "quit             - stop cleaning\n"
 		    "help             - this screen\n"
 		    "?                - help for prompt selection"
@@ -691,6 +726,7 @@ void interactive_main_loop()
 			{'c', "clean",			clean_cmd},
 			{'p', "edit by patterns",	edit_by_patterns_cmd},
 			{'n', "edit by numbers",	edit_by_numbers_cmd},
+			{'i', "rm -i",			rm_i_cmd},
 			{'q', "quit",			quit_cmd},
 			{'h', "help",			help_cmd},
 		};
-- 
1.8.3.rc1.338.gb35aa5d

[PATCH v6 7/7] git-clean: update document for interactive git-clean

[Jiang Xin]

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 Documentation/git-clean.txt | 70 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index f5572..56d60 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -39,13 +39,8 @@ OPTIONS
 
 -i::
 --interactive::
-	Show what would be done and the user must confirm before actually
-	cleaning. In the confirmation dialog, the user can choose to abort
-	the cleaning, or enter into an edit mode. In the edit mode, the
-	user can input space-separated patterns (the same syntax as
-	gitignore), and each clean candidate that matches with one of the
-	patterns will be excluded from cleaning. When the user feels it's
-	OK, presses ENTER and back to the confirmation dialog.
+	Show what would be done and clean files interactively. See
+	``Interactive mode`` for details.
 
 -n::
 --dry-run::
@@ -74,6 +69,67 @@ OPTIONS
 	Remove only files ignored by Git.  This may be useful to rebuild
 	everything from scratch, but keep manually created files.
 
+Interactive mode
+----------------
+When the command enters the interactive mode, it shows the
+files and directories to be cleaned, and goes into its
+interactive command loop.
+
+The command loop shows the list of subcommands available, and
+gives a prompt "What now> ".  In general, when the prompt ends
+with a single '>', you can pick only one of the choices given
+and type return, like this:
+
+------------
+    *** Commands ***
+      1: clean             2: edit by patterns   3: edit by numbers
+      4. rm -i             5. quit               6. help
+    What now> 2
+------------
+
+You also could say `c` or `clean` above as long as the choice is unique.
+
+The main command loop has 6 subcommands.
+
+clean::
+
+   Start cleaning files and directories, and then quit.
+
+edit by patterns::
+
+   This shows the files and directories to be deleted and issues an
+   "Input ignore patterns>>" prompt. You can input a space-seperated
+   patterns to exclude files and directories from deletion.
+   E.g. "*.c *.h" will excludes files end with ".c" and ".h" from
+   deletion. When you are satisfied with the filtered result, press
+   ENTER (empty) back to the main menu.
+
+edit by numbers::
+
+   This shows the files and directories to be deleted and issues an
+   "Select items to delete>>" prompt. When the prompt ends with double
+   '>>' like this, you can make more than one selection, concatenated
+   with whitespace or comma.  Also you can say ranges.  E.g. "2-5 7,9"
+   to choose 2,3,4,5,7,9 from the list.  If the second number in a
+   range is omitted, all remaining patches are taken.  E.g. "7-" to
+   choose 7,8,9 from the list.  You can say '*' to choose everything.
+   Also when you are satisfied with the filtered result, press ENTER
+   (empty) back to the main menu.
+
+rm -i::
+
+  This will show a "rm -i" style cleaning, that you must confirm one
+  by one in order to delete items. This action is not as efficient
+  as the above two actions.
+
+quit::
+
+  This lets you quit without do cleaning.
+
+help::
+
+  Show brief usage of interactive git-clean.
+
 SEE ALSO
 --------
 linkgit:gitignore[5]
-- 
1.8.3.rc1.338.gb35aa5d

Re: [PATCH v6 4/7] git-clean: use a git-add-interactive compatible UI

[Jiang Xin]

2013/5/7 Jiang Xin <worldhello.net@gmail.com>:
> Rewrite menu using a new method `list_and_choose`, which is borrowed
> from `git-add--interactive.perl`. We can reused this method later for
> more actions.
>
> Please NOTE:
>
>  * Method `list_and_choose` return an array of integers, and
>  * it is up to you to free the allocated memory of the array.
>  * The array ends with EOF.
>  * If user pressed CTRL-D (i.e. EOF), no selection returned.
>
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> ---
>  builtin/clean.c | 410 ++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 367 insertions(+), 43 deletions(-)
>
> diff --git a/builtin/clean.c b/builtin/clean.c
> index 6bda3..3b9f3 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -16,6 +16,35 @@
>  #include "column.h"
>  #include "color.h"
>
> +#define MENU_OPTS_SINGLETON            01
> +#define MENU_OPTS_IMMEDIATE            02
> +#define MENU_OPTS_LIST_ONLY            04
> +
> +#define MENU_RETURN_NO_LOOP            10
> +
> +struct menu_opts {
> +       const char *header;
> +       const char *prompt;
> +       int flag;
> +};
> +
> +enum menu_stuff_type {
> +       MENU_STUFF_TYPE_STRING_LIST = 1,
> +       MENU_STUFF_TYPE_MENU_ITEM
> +};
> +
> +struct menu_stuff {
> +       enum menu_stuff_type type;
> +       int nr;
> +       void *stuff;
> +};
> +
> +struct menu_item {
> +       char hotkey;
> +       char *title;
> +       int (*fn)();
> +};
> +
>  static int force = -1; /* unset */
>  static int interactive;
>  static struct string_list del_list = STRING_LIST_INIT_DUP;
> @@ -240,12 +269,284 @@ void pretty_print_dels()
>         copts.indent = "  ";
>         copts.padding = 2;
>         print_columns(&list, colopts, &copts);
> -       putchar('\n');
>         strbuf_release(&buf);
>         string_list_clear(&list, 0);
>  }
>
> -void edit_by_patterns_cmd()
> +void pretty_print_menus(struct string_list *menu_list)
> +{
> +       struct strbuf buf = STRBUF_INIT;
unused buf should be deleted.

> +       unsigned int local_colopts = 0;
> +       struct column_options copts;
> +
> +       /*
> +        * always enable column display, we only consult column.*
> +        * about layout strategy and stuff
> +        */
remove the above comments.

> +       local_colopts = COL_ENABLED | COL_ROW;
> +       memset(&copts, 0, sizeof(copts));
> +       copts.indent = "  ";
> +       copts.padding = 2;
> +       print_columns(menu_list, local_colopts, &copts);
> +       strbuf_release(&buf);
remove strbuf_release of unused variable : buf.

> +}
> +
> +void prompt_help_cmd(int singleton)
> +{
> +       clean_print_color(CLEAN_COLOR_HELP);
> +       printf_ln(singleton ?
> +                 _("Prompt help:\n"
> +                   "1          - select a numbered item\n"
> +                   "foo        - select item based on unique prefix\n"
> +                   "           - (empty) select nothing") :
> +                 _("Prompt help:\n"
> +                   "1          - select a single item\n"
> +                   "3-5        - select a range of items\n"
> +                   "2-3,6-9    - select multiple ranges\n"
> +                   "foo        - select item based on unique prefix\n"
> +                   "-...       - unselect specified items\n"
> +                   "*          - choose all items\n"
> +                   "           - (empty) finish selecting"));
> +       clean_print_color(CLEAN_COLOR_RESET);
> +}
> +
> +/*
> + * Implement a git-add-interactive compatible UI, which is borrowed
> + * from git-add--interactive.perl.
> + *
> + * Return value:
> + *
> + *   - Return an array of integers
> + *   - , and it is up to you to free the allocated memory.
> + *   - The array ends with EOF.
> + *   - If user pressed CTRL-D (i.e. EOF), no selection returned.
> + */
> +int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff)
> +{
> +       static struct string_list menu_list = STRING_LIST_INIT_DUP;
> +       struct strbuf menu = STRBUF_INIT;
> +       struct strbuf choice = STRBUF_INIT;
> +       struct strbuf **choice_list;
> +       int *chosen, *result;
> +       char *p;
> +       int nr = 0;
> +       int i, j;
> +       int eof = 0;
> +
> +       chosen = xmalloc(sizeof(int) * stuff->nr);
> +       memset(chosen, 0, sizeof(int) * stuff->nr);
> +
> +       while (1) {
> +               int i = 0, j = 0;
> +               string_list_clear(&menu_list, 0);
> +
> +               if (opts->header) {
> +                       printf_ln("%s%s%s",
> +                                 clean_get_color(CLEAN_COLOR_HEADER),
> +                                 opts->header,
> +                                 clean_get_color(CLEAN_COLOR_RESET));
> +               }
> +
> +               /* highlight hotkey in menu */
> +               if (MENU_STUFF_TYPE_MENU_ITEM == stuff->type) {
> +                       struct menu_item *item;
> +
> +                       item = (struct menu_item *)stuff->stuff;
> +                       for (i = 0; i < stuff->nr; i++, item++) {
> +                               p = item->title;
> +                               strbuf_addf(&menu, "%s%2d: ", chosen[i] ? "*" : " ", i+1);
> +                               for (; *p; p++) {
> +                                       if (*p == item->hotkey) {
> +                                               strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_PROMPT));
> +                                               strbuf_addch(&menu, *p);
> +                                               strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_RESET));
> +                                       } else {
> +                                               strbuf_addch(&menu, *p);
> +                                       }
> +                               }
> +                               string_list_append(&menu_list, menu.buf);
> +                               strbuf_reset(&menu);
> +                       }
> +               } else if (MENU_STUFF_TYPE_STRING_LIST == stuff->type) {
> +                       struct string_list_item *item;
> +                       struct strbuf buf = STRBUF_INIT;
should call strbuf_release later

> +                       i = 0;
> +
> +                       for_each_string_list_item(item, (struct string_list *)stuff->stuff) {
> +                               const char *qname;
> +
> +                               qname = quote_path_relative(item->string, -1, &buf, *the_prefix);
> +                               strbuf_addf(&menu, "%s%2d: %s", chosen[i] ? "*" : " ", ++i, qname);
> +                               string_list_append(&menu_list, menu.buf);
> +                               strbuf_reset(&menu);
> +                       }
+                       strbuf_release(&buf);

> +               }
> +
> +               pretty_print_menus(&menu_list);
> +
> +               if (opts->flag & MENU_OPTS_LIST_ONLY)
> +                       break;
> +
> +               if (opts->prompt) {
> +                       printf("%s%s%s%s",
> +                              clean_get_color(CLEAN_COLOR_PROMPT),
> +                              opts->prompt,
> +                              opts->flag & MENU_OPTS_SINGLETON ? "> " : ">> ",
> +                              clean_get_color(CLEAN_COLOR_RESET));
> +               }
> +
> +               if (strbuf_getline(&choice, stdin, '\n') != EOF) {
> +                       if (!(opts->flag & MENU_OPTS_SINGLETON)) {
> +                               char *p = choice.buf;
> +                               do {
> +                                       if (*p == ',')
> +                                               *p = ' ';
> +                               } while (*p++);
> +                       }
> +                       strbuf_trim(&choice);
> +               } else {
> +                       eof = 1;
> +                       break;
> +               }
> +
> +               /* help for prompt */
> +               if (!strcmp(choice.buf, "?")) {
> +                       prompt_help_cmd(opts->flag & MENU_OPTS_SINGLETON);
> +                       continue;
> +               }
> +
> +               if (!(opts->flag & MENU_OPTS_SINGLETON) && !choice.len)
> +                       break;
> +
> +               choice_list = strbuf_split_max(&choice, ' ', 0);
Should be freed later

> +               for (i = 0; choice_list[i]; i++) {
> +                       int choose = 1;
> +                       int bottom = 0, top = 0;
> +                       char *p;
> +                       int is_range = 0;
> +                       int is_number = 1;
> +
> +                       strbuf_trim(choice_list[i]);
> +                       if (!choice_list[i]->len)
> +                               continue;
> +
> +                       /* Input that begins with '-'; unchoose */
> +                       if (*choice_list[i]->buf == '-') {
> +                               choose = 0;
> +                               strbuf_remove(choice_list[i], 0, 1);
> +                       }
> +
> +                       p = choice_list[i]->buf;
> +                       for(; *p; p++) {
> +                               if ('-' == *p) {
> +                                       if (!is_range) {
> +                                               is_range = 1;
> +                                               is_number = 0;
> +                                       } else {
> +                                               is_number = 0;
> +                                               is_range = 0;
> +                                               break;
> +                                       }
> +                               } else if (!isdigit(*p)) {
> +                                       is_number = 0;
> +                                       is_range = 0;
> +                                       break;
> +                               }
> +                       }
> +
> +                       if (is_number) {
> +                               bottom = atoi(choice_list[i]->buf);
> +                               top = bottom;
> +                       } else if (is_range) {
> +                               bottom = atoi(choice_list[i]->buf);
> +                               if (!*(strchr(choice_list[i]->buf, '-') + 1)) {
> +                                       top = stuff->nr - 1;
> +                               } else {
> +                                       top = atoi(strchr(choice_list[i]->buf, '-') + 1);
> +                               }
> +                       } else if (!strcmp(choice_list[i]->buf, "*")) {
> +                               bottom = 1;
> +                               top = stuff->nr;
> +                       } else {
> +                               if (MENU_STUFF_TYPE_MENU_ITEM == stuff->type) {
> +                                       struct menu_item *item;
> +
> +                                       item = (struct menu_item *)stuff->stuff;
> +                                       for (j = 0; j < stuff->nr; j++, item++) {
> +                                               if ((choice_list[i]->len == 1 &&
> +                                                    *choice_list[i]->buf == item->hotkey) ||
> +                                                   !strcasecmp(choice_list[i]->buf, item->title)) {
> +                                                       bottom = j + 1;
> +                                                       top = bottom;
> +                                                       break;
> +                                               }
> +                                       }
> +                               } else if (MENU_STUFF_TYPE_STRING_LIST == stuff->type) {
> +                                       struct string_list_item *item;
> +
> +                                       item = ((struct string_list *)stuff->stuff)->items;
> +                                       for (j = 0; j < stuff->nr; j++, item++) {
> +                                               if (!strcasecmp(choice_list[i]->buf, item->string)) {
> +                                                       bottom = j + 1;
> +                                                       top = bottom;
> +                                                       break;
> +                                               }
> +                                       }
> +                               }
> +                       }
> +
> +                       if (top <= 0 || bottom <= 0 || top > stuff-> nr || bottom > top ||
> +                           (opts->flag & MENU_OPTS_SINGLETON && bottom != top)) {
> +                               printf_ln("%sHuh (%s)?%s",
> +                                         clean_get_color(CLEAN_COLOR_ERROR),
> +                                         choice_list[i]->buf,
> +                                         clean_get_color(CLEAN_COLOR_RESET));
> +                               continue;
> +                       }
> +
> +                       /* A range can be specified like 5-7 or 5-. */
> +                       for (j = bottom; j <= top; j++) {
> +                               chosen[j-1] = choose;
> +                               nr++;
> +                       }
> +               }

+               strbuf_list_free(choice_list);

> +
> +               if (opts->flag & MENU_OPTS_SINGLETON) {
> +                       if (nr)
> +                               break;
> +               } else if (opts->flag & MENU_OPTS_IMMEDIATE) {
> +                       break;
> +               }
> +       }
> +
> +
> +       if (eof) {
> +               result = xmalloc(sizeof(int) * 2);
> +               result[0] = EOF;
> +               result[1] = 0;
Allocate one element is OK, like:

+               result = xmalloc(sizeof(int));
+               *result = EOF;

> +       } else {
> +               result = xmalloc(sizeof(int) * (nr + 1));
> +               memset(result, 0, sizeof(int) * (nr + 1));

Add initial for j here:

+               j = 0;

> +               for (i = 0, j = 0; i < stuff->nr && j < nr; i++) {
> +                       if (chosen[i])
> +                               result[j++] = i;
> +               }
> +               result[j] = EOF;
> +       }
> +
> +       free(chosen);
> +       string_list_clear(&menu_list, 0);
> +       strbuf_release(&menu);
> +       strbuf_release(&choice);
> +       return result;
> +}


-- 
Jiang Xin

Re: [PATCH v6 7/7] git-clean: update document for interactive git-clean

[Jiang Xin]

2013/5/7 Jiang Xin <worldhello.net@gmail.com>:
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> ---
>  Documentation/git-clean.txt | 70 ++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 63 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
> index f5572..56d60 100644
> --- a/Documentation/git-clean.txt
> +++ b/Documentation/git-clean.txt
> @@ -39,13 +39,8 @@ OPTIONS
>
>  -i::
>  --interactive::
> -       Show what would be done and the user must confirm before actually
> -       cleaning. In the confirmation dialog, the user can choose to abort
> -       the cleaning, or enter into an edit mode. In the edit mode, the
> -       user can input space-separated patterns (the same syntax as
> -       gitignore), and each clean candidate that matches with one of the
> -       patterns will be excluded from cleaning. When the user feels it's
> -       OK, presses ENTER and back to the confirmation dialog.
> +       Show what would be done and clean files interactively. See
> +       ``Interactive mode`` for details.
          ^^^^^^^^^^^^^^^^^^^^ should be ``Interactive mode''


-- 
Jiang Xin

Re: [PATCH v6 4/7] git-clean: use a git-add-interactive compatible UI

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> 2013/5/7 Jiang Xin <worldhello.net@gmail.com>:
>> Rewrite menu using a new method `list_and_choose`, which is borrowed
>> from `git-add--interactive.perl`. We can reused this method later for
>> more actions.
>>
>> Please NOTE:
>>
>>  * Method `list_and_choose` return an array of integers, and
>>  * it is up to you to free the allocated memory of the array.
>>  * The array ends with EOF.
>>  * If user pressed CTRL-D (i.e. EOF), no selection returned.
>>
>> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
>> ---
>>  builtin/clean.c | 410 ++++++++++++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 367 insertions(+), 43 deletions(-)
>...
>> -void edit_by_patterns_cmd()
>> +void pretty_print_menus(struct string_list *menu_list)
>> +{
>> +       struct strbuf buf = STRBUF_INIT;
> unused buf should be deleted.
>
>> +       unsigned int local_colopts = 0;
>> +       struct column_options copts;
>> +
>> +       /*
>> +        * always enable column display, we only consult column.*
>> +        * about layout strategy and stuff
>> +        */
> remove the above comments.
>
>> +       local_colopts = COL_ENABLED | COL_ROW;
>> +       memset(&copts, 0, sizeof(copts));
>> +       copts.indent = "  ";
>> +       copts.padding = 2;
>> +       print_columns(menu_list, local_colopts, &copts);
>> +       strbuf_release(&buf);
> remove strbuf_release of unused variable : buf.
>
>> +}
>...
>> +               } else if (MENU_STUFF_TYPE_STRING_LIST == stuff->type) {
>> +                       struct string_list_item *item;
>> +                       struct strbuf buf = STRBUF_INIT;
> should call strbuf_release later
> ...
>> +       } else {
>> +               result = xmalloc(sizeof(int) * (nr + 1));
>> +               memset(result, 0, sizeof(int) * (nr + 1));
>
> Add initial for j here:


What is this message trying to achieve?  "self review"???

A bit puzzled....

Re: [PATCH v6 4/7] git-clean: use a git-add-interactive compatible UI

[Jiang Xin]

2013/5/7 Junio C Hamano <gitster@pobox.com>:
> What is this message trying to achieve?  "self review"???
>
> A bit puzzled....

Maybe I should send a new rerolled patch series after this. Yesterday I
wanted to wait for a while to see suggestions and reviews from others.

-- 
Jiang Xin