[PATCH v7 00/10] interactive git clean

[PATCH v7 00/10] interactive git clean

[Jiang Xin]

Significant updates since patch v6 series:

 * Refactor on patch 4/10: split `list_and_choose` into 3 functions,
   to make it easy to read.

 * Mark no public functions as static for patch 1-10.

 * If set 'pager.clean' to true (i.e. isatty(1) is false), die
   instead of do cleaning.

 * New action: flags. The user can update flags for git-clean,
   such as -x/-d/-X/-ff.

 * Alway show interactive menu, even there are no files to clean.
   Because the use can update flags for git-clean.
 
Usage:

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: flags: none         6: quit                7: help
    What now> 2

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

The main command loop has 7 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.

flags::

  This lets you change the flags for git-clean, such as -x/-X/-d/-ff,
  and refresh the cleaning candidates list automatically.

quit::

  This lets you quit without do cleaning.

help::

  Show brief usage of interactive git-clean.


Jiang Xin (10):
  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
  git-clean refactor: save some options in clean_flags
  git-clean refactor: wrap in scan_clean_candidates
  git-clean: change clean flags in interactive mode

 Documentation/config.txt    |   4 +
 Documentation/git-clean.txt |  76 +++-
 builtin/clean.c             | 972 ++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 981 insertions(+), 71 deletions(-)

-- 
1.8.3.rc1.341.g1c24ab7

[PATCH v7 01/10] 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             | 198 +++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 192 insertions(+), 21 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..49aab 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;
 }
 
+static 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);
+}
+
+static 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,41 @@ 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);
-				}
-			}
-			strbuf_reset(&directory);
+			if (remove_directories || (matches == MATCHED_EXACTLY))
+				string_list_append(&del_list, ent->name);
 		} else {
 			if (pathspec && !matches)
 				continue;
-			res = dry_run ? 0 : unlink(ent->name);
+			string_list_append(&del_list, ent->name);
+		}
+	}
+
+	if (interactive && !dry_run && del_list.nr > 0)
+		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 +444,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.341.g1c24ab7

[PATCH v7 02/10] 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 49aab..38ed0 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;
 }
 
+static 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);
+}
+
 static void edit_by_patterns_cmd()
 {
 	struct dir_struct dir;
@@ -153,7 +187,6 @@ static 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 @@ static 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 @@ static void edit_by_patterns_cmd()
 static 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 @@ static void interactive_main_loop()
 		}
 	}
 
-	strbuf_release(&buf);
 	strbuf_release(&confirm);
 }
 
-- 
1.8.3.rc1.341.g1c24ab7

[PATCH v7 03/10] 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 38ed0..9b029 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 @@ static 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 @@ static 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 @@ static 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 @@ static 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.341.g1c24ab7

[PATCH v7 04/10] 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 | 469 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 428 insertions(+), 41 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 9b029..10f3 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -51,6 +51,36 @@ enum color_clean {
 	CLEAN_COLOR_ERROR = 5,
 };
 
+#define MENU_OPTS_SINGLETON		01
+#define MENU_OPTS_IMMEDIATE		02
+#define MENU_OPTS_LIST_ONLY		04
+
+struct menu_opts {
+	const char *header;
+	const char *prompt;
+	int flag;
+};
+
+#define MENU_RETURN_NO_LOOP		10
+
+struct menu_item {
+	char hotkey;
+	char *title;
+	int selected;
+	int (*fn)();
+};
+
+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;
+};
+
 static int parse_clean_color_slot(const char *var, int ofs)
 {
 	if (!strcasecmp(var+ofs, "reset"))
@@ -240,12 +270,345 @@ static void pretty_print_dels()
 	copts.indent = "  ";
 	copts.padding = 2;
 	print_columns(&list, colopts, &copts);
-	putchar('\n');
 	strbuf_release(&buf);
 	string_list_clear(&list, 0);
 }
 
-static void edit_by_patterns_cmd()
+static void pretty_print_menus(struct string_list *menu_list)
+{
+	unsigned int local_colopts = 0;
+	struct column_options copts;
+
+	local_colopts = COL_ENABLED | COL_ROW;
+	memset(&copts, 0, sizeof(copts));
+	copts.indent = "  ";
+	copts.padding = 2;
+	print_columns(menu_list, local_colopts, &copts);
+}
+
+static 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);
+}
+
+/*
+ * display menu stuff with number prefix and hotkey highlight
+ */
+static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen)
+{
+	static struct string_list menu_list = STRING_LIST_INIT_DUP;
+	struct strbuf menu = STRBUF_INIT;
+	int i;
+
+	/* 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++) {
+			char *p;
+			int highlighted = 0;
+
+			p = item->title;
+			if ((*chosen)[i] < 0)
+				(*chosen)[i] = item->selected ? 1 : 0;
+			strbuf_addf(&menu, "%s%2d: ", (*chosen)[i] ? "*" : " ", i+1);
+			for (; *p; p++) {
+				if (!highlighted && *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));
+					highlighted = 1;
+				} 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;
+
+			if ((*chosen)[i] < 0)
+				(*chosen)[i] = 0;
+			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);
+
+	strbuf_release(&menu);
+	string_list_clear(&menu_list, 0);
+}
+
+/*
+ * Parse user input, and return choice(s) for menu (menu_stuff).
+ *
+ * Input
+ *     (for single choice)
+ *         1          - select a numbered item
+ *         foo        - select item based on menu title
+ *                    - (empty) select nothing
+ *
+ *     (for multiple choice)
+ *         1          - select a single item
+ *         3-5        - select a range of items
+ *         2-3,6-9    - select multiple ranges
+ *         foo        - select item based on menu title
+ *         -...       - unselect specified items
+ *         *          - choose all items
+ *                    - (empty) finish selecting
+ *
+ * The parse result will be saved in array **chosen, and
+ * return number of total selections.
+ */
+static int parse_choice(struct menu_stuff *menu_stuff,
+			int is_single,
+			struct strbuf input,
+			int **chosen)
+{
+	struct strbuf **choice_list, **choice_p;
+	int nr = 0;
+	int i;
+
+	if (is_single) {
+		choice_list = strbuf_split_max(&input, '\n', 0);
+	} else {
+		char *p = input.buf;
+		do {
+			if (*p == ',')
+				*p = ' ';
+		} while (*p++);
+		choice_list = strbuf_split_max(&input, ' ', 0);
+	}
+
+	for (choice_p = choice_list; *choice_p; choice_p++) {
+		char *p;
+		int choose = 1;
+		int bottom = 0, top = 0;
+		int is_range, is_number;
+
+		strbuf_trim(*choice_p);
+		if (!(*choice_p)->len)
+			continue;
+
+		/* Input that begins with '-'; unchoose */
+		if (*(*choice_p)->buf == '-') {
+			choose = 0;
+			strbuf_remove((*choice_p), 0, 1);
+		}
+
+		is_range = 0;
+		is_number = 1;
+		for(p = (*choice_p)->buf; *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_p)->buf);
+			top = bottom;
+		} else if (is_range) {
+			bottom = atoi((*choice_p)->buf);
+			if (!*(strchr((*choice_p)->buf, '-') + 1)) {
+				top = menu_stuff->nr - 1;
+			} else {
+				top = atoi(strchr((*choice_p)->buf, '-') + 1);
+			}
+		} else if (!strcmp((*choice_p)->buf, "*")) {
+			bottom = 1;
+			top = menu_stuff->nr;
+		} else {
+			if (MENU_STUFF_TYPE_MENU_ITEM == menu_stuff->type) {
+				struct menu_item *item;
+
+				item = (struct menu_item *)menu_stuff->stuff;
+				for (i = 0; i < menu_stuff->nr; i++, item++) {
+					if (((*choice_p)->len == 1 &&
+					     *(*choice_p)->buf == item->hotkey) ||
+					    !strcasecmp((*choice_p)->buf, item->title)) {
+						bottom = i + 1;
+						top = bottom;
+						break;
+					}
+				}
+			} else if (MENU_STUFF_TYPE_STRING_LIST == menu_stuff->type) {
+				struct string_list_item *item;
+
+				item = ((struct string_list *)menu_stuff->stuff)->items;
+				for (i = 0; i < menu_stuff->nr; i++, item++) {
+					if (!strcasecmp((*choice_p)->buf, item->string)) {
+						bottom = i + 1;
+						top = bottom;
+						break;
+					}
+				}
+			}
+		}
+
+		if (top <= 0 || bottom <= 0 || top > menu_stuff->nr || bottom > top ||
+		    (is_single && bottom != top)) {
+			clean_print_color(CLEAN_COLOR_ERROR);
+			printf_ln(_("Huh (%s)?"), (*choice_p)->buf);
+			clean_print_color(CLEAN_COLOR_RESET);
+			continue;
+		}
+
+		/* A range can be specified like 5-7 or 5-. */
+		for (i = bottom; i <= top; i++)
+			(*chosen)[i-1] = choose;
+	}
+
+	strbuf_list_free(choice_list);
+
+	for (i = 0; i < menu_stuff->nr; i++)
+		nr += (*chosen)[i];
+	return nr;
+}
+
+/*
+ * 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.
+ */
+static int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff)
+{
+	struct strbuf choice = STRBUF_INIT;
+	int *chosen, *result;
+	int nr = 0;
+	int eof = 0;
+	int i;
+
+	chosen = xmalloc(sizeof(int) * stuff->nr);
+	/* set chosen as uninitialized */
+	for (i = 0; i < stuff->nr; i++)
+		chosen[i] = -1;
+
+	while (1) {
+		if (opts->header) {
+			printf_ln("%s%s%s",
+				  clean_get_color(CLEAN_COLOR_HEADER),
+				  opts->header,
+				  clean_get_color(CLEAN_COLOR_RESET));
+		}
+
+		/* the chosen array can be initialized by menu_item.selected  */
+		print_highlight_menu_stuff(stuff, &chosen);
+
+		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) {
+			strbuf_trim(&choice);
+		} else {
+			eof = 1;
+			break;
+		}
+
+		/* help for prompt */
+		if (!strcmp(choice.buf, "?")) {
+			prompt_help_cmd(opts->flag & MENU_OPTS_SINGLETON);
+			continue;
+		}
+
+		/* for a multiple-choice menu, press ENTER (empty) will return back */
+		if (!(opts->flag & MENU_OPTS_SINGLETON) && !choice.len)
+			break;
+
+		nr = parse_choice(stuff,
+				  opts->flag & MENU_OPTS_SINGLETON,
+				  choice,
+				  &chosen);
+
+		if (opts->flag & MENU_OPTS_SINGLETON) {
+			if (nr)
+				break;
+		} else if (opts->flag & MENU_OPTS_IMMEDIATE) {
+			break;
+		}
+	}
+
+	if (eof) {
+		result = xmalloc(sizeof(int));
+		*result = EOF;
+	} else {
+		int j = 0;
+
+		/* recalculate nr, for a multiple-choice menu with initial selections */
+		if (!nr) {
+			for (i = 0; i < stuff->nr; i++)
+				nr += chosen[i];
+		}
+
+		result = xmalloc(sizeof(int) * (nr + 1));
+		memset(result, 0, sizeof(int) * (nr + 1));
+		for (i = 0; i < stuff->nr && j < nr; i++) {
+			if (chosen[i])
+				result[j++] = i;
+		}
+		result[j] = EOF;
+	}
+
+	free(chosen);
+	strbuf_release(&choice);
+	return result;
+}
+
+static int clean_cmd()
+{
+	return MENU_RETURN_NO_LOOP;
+}
+
+static int edit_by_patterns_cmd()
 {
 	struct dir_struct dir;
 	struct strbuf confirm = STRBUF_INIT;
@@ -257,16 +620,10 @@ static 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 +681,86 @@ static void edit_by_patterns_cmd()
 
 	strbuf_release(&buf);
 	strbuf_release(&confirm);
+	return 0;
 }
 
-static void interactive_main_loop()
+static int quit_cmd()
 {
-	struct strbuf confirm = STRBUF_INIT;
+	string_list_clear(&del_list, 0);
+	printf_ln(_("Bye."));
+	return MENU_RETURN_NO_LOOP;
+}
 
+static 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;
+}
+
+static 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",			0, clean_cmd},
+			{'p', "edit by patterns",	0, edit_by_patterns_cmd},
+			{'q', "quit",			0, quit_cmd},
+			{'h', "help",			0, 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)
-- 
1.8.3.rc1.341.g1c24ab7

[PATCH v7 05/10] 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 10f3..75e37 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -684,6 +684,43 @@ static int edit_by_patterns_cmd()
 	return 0;
 }
 
+static 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;
+}
+
 static int quit_cmd()
 {
 	string_list_clear(&del_list, 0);
@@ -697,6 +734,7 @@ static 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"
@@ -714,6 +752,7 @@ static void interactive_main_loop()
 		struct menu_item menus[] = {
 			{'c', "clean",			0, clean_cmd},
 			{'p', "edit by patterns",	0, edit_by_patterns_cmd},
+			{'n', "edit by numbers",	0, edit_by_numbers_cmd},
 			{'q', "quit",			0, quit_cmd},
 			{'h', "help",			0, help_cmd},
 		};
-- 
1.8.3.rc1.341.g1c24ab7

[PATCH v7 06/10] 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 75e37..5bb36 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -721,6 +721,40 @@ static int edit_by_numbers_cmd()
 	return 0;
 }
 
+static 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;
+}
+
 static int quit_cmd()
 {
 	string_list_clear(&del_list, 0);
@@ -735,6 +769,7 @@ static 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"
@@ -753,6 +788,7 @@ static void interactive_main_loop()
 			{'c', "clean",			0, clean_cmd},
 			{'p', "edit by patterns",	0, edit_by_patterns_cmd},
 			{'n', "edit by numbers",	0, edit_by_numbers_cmd},
+			{'i', "rm -i",			0, rm_i_cmd},
 			{'q', "quit",			0, quit_cmd},
 			{'h', "help",			0, help_cmd},
 		};
-- 
1.8.3.rc1.341.g1c24ab7

[PATCH v7 07/10] 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..47e8e 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.341.g1c24ab7

[PATCH v7 08/10] git-clean refactor: save some options in clean_flags

[Jiang Xin]

Save some options in variable clean_flags, such as -ff (force > 1),
-x (ignored), -X (ignored_only), and -d (remove_directories). We may
change clean_flags later in the interactive git-clean.

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

diff --git a/builtin/clean.c b/builtin/clean.c
index 5bb36..d46f3 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -16,7 +16,13 @@
 #include "column.h"
 #include "color.h"
 
+#define CLEAN_OPTS_SHOW_IGNORED		01
+#define CLEAN_OPTS_IGNORED_ONLY		02
+#define CLEAN_OPTS_REMOVE_DIRECTORIES	04
+#define CLEAN_OPTS_REMOVE_NESTED_GIT	010
+
 static int force = -1; /* unset */
+static int clean_flags = 0;
 static int interactive;
 static struct string_list del_list = STRING_LIST_INIT_DUP;
 static const char **the_prefix;
@@ -843,7 +849,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	int i, res;
 	int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
 	int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
-	int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
 	struct strbuf directory = STRBUF_INIT;
 	struct dir_struct dir;
 	static const char **pathspec;
@@ -879,13 +884,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, *the_prefix, options, builtin_clean_usage,
 			     0);
 
-	memset(&dir, 0, sizeof(dir));
-	if (ignored_only)
-		dir.flags |= DIR_SHOW_IGNORED;
-
-	if (ignored && ignored_only)
-		die(_("-x and -X cannot be used together"));
-
 	if (interactive) {
 		if (!isatty(0) || !isatty(1))
 			die(_("interactive clean can not run without a valid tty; "
@@ -899,15 +897,29 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 				  "refusing to clean"));
 	}
 
+	if (read_cache() < 0)
+		die(_("index file corrupt"));
+
 	if (force > 1)
-		rm_flags = 0;
+		clean_flags |= CLEAN_OPTS_REMOVE_NESTED_GIT;
+	if (ignored)
+		clean_flags |= CLEAN_OPTS_SHOW_IGNORED;
+	if (ignored_only)
+		clean_flags |= CLEAN_OPTS_IGNORED_ONLY;
+	if (remove_directories)
+		clean_flags |= CLEAN_OPTS_REMOVE_DIRECTORIES;
 
-	dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
+	memset(&dir, 0, sizeof(dir));
+	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY)
+		dir.flags |= DIR_SHOW_IGNORED;
 
-	if (read_cache() < 0)
-		die(_("index file corrupt"));
+	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY &&
+	    clean_flags & CLEAN_OPTS_SHOW_IGNORED)
+		die(_("-x and -X cannot be used together"));
+
+	dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
 
-	if (!ignored)
+	if (!(clean_flags & CLEAN_OPTS_SHOW_IGNORED))
 		setup_standard_excludes(&dir);
 
 	el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
@@ -961,7 +973,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 ((clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES) ||
+			    (matches == MATCHED_EXACTLY))
 				string_list_append(&del_list, ent->name);
 		} else {
 			if (pathspec && !matches)
@@ -981,7 +994,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 		if (S_ISDIR(st.st_mode)) {
 			strbuf_addstr(&directory, item->string);
-			if (remove_dirs(&directory, *the_prefix, rm_flags, dry_run, quiet, &gone))
+			if (remove_dirs(&directory, *the_prefix,
+					clean_flags & CLEAN_OPTS_REMOVE_NESTED_GIT ?
+						0 : REMOVE_DIR_KEEP_NESTED_GIT,
+					dry_run, quiet, &gone))
 				errors++;
 			if (gone && !quiet) {
 				qname = quote_path_relative(directory.buf, directory.len, &buf, *the_prefix);
-- 
1.8.3.rc1.341.g1c24ab7

[PATCH v7 09/10] git-clean refactor: wrap in scan_clean_candidates

[Jiang Xin]

Add new function `scan_clean_candidates`, which determines the del_list
(i.e. the cleaning candidates). This function will be reused later in
the interactive git-clean, so we can change flags of git-clean and
refresh the del_list.

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

diff --git a/builtin/clean.c b/builtin/clean.c
index d46f3..0d8561 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -254,6 +254,95 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 	return ret;
 }
 
+static void scan_clean_candidates(const char **pathspec, struct string_list exclude_list)
+{
+	struct dir_struct dir;
+	struct exclude_list *el;
+	char *seen = NULL;
+	const char **pathspec_p = pathspec;
+	int pathspec_nr = 0;
+	int i;
+
+	while (pathspec_p && *(pathspec_p++))
+		pathspec_nr++;
+
+	memset(&dir, 0, sizeof(dir));
+	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY)
+		dir.flags |= DIR_SHOW_IGNORED;
+
+	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY &&
+	    clean_flags & CLEAN_OPTS_SHOW_IGNORED)
+		die(_("-x and -X cannot be used together"));
+
+	dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
+
+	if (!(clean_flags & CLEAN_OPTS_SHOW_IGNORED))
+		setup_standard_excludes(&dir);
+
+	el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
+	for (i = 0; i < exclude_list.nr; i++)
+		add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
+
+	fill_directory(&dir, pathspec);
+
+	if (pathspec)
+		seen = xmalloc(pathspec_nr > 0 ? pathspec_nr : 1);
+
+	string_list_clear(&del_list, 0);
+
+	for (i = 0; i < dir.nr; i++) {
+		struct dir_entry *ent = dir.entries[i];
+		int len, pos;
+		int matches = 0;
+		struct cache_entry *ce;
+		struct stat st;
+
+		/*
+		 * Remove the '/' at the end that directory
+		 * walking adds for directory entries.
+		 */
+		len = ent->len;
+		if (len && ent->name[len-1] == '/')
+			len--;
+		pos = cache_name_pos(ent->name, len);
+		if (0 <= pos)
+			continue;	/* exact match */
+		pos = -pos - 1;
+		if (pos < active_nr) {
+			ce = active_cache[pos];
+			if (ce_namelen(ce) == len &&
+			    !memcmp(ce->name, ent->name, len))
+				continue; /* Yup, this one exists unmerged */
+		}
+
+		/*
+		 * we might have removed this as part of earlier
+		 * recursive directory removal, so lstat() here could
+		 * fail with ENOENT.
+		 */
+		if (lstat(ent->name, &st))
+			continue;
+
+		if (pathspec) {
+			memset(seen, 0, pathspec_nr > 0 ? pathspec_nr : 1);
+			matches = match_pathspec(pathspec, ent->name, len,
+						 0, seen);
+		}
+
+		if (S_ISDIR(st.st_mode)) {
+			if ((clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES) ||
+			    (matches == MATCHED_EXACTLY))
+				string_list_append(&del_list, ent->name);
+		} else {
+			if (pathspec && !matches)
+				continue;
+			string_list_append(&del_list, ent->name);
+		}
+	}
+
+	free(seen);
+}
+
 static void pretty_print_dels()
 {
 	struct string_list list = STRING_LIST_INIT_DUP;
@@ -846,18 +935,15 @@ static void interactive_main_loop()
 
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
-	int i, res;
+	int res;
 	int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
 	int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
 	struct strbuf directory = STRBUF_INIT;
-	struct dir_struct dir;
 	static const char **pathspec;
 	struct strbuf buf = STRBUF_INIT;
-	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
-	struct exclude_list *el;
 	struct string_list_item *item;
+	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 	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")),
@@ -909,79 +995,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	if (remove_directories)
 		clean_flags |= CLEAN_OPTS_REMOVE_DIRECTORIES;
 
-	memset(&dir, 0, sizeof(dir));
-	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY)
-		dir.flags |= DIR_SHOW_IGNORED;
-
-	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY &&
-	    clean_flags & CLEAN_OPTS_SHOW_IGNORED)
-		die(_("-x and -X cannot be used together"));
-
-	dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
-
-	if (!(clean_flags & CLEAN_OPTS_SHOW_IGNORED))
-		setup_standard_excludes(&dir);
-
-	el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
-	for (i = 0; i < exclude_list.nr; i++)
-		add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
-
 	pathspec = get_pathspec(*the_prefix, argv);
 
-	fill_directory(&dir, pathspec);
-
-	if (pathspec)
-		seen = xmalloc(argc > 0 ? argc : 1);
-
-	for (i = 0; i < dir.nr; i++) {
-		struct dir_entry *ent = dir.entries[i];
-		int len, pos;
-		int matches = 0;
-		struct cache_entry *ce;
-		struct stat st;
-
-		/*
-		 * Remove the '/' at the end that directory
-		 * walking adds for directory entries.
-		 */
-		len = ent->len;
-		if (len && ent->name[len-1] == '/')
-			len--;
-		pos = cache_name_pos(ent->name, len);
-		if (0 <= pos)
-			continue;	/* exact match */
-		pos = -pos - 1;
-		if (pos < active_nr) {
-			ce = active_cache[pos];
-			if (ce_namelen(ce) == len &&
-			    !memcmp(ce->name, ent->name, len))
-				continue; /* Yup, this one exists unmerged */
-		}
-
-		/*
-		 * we might have removed this as part of earlier
-		 * recursive directory removal, so lstat() here could
-		 * fail with ENOENT.
-		 */
-		if (lstat(ent->name, &st))
-			continue;
-
-		if (pathspec) {
-			memset(seen, 0, argc > 0 ? argc : 1);
-			matches = match_pathspec(pathspec, ent->name, len,
-						 0, seen);
-		}
-
-		if (S_ISDIR(st.st_mode)) {
-			if ((clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES) ||
-			    (matches == MATCHED_EXACTLY))
-				string_list_append(&del_list, ent->name);
-		} else {
-			if (pathspec && !matches)
-				continue;
-			string_list_append(&del_list, ent->name);
-		}
-	}
+	scan_clean_candidates(pathspec, exclude_list);
 
 	if (interactive && !dry_run && del_list.nr > 0)
 		interactive_main_loop();
@@ -1016,7 +1032,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 			}
 		}
 	}
-	free(seen);
 
 	strbuf_release(&directory);
 	string_list_clear(&exclude_list, 0);
-- 
1.8.3.rc1.341.g1c24ab7

[PATCH v7 10/10] git-clean: change clean flags in interactive mode

[Jiang Xin]

Add new action in the interactive mode, so that the user can change
git-clean flags, such as -x/-X/-d/-ff, and refresh the cleaning
candidates list.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 Documentation/git-clean.txt |  11 +++--
 builtin/clean.c             | 117 +++++++++++++++++++++++++++++++++++++-------
 2 files changed, 108 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 47e8e..c7885 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -82,14 +82,14 @@ and type return, like this:
 
 ------------
     *** Commands ***
-      1: clean             2: edit by patterns   3: edit by numbers
-      4. rm -i             5. quit               6. help
+      1: clean               2: edit by patterns    3: edit by numbers     4: rm -i
+      5: flags: none         6: quit                7: 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.
+The main command loop has 7 subcommands.
 
 clean::
 
@@ -122,6 +122,11 @@ rm -i::
   by one in order to delete items. This action is not as efficient
   as the above two actions.
 
+flags::
+
+  This lets you change the flags for git-clean, such as -x/-X/-d/-ff,
+  and refresh the cleaning candidates list automatically.
+
 quit::
 
   This lets you quit without do cleaning.
diff --git a/builtin/clean.c b/builtin/clean.c
index 0d8561..b2941 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -850,6 +850,66 @@ static int rm_i_cmd()
 	return MENU_RETURN_NO_LOOP;
 }
 
+static int flags_cmd()
+{
+	struct menu_opts menu_opts;
+	struct menu_stuff menu_stuff;
+	struct menu_item menus[] = {
+		{'d', "(-d) remove directories",
+			clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES,	NULL},
+		{'x', "(-x) show ignored",
+			clean_flags & CLEAN_OPTS_SHOW_IGNORED,		NULL},
+		{'X', "(-X) ignored only",
+			clean_flags & CLEAN_OPTS_IGNORED_ONLY,		NULL},
+		{'f', "(-ff) remove nested.git",
+			clean_flags & CLEAN_OPTS_REMOVE_NESTED_GIT,	NULL},
+	};
+	int new_flags = 0;
+	int *chosen;
+	int i;
+
+	menu_opts.header = NULL;
+	menu_opts.prompt = "Change flags";
+	menu_opts.flag = 0;
+
+	menu_stuff.type = MENU_STUFF_TYPE_MENU_ITEM;
+	menu_stuff.stuff = menus;
+	menu_stuff.nr = sizeof(menus) / sizeof(struct menu_item);
+
+	chosen = list_and_choose(&menu_opts, &menu_stuff);
+
+	for (i = 0; chosen[i] != EOF; i++) {
+		switch (chosen[i]) {
+		case 0:
+			new_flags |= CLEAN_OPTS_REMOVE_DIRECTORIES;
+			break;
+		case 1:
+			new_flags |= CLEAN_OPTS_SHOW_IGNORED;
+			break;
+		case 2:
+			new_flags |= CLEAN_OPTS_IGNORED_ONLY;
+			break;
+		case 3:
+			new_flags |= CLEAN_OPTS_REMOVE_NESTED_GIT;
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (new_flags & CLEAN_OPTS_IGNORED_ONLY &&
+	    new_flags & CLEAN_OPTS_SHOW_IGNORED) {
+		clean_print_color(CLEAN_COLOR_ERROR);
+		printf_ln(_("-x and -X cannot be used together"));
+		clean_print_color(CLEAN_COLOR_RESET);
+	} else {
+		clean_flags = new_flags;
+	}
+
+	free(chosen);
+	return 0;
+}
+
 static int quit_cmd()
 {
 	string_list_clear(&del_list, 0);
@@ -865,6 +925,7 @@ static int help_cmd(int x)
 		    "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"
+		    "flags            - change git-clean flags and update the cleaning candidates\n"
 		    "quit             - stop cleaning\n"
 		    "help             - this screen\n"
 		    "?                - help for prompt selection"
@@ -873,10 +934,13 @@ static int help_cmd(int x)
 	return 0;
 }
 
-static void interactive_main_loop()
+static void interactive_main_loop(const char **pathspec, struct string_list exclude_list)
 {
+	int cached_clean_flags = clean_flags;
+	char flags_title[20];
+
 	/* dels list may become empty after return back from edit mode */
-	while (del_list.nr) {
+	while (1) {
 		struct menu_opts menu_opts;
 		struct menu_stuff menu_stuff;
 		struct menu_item menus[] = {
@@ -884,11 +948,24 @@ static void interactive_main_loop()
 			{'p', "edit by patterns",	0, edit_by_patterns_cmd},
 			{'n', "edit by numbers",	0, edit_by_numbers_cmd},
 			{'i', "rm -i",			0, rm_i_cmd},
+			{'f', flags_title,		0, flags_cmd},
 			{'q', "quit",			0, quit_cmd},
 			{'h', "help",			0, help_cmd},
 		};
 		int *chosen;
 
+		if (!clean_flags) {
+			strncpy(flags_title, "flags: none", sizeof(flags_title)/sizeof(char));
+		} else {
+			snprintf(flags_title, sizeof(flags_title)/sizeof(char),
+				 "flags: -%s%s%s%s",
+				 clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES ? "d" : "",
+				 clean_flags & CLEAN_OPTS_SHOW_IGNORED ? "x" : "",
+				 clean_flags & CLEAN_OPTS_IGNORED_ONLY ? "X" : "",
+				 clean_flags & CLEAN_OPTS_REMOVE_NESTED_GIT ? "ff" : ""
+				);
+		}
+
 		menu_opts.header = _("*** Commands ***");
 		menu_opts.prompt = "What now";
 		menu_opts.flag = MENU_OPTS_SINGLETON;
@@ -897,14 +974,26 @@ static void interactive_main_loop()
 		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);
+		if (cached_clean_flags != clean_flags) {
+			scan_clean_candidates(pathspec, exclude_list);
+			cached_clean_flags = clean_flags;
+		}
 
-		/* display dels in columns */
-		pretty_print_dels();
+		if (del_list.nr) {
+			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);
+
+			/* display dels in columns */
+			pretty_print_dels();
+		} else {
+			clean_print_color(CLEAN_COLOR_HEADER);
+			printf_ln(_("NOTE: No more files to clean. Press \"f\" can change the flags."));
+			putchar('\n');
+			clean_print_color(CLEAN_COLOR_RESET);
+		}
 
 		/* main menu */
 		chosen = list_and_choose(&menu_opts, &menu_stuff);
@@ -915,12 +1004,6 @@ static void interactive_main_loop()
 			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 {
@@ -999,8 +1082,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	scan_clean_candidates(pathspec, exclude_list);
 
-	if (interactive && !dry_run && del_list.nr > 0)
-		interactive_main_loop();
+	if (interactive && !dry_run)
+		interactive_main_loop(pathspec, exclude_list);
 
 	for_each_string_list_item(item, &del_list) {
 		struct stat st;
-- 
1.8.3.rc1.341.g1c24ab7

Re: [PATCH v7 04/10] git-clean: use a git-add-interactive compatible UI

[Junio C Hamano]

Nice (I didn't read the code, though).

Re: [PATCH v7 03/10] Add colors to interactive git-clean

[Matthieu Moy]

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

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

This should go to the documentation (a short summary is welcome in the
commit messages in addition, but users won't read this...)

> +	if (!prefixcmp(var, "color.interactive.")) {
> +		int slot = parse_clean_color_slot(var, 18);

For readability and maintainability: please use
strlen("color.interactive."), not 18.

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

Re: [PATCH v7 03/10] Add colors to interactive git-clean

[Jiang Xin]

2013/5/13 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Jiang Xin <worldhello.net@gmail.com> writes:
>
>>  * color.interactive.<slot>: Use customized color for interactive
>>    git-clean output (like git add --interactive). <slot> may be
>>    prompt, header, help or error.
>
> This should go to the documentation (a short summary is welcome in the
> commit messages in addition, but users won't read this...)
>
>> +     if (!prefixcmp(var, "color.interactive.")) {
>> +             int slot = parse_clean_color_slot(var, 18);
>
> For readability and maintainability: please use
> strlen("color.interactive."), not 18.

Feel like conventional:

    git grep -C2 prefixcmp builtin/apply.c builtin/archive.c
builtin/branch.c builtin/checkout.c

But maybe 18 characters are too long. ;-)


-- 
Jiang Xin
http://www.worldhello.net/

Re: [PATCH v7 03/10] Add colors to interactive git-clean

[Junio C Hamano]

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

> 2013/5/13 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
>> Jiang Xin <worldhello.net@gmail.com> writes:
>>
>>>  * color.interactive.<slot>: Use customized color for interactive
>>>    git-clean output (like git add --interactive). <slot> may be
>>>    prompt, header, help or error.
>>
>> This should go to the documentation (a short summary is welcome in the
>> commit messages in addition, but users won't read this...)
>>
>>> +     if (!prefixcmp(var, "color.interactive.")) {
>>> +             int slot = parse_clean_color_slot(var, 18);
>>
>> For readability and maintainability: please use
>> strlen("color.interactive."), not 18.
>
> Feel like conventional:
>
>     git grep -C2 prefixcmp builtin/apply.c builtin/archive.c
> builtin/branch.c builtin/checkout.c
>
> But maybe 18 characters are too long. ;-)

Why does it even have to know where the prefix ends or how long the
prefix is?

Doesn't it suggest that perhaps the parse_clean_color_slot()'s
external interface is misdesigned?  In the other examples you
showed, e.g.

    builtin/apply.c:	if (!prefixcmp(buffer, "delta ")) {
    builtin/apply.c-		patch_method = BINARY_DELTA_DEFLATED;
    builtin/apply.c-		origlen = strtoul(buffer + 6, NULL, 10);

we are calling external libraries that are designed to take a
pointer to a character, but the parse-clean-color-slot knows that it
is fed the name of a configuration variable and it knows the shape
of its input far better than a generic function like strtoul(), no?

Re: [PATCH v7 03/10] Add colors to interactive git-clean

[Matthieu Moy]

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

> 2013/5/13 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
>> Jiang Xin <worldhello.net@gmail.com> writes:
>>
>>>  * color.interactive.<slot>: Use customized color for interactive
>>>    git-clean output (like git add --interactive). <slot> may be
>>>    prompt, header, help or error.
>>
>> This should go to the documentation (a short summary is welcome in the
>> commit messages in addition, but users won't read this...)
>>
>>> +     if (!prefixcmp(var, "color.interactive.")) {
>>> +             int slot = parse_clean_color_slot(var, 18);
>>
>> For readability and maintainability: please use
>> strlen("color.interactive."), not 18.
>
> Feel like conventional:
>
>     git grep -C2 prefixcmp builtin/apply.c builtin/archive.c
> builtin/branch.c builtin/checkout.c

I know there are already many instances of this in the code, but I don't
think it's a good idea to add even more.

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

Re: [PATCH v7 02/10] Show items of interactive git-clean in columns

[Junio C Hamano]

Nice (I didn't read the code, though).

Re: [PATCH v7 02/10] Show items of interactive git-clean in columns

[Matthieu Moy]

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

> +column.clean::
> +	Specify whether to output cleaning files in `git clean -i` in columns.
> +	See `column.ui` for details.
> +


> +static void pretty_print_dels()

Ah, OK. That's the helper function I was expecting in 01/10. The patches
would be easier to review if the helper was introduced in 01/10 and
then modified here to add column display, but the result is OK.

> +	/*
> +	 * always enable column display, we only consult column.*
> +	 * about layout strategy and stuff
> +	 */

Isn't this conflicting with the documentation above?

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

Re: [PATCH v7 01/10] Add support for -i/--interactive to git-clean

[Junio C Hamano]

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

> 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>

Listing everybody who has ever said anything in the review thread?
I can understand that you may want to give credit to those who
significantly helped, but please do not overdo it.

In any case, with the help of their inputs, you brought the patch
into its final shape.  Please sign-off at the _end_.

> ---
>  Documentation/git-clean.txt |  15 +++-
>  builtin/clean.c             | 198 +++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 192 insertions(+), 21 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..49aab 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;

Ehh, why?

>  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;
>  }
>  
> +static void edit_by_patterns_cmd()

static void edit_by_patterns_cmd(void)

> +{
> +	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 */

An unnecessary and overlong comment.  The message shown already
tells the reader what is going on anyway, no?

> +		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;

Why break here?  If we got nothing, wouldn't confirm.len be zero?
If we did get something but the input got flushed without line-end,
sending '\n' to the terminal may be justified, but in that case you
would may have something useful, and asking confirm.len if it is
empty would be the consistent way to check between two cases, no?

> +		}
> +
> +		/* 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++;
> +			}

A few points:

 * Pass prefix as a parameter to this function, just like how
   remove_dirs() gets called, and get rid of the_prefix.

 * The result of quote_* is designed to avoid ambiguities, by
   applying C-style quotes like HT => \t and adding "" pair around
   it as necessary.  I doubt feeding it to is_excluded() makes any
   sense.  You probably meant path_relative(), but I am not sure.

> +		}
> +
> +		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);
> +}
> +
> +static 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);

None of these changes s/prefix/*the_prefix/ looks remotely
justifiable.  Does anybody change *the_prefix after it is set and if
so how?

>  	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,41 @@ 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);
> -				}
> -			}
> -			strbuf_reset(&directory);
> +			if (remove_directories || (matches == MATCHED_EXACTLY))
> +				string_list_append(&del_list, ent->name);
>  		} else {
>  			if (pathspec && !matches)
>  				continue;
> -			res = dry_run ? 0 : unlink(ent->name);
> +			string_list_append(&del_list, ent->name);
> +		}
> +	}
> +
> +	if (interactive && !dry_run && del_list.nr > 0)
> +		interactive_main_loop();
> +
> +	for_each_string_list_item(item, &del_list) {
> +		struct stat st;
> +
> +		if (lstat(item->string, &st))
> +			continue;

Ignoring errors silently?

With the "interactive" stuff, can you get into a situation where you
originally propose to remove D and D/F but the user tells you to
remove D (editing D/F away), or vice versa?

I think this patch should be in at least two parts:

 - Introduce the two-phase "collect in del_list, remove in a
   separate loop at the end" restructuring.

 - (optional, if you are feeling ambitious) Change the path that is
   stored in del_list relative to the prefix, so that all functions
   that operate on the string in the del_list do not have to do
   *_relative() thing.  Some functions may instead have to prepend
   prefix but if they are minority compared to the users of
   *_relative(), it may be an overall win from the readability's
   point of view.

 - Add the "interactively allow you to reduce the del_list" bit
   between the two phases.

> +		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 +444,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);
>  }

Re: [PATCH v7 01/10] Add support for -i/--interactive to git-clean

[Jiang Xin]

2013/5/9 Junio C Hamano <gitster@pobox.com>:
>> @@ -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;
>
> Ehh, why?

Next reroll will save relative paths in del_list, and eliminate "**the_prefix".


>> +
>> +             printf(_("Input ignore patterns>> "));
>> +             if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
>> +                     strbuf_trim(&confirm);
>> +             } else {
>> +                     putchar('\n');
>> +                     break;
>
> Why break here?  If we got nothing, wouldn't confirm.len be zero?
> If we did get something but the input got flushed without line-end,
> sending '\n' to the terminal may be justified, but in that case you
> would may have something useful, and asking confirm.len if it is
> empty would be the consistent way to check between two cases, no?

Yes, this break is unnecessary, it left from pervious revision.


>
> A few points:
>
>  * Pass prefix as a parameter to this function, just like how
>    remove_dirs() gets called, and get rid of the_prefix.
>
>  * The result of quote_* is designed to avoid ambiguities, by
>    applying C-style quotes like HT => \t and adding "" pair around
>    it as necessary.  I doubt feeding it to is_excluded() makes any
>    sense.  You probably meant path_relative(), but I am not sure.

Appreciated, that is what I need. I write a local version of path_relative,
a combination of path_relative (in quote.c) and relative_path (in path.c),
like this:

        static const char *path_relative(const char *in, const char *prefix)

>> +     for_each_string_list_item(item, &del_list) {
>> +             struct stat st;
>> +
>> +             if (lstat(item->string, &st))
>> +                     continue;
>
> Ignoring errors silently?
>
> With the "interactive" stuff, can you get into a situation where you
> originally propose to remove D and D/F but the user tells you to
> remove D (editing D/F away), or vice versa?

I can not find out such a case, that remove parent directory D,
while left file in it, such as D/F.

> I think this patch should be in at least two parts:
>
>  - Introduce the two-phase "collect in del_list, remove in a
>    separate loop at the end" restructuring.
>
>  - (optional, if you are feeling ambitious) Change the path that is
>    stored in del_list relative to the prefix, so that all functions
>    that operate on the string in the del_list do not have to do
>    *_relative() thing.  Some functions may instead have to prepend
>    prefix but if they are minority compared to the users of
>    *_relative(), it may be an overall win from the readability's
>    point of view.
>
>  - Add the "interactively allow you to reduce the del_list" bit
>    between the two phases.
>

Will send new reroll soon.


-- 
Jiang Xin

[PATCH v8 00/12] Interactive git clean

[Jiang Xin]

Updates since v7 series:

 * Eliminate global variable "**the_prefix".
 * Save relative paths in del_list.
 * Split 1/10 of v7 into 3 patches for readability.
 * Change orders of patches, thanks to Eric.
 * Update menu and hotkeys with the help of Eric.

Usage:

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: filter by pattern     3: select by numbers
      4. ask each      5. toggle flags: none    6. quit
      7: help
    What now> 2

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

The main command loop has 7 subcommands.

clean::

   Start cleaning files and directories, and then quit.

filter by pattern::

   This shows the files and directories to be deleted and issues an
   "Input ignore patterns>>" prompt. You can input 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.

select 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.

ask each::

  This will start to clean, and you must confirm one by one in order
  to delete items. Please note that this action is not as efficient
  as the above two actions.

toggle flags::

  This lets you change the flags for git-clean, such as -x/-X/-d/-ff,
  and refresh the cleaning candidates list automatically.

quit::

  This lets you quit without do cleaning.

help::

  Show brief usage of interactive git-clean.



Jiang Xin (12):
  git-clean refactor: hold cleaning items in del_list
  git-clean: add support for -i/--interactive
  git-clean: show items of del_list in columns
  git-clean: add colors to interactive git-clean
  git-clean: use a git-add-interactive compatible UI
  git-clean: add filter by pattern interactive action
  git-clean: add select by numbers interactive action
  git-clean: add ask each interactive action
  git-clean refactor: save some options in clean_flags
  git-clean refactor: add wrapper scan_clean_candidates
  git-clean: add toggle flags interactive action
  git-clean: update document for interactive git-clean

 Documentation/config.txt    |    4 +
 Documentation/git-clean.txt |   77 +++-
 builtin/clean.c             | 1019 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 1023 insertions(+), 77 deletions(-)

-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 01/12] git-clean refactor: hold cleaning items in del_list

[Jiang Xin]

Refactor git-clean operations into two phases:

 * collect cleaning candidates in del_list,
 * and remove them in a separate loop at the end.

We will introduce an interactive git-clean between the two phases.
The interactive git-clean will show what would be done and confirm
before do real cleaning.

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

diff --git a/builtin/clean.c b/builtin/clean.c
index 04e39..ccd4 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -15,6 +15,7 @@
 #include "quote.h"
 
 static int force = -1; /* unset */
+static struct string_list del_list = STRING_LIST_INIT_DUP;
 
 static const char *const builtin_clean_usage[] = {
 	N_("git clean [-d] [-f] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>..."),
@@ -142,18 +143,61 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 	return ret;
 }
 
+/*
+ * Give path as relative to prefix.
+ *
+ * This function is a combination of path_relative (in quote.c) and
+ * relative_path (in path.c)
+ */
+static const char *path_relative(const char *in, const char *prefix)
+{
+	static char buf[PATH_MAX + 1];
+	int off, i;
+	int len, prefix_len;
+
+	len = strlen(in);
+	if (prefix)
+		prefix_len = strlen(prefix);
+	else
+		prefix_len = 0;
+
+	off = 0;
+	i = 0;
+	while (i < prefix_len && i < len && prefix[i] == in[i]) {
+		if (prefix[i] == '/')
+			off = i + 1;
+		i++;
+	}
+	in += off;
+	len -= off;
+
+	if (i >= prefix_len)
+		return in;
+
+	buf[0] = '\0';
+	while (i < prefix_len) {
+		if (prefix[i] == '/')
+			strcat(buf, "../");
+		i++;
+	}
+	strcat(buf, in);
+
+	return buf;
+}
+
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
 	int i, res;
 	int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
 	int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
 	int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
-	struct strbuf directory = STRBUF_INIT;
+	struct strbuf abs_path = STRBUF_INIT;
 	struct dir_struct dir;
 	static const char **pathspec;
 	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[] = {
@@ -223,6 +267,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		int matches = 0;
 		struct cache_entry *ce;
 		struct stat st;
+		const char *rel;
 
 		/*
 		 * Remove the '/' at the end that directory
@@ -242,11 +287,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 				continue; /* Yup, this one exists unmerged */
 		}
 
-		/*
-		 * we might have removed this as part of earlier
-		 * recursive directory removal, so lstat() here could
-		 * fail with ENOENT.
-		 */
 		if (lstat(ent->name, &st))
 			continue;
 
@@ -257,33 +297,60 @@ 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);
-				}
+				rel = path_relative(ent->name, prefix);
+				string_list_append(&del_list, rel);
 			}
-			strbuf_reset(&directory);
 		} else {
 			if (pathspec && !matches)
 				continue;
-			res = dry_run ? 0 : unlink(ent->name);
+			rel = path_relative(ent->name, prefix);
+			string_list_append(&del_list, rel);
+		}
+	}
+
+	/* TODO: do interactive git-clean here, which will modify del_list */
+
+	for_each_string_list_item(item, &del_list) {
+		struct stat st;
+
+		if (prefix) {
+			strbuf_addstr(&abs_path, prefix);
+		}
+		strbuf_addstr(&abs_path, item->string);
+
+		/*
+		 * we might have removed this as part of earlier
+		 * recursive directory removal, so lstat() here could
+		 * fail with ENOENT.
+		 */
+		if (lstat(abs_path.buf, &st))
+			continue;
+
+		if (S_ISDIR(st.st_mode)) {
+			if (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone))
+				errors++;
+			if (gone && !quiet) {
+				qname = quote_path_relative(item->string, -1, &buf, NULL);
+				printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
+			}
+		} else {
+			res = dry_run ? 0 : unlink(abs_path.buf);
 			if (res) {
-				qname = quote_path_relative(ent->name, -1, &buf, prefix);
+				qname = quote_path_relative(item->string, -1, &buf, NULL);
 				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, NULL);
 				printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
 			}
 		}
+		strbuf_reset(&abs_path);
 	}
 	free(seen);
 
-	strbuf_release(&directory);
+	strbuf_release(&abs_path);
+	string_list_clear(&del_list, 0);
 	string_list_clear(&exclude_list, 0);
 	return (errors != 0);
 }
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 02/12] git-clean: add support for -i/--interactive

[Jiang Xin]

Show what would be done and the user must confirm before actually
cleaning.

    Would remove ...
    Would remove ...
    Would remove ...

    Remove (y/n) ?

Press "y" to start cleaning, and press "n" if you want to abort.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 Documentation/git-clean.txt | 10 ++++++--
 builtin/clean.c             | 61 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index bdc3a..186e34 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,13 @@ 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.
 
 -n::
 --dry-run::
diff --git a/builtin/clean.c b/builtin/clean.c
index ccd4..2f9b9 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -15,10 +15,11 @@
 #include "quote.h"
 
 static int force = -1; /* unset */
+static int interactive;
 static struct string_list del_list = STRING_LIST_INIT_DUP;
 
 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
 };
 
@@ -185,6 +186,50 @@ static const char *path_relative(const char *in, const char *prefix)
 	return buf;
 }
 
+static void interactive_main_loop(void)
+{
+	struct strbuf confirm = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
+	struct string_list_item *item;
+	const char *qname;
+
+	while (del_list.nr) {
+		putchar('\n');
+		for_each_string_list_item(item, &del_list) {
+			qname = quote_path_relative(item->string, -1, &buf, NULL);
+			printf(_(msg_would_remove), qname);
+		}
+		putchar('\n');
+
+		printf(_("Remove (y/n) ? "));
+		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 {
+				continue;
+			}
+		}
+	}
+
+	strbuf_release(&buf);
+	strbuf_release(&confirm);
+}
+
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
 	int i, res;
@@ -204,6 +249,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		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"),
@@ -230,12 +276,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"));
 	}
 
@@ -309,7 +359,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	/* TODO: do interactive git-clean here, which will modify del_list */
+	if (interactive && !dry_run && del_list.nr > 0)
+		interactive_main_loop();
 
 	for_each_string_list_item(item, &del_list) {
 		struct stat st;
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 03/12] git-clean: show items of del_list 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.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Comments-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/config.txt |  4 ++++
 builtin/clean.c          | 49 +++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 44 insertions(+), 9 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 2f9b9..a16988 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -13,10 +13,12 @@
 #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 unsigned int colopts;
 
 static const char *const builtin_clean_usage[] = {
 	N_("git clean [-d] [-f] [-i] [-n] [-q] [-e <pattern>] [-x | -X] [--] <paths>..."),
@@ -31,8 +33,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);
 }
 
@@ -186,21 +193,46 @@ static const char *path_relative(const char *in, const char *prefix)
 	return buf;
 }
 
-static void interactive_main_loop(void)
+static void pretty_print_dels(void)
 {
-	struct strbuf confirm = STRBUF_INIT;
-	struct strbuf buf = STRBUF_INIT;
+	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, NULL);
+		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);
+}
+
+static void interactive_main_loop(void)
+{
+	struct strbuf confirm = STRBUF_INIT;
 
 	while (del_list.nr) {
 		putchar('\n');
-		for_each_string_list_item(item, &del_list) {
-			qname = quote_path_relative(item->string, -1, &buf, NULL);
-			printf(_(msg_would_remove), qname);
-		}
+		printf_ln(Q_("Would remove the following item:",
+			     "Would remove the following items:",
+			     del_list.nr));
 		putchar('\n');
 
+		pretty_print_dels();
+
 		printf(_("Remove (y/n) ? "));
 		if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
 			strbuf_trim(&confirm);
@@ -226,7 +258,6 @@ static void interactive_main_loop(void)
 		}
 	}
 
-	strbuf_release(&buf);
 	strbuf_release(&confirm);
 }
 
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 04/12] git-clean: 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 | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index a16988..126fc 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;
@@ -31,16 +32,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)
@@ -226,14 +292,18 @@ static void interactive_main_loop(void)
 
 	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');
 
 		pretty_print_dels();
 
+		clean_print_color(CLEAN_COLOR_PROMPT);
 		printf(_("Remove (y/n) ? "));
+		clean_print_color(CLEAN_COLOR_RESET);
 		if (strbuf_getline(&confirm, stdin, '\n') != EOF) {
 			strbuf_trim(&confirm);
 		} else {
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 05/12] 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 will use this framework to add
new actions for interactive git-clean later.

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 | 447 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 418 insertions(+), 29 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 126fc..a9a1ee 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -50,6 +50,36 @@ enum color_clean {
 	CLEAN_COLOR_ERROR = 5,
 };
 
+#define MENU_OPTS_SINGLETON		01
+#define MENU_OPTS_IMMEDIATE		02
+#define MENU_OPTS_LIST_ONLY		04
+
+struct menu_opts {
+	const char *header;
+	const char *prompt;
+	int flag;
+};
+
+#define MENU_RETURN_NO_LOOP		10
+
+struct menu_item {
+	char hotkey;
+	char *title;
+	int selected;
+	int (*fn)();
+};
+
+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;
+};
+
 static int parse_clean_color_slot(const char *var, int ofs)
 {
 	if (!strcasecmp(var+ofs, "reset"))
@@ -281,54 +311,413 @@ static void pretty_print_dels(void)
 	copts.indent = "  ";
 	copts.padding = 2;
 	print_columns(&list, colopts, &copts);
-	putchar('\n');
 	strbuf_release(&buf);
 	string_list_clear(&list, 0);
 }
 
-static void interactive_main_loop(void)
+static void pretty_print_menus(struct string_list *menu_list)
+{
+	unsigned int local_colopts = 0;
+	struct column_options copts;
+
+	local_colopts = COL_ENABLED | COL_ROW;
+	memset(&copts, 0, sizeof(copts));
+	copts.indent = "  ";
+	copts.padding = 2;
+	print_columns(menu_list, local_colopts, &copts);
+}
+
+static 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);
+}
+
+/*
+ * display menu stuff with number prefix and hotkey highlight
+ */
+static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen)
+{
+	static struct string_list menu_list = STRING_LIST_INIT_DUP;
+	struct strbuf menu = STRBUF_INIT;
+	int i;
+
+	/* 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++) {
+			char *p;
+			int highlighted = 0;
+
+			p = item->title;
+			if ((*chosen)[i] < 0)
+				(*chosen)[i] = item->selected ? 1 : 0;
+			strbuf_addf(&menu, "%s%2d: ", (*chosen)[i] ? "*" : " ", i+1);
+			for (; *p; p++) {
+				if (!highlighted && *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));
+					highlighted = 1;
+				} 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) {
+			if ((*chosen)[i] < 0)
+				(*chosen)[i] = 0;
+			strbuf_addf(&menu, "%s%2d: %s", (*chosen)[i] ? "*" : " ", ++i, item->string);
+			string_list_append(&menu_list, menu.buf);
+			strbuf_reset(&menu);
+		}
+		strbuf_release(&buf);
+	}
+
+	pretty_print_menus(&menu_list);
+
+	strbuf_release(&menu);
+	string_list_clear(&menu_list, 0);
+}
+
+/*
+ * Parse user input, and return choice(s) for menu (menu_stuff).
+ *
+ * Input
+ *     (for single choice)
+ *         1          - select a numbered item
+ *         foo        - select item based on menu title
+ *                    - (empty) select nothing
+ *
+ *     (for multiple choice)
+ *         1          - select a single item
+ *         3-5        - select a range of items
+ *         2-3,6-9    - select multiple ranges
+ *         foo        - select item based on menu title
+ *         -...       - unselect specified items
+ *         *          - choose all items
+ *                    - (empty) finish selecting
+ *
+ * The parse result will be saved in array **chosen, and
+ * return number of total selections.
+ */
+static int parse_choice(struct menu_stuff *menu_stuff,
+			int is_single,
+			struct strbuf input,
+			int **chosen)
+{
+	struct strbuf **choice_list, **choice_p;
+	int nr = 0;
+	int i;
+
+	if (is_single) {
+		choice_list = strbuf_split_max(&input, '\n', 0);
+	} else {
+		char *p = input.buf;
+		do {
+			if (*p == ',')
+				*p = ' ';
+		} while (*p++);
+		choice_list = strbuf_split_max(&input, ' ', 0);
+	}
+
+	for (choice_p = choice_list; *choice_p; choice_p++) {
+		char *p;
+		int choose = 1;
+		int bottom = 0, top = 0;
+		int is_range, is_number;
+
+		strbuf_trim(*choice_p);
+		if (!(*choice_p)->len)
+			continue;
+
+		/* Input that begins with '-'; unchoose */
+		if (*(*choice_p)->buf == '-') {
+			choose = 0;
+			strbuf_remove((*choice_p), 0, 1);
+		}
+
+		is_range = 0;
+		is_number = 1;
+		for(p = (*choice_p)->buf; *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_p)->buf);
+			top = bottom;
+		} else if (is_range) {
+			bottom = atoi((*choice_p)->buf);
+			if (!*(strchr((*choice_p)->buf, '-') + 1)) {
+				top = menu_stuff->nr - 1;
+			} else {
+				top = atoi(strchr((*choice_p)->buf, '-') + 1);
+			}
+		} else if (!strcmp((*choice_p)->buf, "*")) {
+			bottom = 1;
+			top = menu_stuff->nr;
+		} else {
+			if (MENU_STUFF_TYPE_MENU_ITEM == menu_stuff->type) {
+				struct menu_item *item;
+
+				item = (struct menu_item *)menu_stuff->stuff;
+				for (i = 0; i < menu_stuff->nr; i++, item++) {
+					if (((*choice_p)->len == 1 &&
+					     *(*choice_p)->buf == item->hotkey) ||
+					    !strcasecmp((*choice_p)->buf, item->title)) {
+						bottom = i + 1;
+						top = bottom;
+						break;
+					}
+				}
+			} else if (MENU_STUFF_TYPE_STRING_LIST == menu_stuff->type) {
+				struct string_list_item *item;
+
+				item = ((struct string_list *)menu_stuff->stuff)->items;
+				for (i = 0; i < menu_stuff->nr; i++, item++) {
+					if (!strcasecmp((*choice_p)->buf, item->string)) {
+						bottom = i + 1;
+						top = bottom;
+						break;
+					}
+				}
+			}
+		}
+
+		if (top <= 0 || bottom <= 0 || top > menu_stuff->nr || bottom > top ||
+		    (is_single && bottom != top)) {
+			clean_print_color(CLEAN_COLOR_ERROR);
+			printf_ln(_("Huh (%s)?"), (*choice_p)->buf);
+			clean_print_color(CLEAN_COLOR_RESET);
+			continue;
+		}
+
+		/* A range can be specified like 5-7 or 5-. */
+		for (i = bottom; i <= top; i++)
+			(*chosen)[i-1] = choose;
+	}
+
+	strbuf_list_free(choice_list);
+
+	for (i = 0; i < menu_stuff->nr; i++)
+		nr += (*chosen)[i];
+	return nr;
+}
+
+/*
+ * 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.
+ */
+static int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff)
 {
-	struct strbuf confirm = STRBUF_INIT;
+	struct strbuf choice = STRBUF_INIT;
+	int *chosen, *result;
+	int nr = 0;
+	int eof = 0;
+	int i;
+
+	chosen = xmalloc(sizeof(int) * stuff->nr);
+	/* set chosen as uninitialized */
+	for (i = 0; i < stuff->nr; i++)
+		chosen[i] = -1;
+
+	for (;;) {
+		if (opts->header) {
+			printf_ln("%s%s%s",
+				  clean_get_color(CLEAN_COLOR_HEADER),
+				  opts->header,
+				  clean_get_color(CLEAN_COLOR_RESET));
+		}
+
+		/* the chosen array can be initialized by menu_item.selected  */
+		print_highlight_menu_stuff(stuff, &chosen);
+
+		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) {
+			strbuf_trim(&choice);
+		} else {
+			eof = 1;
+			break;
+		}
 
+		/* help for prompt */
+		if (!strcmp(choice.buf, "?")) {
+			prompt_help_cmd(opts->flag & MENU_OPTS_SINGLETON);
+			continue;
+		}
+
+		/* for a multiple-choice menu, press ENTER (empty) will return back */
+		if (!(opts->flag & MENU_OPTS_SINGLETON) && !choice.len)
+			break;
+
+		nr = parse_choice(stuff,
+				  opts->flag & MENU_OPTS_SINGLETON,
+				  choice,
+				  &chosen);
+
+		if (opts->flag & MENU_OPTS_SINGLETON) {
+			if (nr)
+				break;
+		} else if (opts->flag & MENU_OPTS_IMMEDIATE) {
+			break;
+		}
+	}
+
+	if (eof) {
+		result = xmalloc(sizeof(int));
+		*result = EOF;
+	} else {
+		int j = 0;
+
+		/* recalculate nr, for a multiple-choice menu with initial selections */
+		if (!nr) {
+			for (i = 0; i < stuff->nr; i++)
+				nr += chosen[i];
+		}
+
+		result = xmalloc(sizeof(int) * (nr + 1));
+		memset(result, 0, sizeof(int) * (nr + 1));
+		for (i = 0; i < stuff->nr && j < nr; i++) {
+			if (chosen[i])
+				result[j++] = i;
+		}
+		result[j] = EOF;
+	}
+
+	free(chosen);
+	strbuf_release(&choice);
+	return result;
+}
+
+static int clean_cmd(void)
+{
+	return MENU_RETURN_NO_LOOP;
+}
+
+static int quit_cmd(void)
+{
+	string_list_clear(&del_list, 0);
+	printf_ln(_("Bye."));
+	return MENU_RETURN_NO_LOOP;
+}
+
+static int help_cmd(void)
+{
+	clean_print_color(CLEAN_COLOR_HELP);
+	printf_ln(_(
+		    "clean               - start cleaning\n"
+		    "quit                - stop cleaning\n"
+		    "help                - this screen\n"
+		    "?                   - help for prompt selection"
+		   ));
+	clean_print_color(CLEAN_COLOR_RESET);
+	return 0;
+}
+
+static void interactive_main_loop(void)
+{
 	while (del_list.nr) {
-		putchar('\n');
+		struct menu_opts menu_opts;
+		struct menu_stuff menu_stuff;
+		struct menu_item menus[] = {
+			{'c', "clean",			0, clean_cmd},
+			{'q', "quit",			0, quit_cmd},
+			{'h', "help",			0, 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');
 
 		pretty_print_dels();
 
-		clean_print_color(CLEAN_COLOR_PROMPT);
-		printf(_("Remove (y/n) ? "));
-		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 {
+		chosen = list_and_choose(&menu_opts, &menu_stuff);
+
+		if (*chosen != EOF) {
+			int ret;
+			ret = menus[*chosen].fn();
+			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)
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 06/12] git-clean: add filter by pattern interactive action

[Jiang Xin]

Add a new action for interactive git-clean: filter by pattern. When
the user chooses this action, 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>
---
 builtin/clean.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/builtin/clean.c b/builtin/clean.c
index a9a1ee..48731 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -646,6 +646,72 @@ static int clean_cmd(void)
 	return MENU_RETURN_NO_LOOP;
 }
 
+static int filter_by_patterns_cmd(void)
+{
+	struct dir_struct dir;
+	struct strbuf confirm = STRBUF_INIT;
+	struct strbuf **ignore_list;
+	struct string_list_item *item;
+	struct exclude_list *el;
+	int changed = -1, i;
+
+	for (;;) {
+		if (!del_list.nr)
+			break;
+
+		if (changed)
+			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
+			putchar('\n');
+
+		/* Quit filter_by_pattern mode if press ENTER or Ctrl-D */
+		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;
+
+			if (is_excluded(&dir, item->string, &dtype)) {
+				*item->string = '\0';
+				changed++;
+			}
+		}
+
+		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);
+		clear_directory(&dir);
+	}
+
+	strbuf_release(&confirm);
+	return 0;
+}
+
 static int quit_cmd(void)
 {
 	string_list_clear(&del_list, 0);
@@ -658,6 +724,7 @@ static int help_cmd(void)
 	clean_print_color(CLEAN_COLOR_HELP);
 	printf_ln(_(
 		    "clean               - start cleaning\n"
+		    "filter by pattern   - exclude items from deletion\n"
 		    "quit                - stop cleaning\n"
 		    "help                - this screen\n"
 		    "?                   - help for prompt selection"
@@ -673,6 +740,7 @@ static void interactive_main_loop(void)
 		struct menu_stuff menu_stuff;
 		struct menu_item menus[] = {
 			{'c', "clean",			0, clean_cmd},
+			{'f', "filter by pattern",	0, filter_by_patterns_cmd},
 			{'q', "quit",			0, quit_cmd},
 			{'h', "help",			0, help_cmd},
 		};
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 07/12] git-clean: add select by numbers interactive action

[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 48731..17b65 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -712,6 +712,43 @@ static int filter_by_patterns_cmd(void)
 	return 0;
 }
 
+static int select_by_numbers_cmd(void)
+{
+	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;
+}
+
 static int quit_cmd(void)
 {
 	string_list_clear(&del_list, 0);
@@ -725,6 +762,7 @@ static int help_cmd(void)
 	printf_ln(_(
 		    "clean               - start cleaning\n"
 		    "filter by pattern   - exclude items from deletion\n"
+		    "select by numbers   - select items to be deleted by numbers\n"
 		    "quit                - stop cleaning\n"
 		    "help                - this screen\n"
 		    "?                   - help for prompt selection"
@@ -741,6 +779,7 @@ static void interactive_main_loop(void)
 		struct menu_item menus[] = {
 			{'c', "clean",			0, clean_cmd},
 			{'f', "filter by pattern",	0, filter_by_patterns_cmd},
+			{'s', "select by numbers",	0, select_by_numbers_cmd},
 			{'q', "quit",			0, quit_cmd},
 			{'h', "help",			0, help_cmd},
 		};
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 08/12] git-clean: add ask each interactive action

[Jiang Xin]

Add a new action for interactive git-clean: ask each. It's just like
the "rm -i" command, that the user must confirm one by one for each
file or directory to be cleaned.

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 17b65..85345 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -749,6 +749,40 @@ static int select_by_numbers_cmd(void)
 	return 0;
 }
 
+static int rm_i_cmd(void)
+{
+	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, NULL);
+			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;
+}
+
 static int quit_cmd(void)
 {
 	string_list_clear(&del_list, 0);
@@ -763,6 +797,7 @@ static int help_cmd(void)
 		    "clean               - start cleaning\n"
 		    "filter by pattern   - exclude items from deletion\n"
 		    "select by numbers   - select items to be deleted by numbers\n"
+		    "ask each            - confirm each deletion (like \"rm -i\")\n"
 		    "quit                - stop cleaning\n"
 		    "help                - this screen\n"
 		    "?                   - help for prompt selection"
@@ -780,6 +815,7 @@ static void interactive_main_loop(void)
 			{'c', "clean",			0, clean_cmd},
 			{'f', "filter by pattern",	0, filter_by_patterns_cmd},
 			{'s', "select by numbers",	0, select_by_numbers_cmd},
+			{'a', "ask each",		0, rm_i_cmd},
 			{'q', "quit",			0, quit_cmd},
 			{'h', "help",			0, help_cmd},
 		};
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 09/12] git-clean refactor: save some options in clean_flags

[Jiang Xin]

Save some options in variable clean_flags, such as -ff (force > 1),
-x (ignored), -X (ignored_only), and -d (remove_directories). We may
change clean_flags later in the interactive git-clean.

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

diff --git a/builtin/clean.c b/builtin/clean.c
index 85345..26d00 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -16,7 +16,13 @@
 #include "column.h"
 #include "color.h"
 
+#define CLEAN_OPTS_SHOW_IGNORED		01
+#define CLEAN_OPTS_IGNORED_ONLY		02
+#define CLEAN_OPTS_REMOVE_DIRECTORIES	04
+#define CLEAN_OPTS_REMOVE_NESTED_GIT	010
+
 static int force = -1; /* unset */
+static int clean_flags = 0;
 static int interactive;
 static struct string_list del_list = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
@@ -868,7 +874,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	int i, res;
 	int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
 	int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
-	int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
 	struct strbuf abs_path = STRBUF_INIT;
 	struct dir_struct dir;
 	static const char **pathspec;
@@ -902,13 +907,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, options, builtin_clean_usage,
 			     0);
 
-	memset(&dir, 0, sizeof(dir));
-	if (ignored_only)
-		dir.flags |= DIR_SHOW_IGNORED;
-
-	if (ignored && ignored_only)
-		die(_("-x and -X cannot be used together"));
-
 	if (interactive) {
 		if (!isatty(0) || !isatty(1))
 			die(_("interactive clean can not run without a valid tty; "
@@ -922,15 +920,29 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 				  "refusing to clean"));
 	}
 
+	if (read_cache() < 0)
+		die(_("index file corrupt"));
+
 	if (force > 1)
-		rm_flags = 0;
+		clean_flags |= CLEAN_OPTS_REMOVE_NESTED_GIT;
+	if (ignored)
+		clean_flags |= CLEAN_OPTS_SHOW_IGNORED;
+	if (ignored_only)
+		clean_flags |= CLEAN_OPTS_IGNORED_ONLY;
+	if (remove_directories)
+		clean_flags |= CLEAN_OPTS_REMOVE_DIRECTORIES;
 
-	dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
+	memset(&dir, 0, sizeof(dir));
+	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY)
+		dir.flags |= DIR_SHOW_IGNORED;
 
-	if (read_cache() < 0)
-		die(_("index file corrupt"));
+	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY &&
+	    clean_flags & CLEAN_OPTS_SHOW_IGNORED)
+		die(_("-x and -X cannot be used together"));
+
+	dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
 
-	if (!ignored)
+	if (!(clean_flags & CLEAN_OPTS_SHOW_IGNORED))
 		setup_standard_excludes(&dir);
 
 	el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
@@ -980,7 +992,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 ((clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES) ||
+			    (matches == MATCHED_EXACTLY)) {
 				rel = path_relative(ent->name, prefix);
 				string_list_append(&del_list, rel);
 			}
@@ -1012,7 +1025,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 			continue;
 
 		if (S_ISDIR(st.st_mode)) {
-			if (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone))
+			if (remove_dirs(&abs_path, prefix,
+					clean_flags & CLEAN_OPTS_REMOVE_NESTED_GIT ?
+						0 : REMOVE_DIR_KEEP_NESTED_GIT,
+					dry_run, quiet, &gone))
 				errors++;
 			if (gone && !quiet) {
 				qname = quote_path_relative(item->string, -1, &buf, NULL);
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 10/12] git-clean refactor: add wrapper scan_clean_candidates

[Jiang Xin]

Add new wrapper `scan_clean_candidates`, which determines the del_list
(i.e. the cleaning candidates). This function will be reused later in
the interactive git-clean, so we can change flags of git-clean and
refresh the del_list.

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

diff --git a/builtin/clean.c b/builtin/clean.c
index 26d00..232d48 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -295,6 +295,96 @@ static const char *path_relative(const char *in, const char *prefix)
 	return buf;
 }
 
+static void scan_clean_candidates(const char **pathspec,
+				  struct string_list exclude_list,
+				  const char *prefix)
+{
+	struct dir_struct dir;
+	struct exclude_list *el;
+	char *seen = NULL;
+	const char **pathspec_p = pathspec;
+	const char *rel;
+	int pathspec_nr = 0;
+	int i;
+
+	while (pathspec_p && *(pathspec_p++))
+		pathspec_nr++;
+
+	memset(&dir, 0, sizeof(dir));
+	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY)
+		dir.flags |= DIR_SHOW_IGNORED;
+
+	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY &&
+	    clean_flags & CLEAN_OPTS_SHOW_IGNORED)
+		die(_("-x and -X cannot be used together"));
+
+	dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
+
+	if (!(clean_flags & CLEAN_OPTS_SHOW_IGNORED))
+		setup_standard_excludes(&dir);
+
+	el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
+	for (i = 0; i < exclude_list.nr; i++)
+		add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
+
+	fill_directory(&dir, pathspec);
+
+	if (pathspec)
+		seen = xmalloc(pathspec_nr > 0 ? pathspec_nr : 1);
+
+	string_list_clear(&del_list, 0);
+
+	for (i = 0; i < dir.nr; i++) {
+		struct dir_entry *ent = dir.entries[i];
+		int len, pos;
+		int matches = 0;
+		struct cache_entry *ce;
+		struct stat st;
+
+		/*
+		 * Remove the '/' at the end that directory
+		 * walking adds for directory entries.
+		 */
+		len = ent->len;
+		if (len && ent->name[len-1] == '/')
+			len--;
+		pos = cache_name_pos(ent->name, len);
+		if (0 <= pos)
+			continue;	/* exact match */
+		pos = -pos - 1;
+		if (pos < active_nr) {
+			ce = active_cache[pos];
+			if (ce_namelen(ce) == len &&
+			    !memcmp(ce->name, ent->name, len))
+				continue; /* Yup, this one exists unmerged */
+		}
+
+		if (lstat(ent->name, &st))
+			continue;
+
+		if (pathspec) {
+			memset(seen, 0, pathspec_nr > 0 ? pathspec_nr : 1);
+			matches = match_pathspec(pathspec, ent->name, len,
+						 0, seen);
+		}
+
+		if (S_ISDIR(st.st_mode)) {
+			if ((clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES) ||
+			    (matches == MATCHED_EXACTLY)) {
+				rel = path_relative(ent->name, prefix);
+				string_list_append(&del_list, rel);
+			}
+		} else {
+			if (pathspec && !matches)
+				continue;
+			rel = path_relative(ent->name, prefix);
+			string_list_append(&del_list, rel);
+		}
+	}
+
+	free(seen);
+}
+
 static void pretty_print_dels(void)
 {
 	struct string_list list = STRING_LIST_INIT_DUP;
@@ -871,18 +961,15 @@ static void interactive_main_loop(void)
 
 int cmd_clean(int argc, const char **argv, const char *prefix)
 {
-	int i, res;
+	int res;
 	int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
 	int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
 	struct strbuf abs_path = STRBUF_INIT;
-	struct dir_struct dir;
 	static const char **pathspec;
 	struct strbuf buf = STRBUF_INIT;
-	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
-	struct exclude_list *el;
 	struct string_list_item *item;
+	struct string_list exclude_list = STRING_LIST_INIT_NODUP;
 	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")),
@@ -932,78 +1019,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	if (remove_directories)
 		clean_flags |= CLEAN_OPTS_REMOVE_DIRECTORIES;
 
-	memset(&dir, 0, sizeof(dir));
-	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY)
-		dir.flags |= DIR_SHOW_IGNORED;
-
-	if (clean_flags & CLEAN_OPTS_IGNORED_ONLY &&
-	    clean_flags & CLEAN_OPTS_SHOW_IGNORED)
-		die(_("-x and -X cannot be used together"));
-
-	dir.flags |= DIR_SHOW_OTHER_DIRECTORIES;
-
-	if (!(clean_flags & CLEAN_OPTS_SHOW_IGNORED))
-		setup_standard_excludes(&dir);
-
-	el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
-	for (i = 0; i < exclude_list.nr; i++)
-		add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
-
 	pathspec = get_pathspec(prefix, argv);
 
-	fill_directory(&dir, pathspec);
-
-	if (pathspec)
-		seen = xmalloc(argc > 0 ? argc : 1);
-
-	for (i = 0; i < dir.nr; i++) {
-		struct dir_entry *ent = dir.entries[i];
-		int len, pos;
-		int matches = 0;
-		struct cache_entry *ce;
-		struct stat st;
-		const char *rel;
-
-		/*
-		 * Remove the '/' at the end that directory
-		 * walking adds for directory entries.
-		 */
-		len = ent->len;
-		if (len && ent->name[len-1] == '/')
-			len--;
-		pos = cache_name_pos(ent->name, len);
-		if (0 <= pos)
-			continue;	/* exact match */
-		pos = -pos - 1;
-		if (pos < active_nr) {
-			ce = active_cache[pos];
-			if (ce_namelen(ce) == len &&
-			    !memcmp(ce->name, ent->name, len))
-				continue; /* Yup, this one exists unmerged */
-		}
-
-		if (lstat(ent->name, &st))
-			continue;
-
-		if (pathspec) {
-			memset(seen, 0, argc > 0 ? argc : 1);
-			matches = match_pathspec(pathspec, ent->name, len,
-						 0, seen);
-		}
-
-		if (S_ISDIR(st.st_mode)) {
-			if ((clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES) ||
-			    (matches == MATCHED_EXACTLY)) {
-				rel = path_relative(ent->name, prefix);
-				string_list_append(&del_list, rel);
-			}
-		} else {
-			if (pathspec && !matches)
-				continue;
-			rel = path_relative(ent->name, prefix);
-			string_list_append(&del_list, rel);
-		}
-	}
+	scan_clean_candidates(pathspec, exclude_list, prefix);
 
 	if (interactive && !dry_run && del_list.nr > 0)
 		interactive_main_loop();
@@ -1047,7 +1065,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		}
 		strbuf_reset(&abs_path);
 	}
-	free(seen);
 
 	strbuf_release(&abs_path);
 	string_list_clear(&del_list, 0);
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 11/12] git-clean: add toggle flags interactive action

[Jiang Xin]

Add new action in the interactive mode, so that the user can change
git-clean flags, such as -x/-X/-d/-ff, and refresh the cleaning
candidates list.

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

diff --git a/builtin/clean.c b/builtin/clean.c
index 232d48..b4472 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -879,6 +879,66 @@ static int rm_i_cmd(void)
 	return MENU_RETURN_NO_LOOP;
 }
 
+static int toggle_flags_cmd(void)
+{
+	struct menu_opts menu_opts;
+	struct menu_stuff menu_stuff;
+	struct menu_item menus[] = {
+		{'d', "(d) remove directories",
+			clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES,	NULL},
+		{'x', "(x) show ignored",
+			clean_flags & CLEAN_OPTS_SHOW_IGNORED,		NULL},
+		{'X', "(X) ignored only",
+			clean_flags & CLEAN_OPTS_IGNORED_ONLY,		NULL},
+		{'f', "(ff) remove nested.git",
+			clean_flags & CLEAN_OPTS_REMOVE_NESTED_GIT,	NULL},
+	};
+	int new_flags = 0;
+	int *chosen;
+	int i;
+
+	menu_opts.header = NULL;
+	menu_opts.prompt = "Change flags";
+	menu_opts.flag = 0;
+
+	menu_stuff.type = MENU_STUFF_TYPE_MENU_ITEM;
+	menu_stuff.stuff = menus;
+	menu_stuff.nr = sizeof(menus) / sizeof(struct menu_item);
+
+	chosen = list_and_choose(&menu_opts, &menu_stuff);
+
+	for (i = 0; chosen[i] != EOF; i++) {
+		switch (chosen[i]) {
+		case 0:
+			new_flags |= CLEAN_OPTS_REMOVE_DIRECTORIES;
+			break;
+		case 1:
+			new_flags |= CLEAN_OPTS_SHOW_IGNORED;
+			break;
+		case 2:
+			new_flags |= CLEAN_OPTS_IGNORED_ONLY;
+			break;
+		case 3:
+			new_flags |= CLEAN_OPTS_REMOVE_NESTED_GIT;
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (new_flags & CLEAN_OPTS_IGNORED_ONLY &&
+	    new_flags & CLEAN_OPTS_SHOW_IGNORED) {
+		clean_print_color(CLEAN_COLOR_ERROR);
+		printf_ln(_("-x and -X cannot be used together"));
+		clean_print_color(CLEAN_COLOR_RESET);
+	} else {
+		clean_flags = new_flags;
+	}
+
+	free(chosen);
+	return 0;
+}
+
 static int quit_cmd(void)
 {
 	string_list_clear(&del_list, 0);
@@ -894,6 +954,7 @@ static int help_cmd(void)
 		    "filter by pattern   - exclude items from deletion\n"
 		    "select by numbers   - select items to be deleted by numbers\n"
 		    "ask each            - confirm each deletion (like \"rm -i\")\n"
+		    "toggle flags        - toggle git-clean flags and update the list\n"
 		    "quit                - stop cleaning\n"
 		    "help                - this screen\n"
 		    "?                   - help for prompt selection"
@@ -902,9 +963,14 @@ static int help_cmd(void)
 	return 0;
 }
 
-static void interactive_main_loop(void)
+static void interactive_main_loop(const char **pathspec,
+				  struct string_list exclude_list,
+				  const char *prefix)
 {
-	while (del_list.nr) {
+	int cached_clean_flags = clean_flags;
+	char flags_title[40];
+
+	for (;;) {
 		struct menu_opts menu_opts;
 		struct menu_stuff menu_stuff;
 		struct menu_item menus[] = {
@@ -912,11 +978,24 @@ static void interactive_main_loop(void)
 			{'f', "filter by pattern",	0, filter_by_patterns_cmd},
 			{'s', "select by numbers",	0, select_by_numbers_cmd},
 			{'a', "ask each",		0, rm_i_cmd},
+			{'t', flags_title,		0, toggle_flags_cmd},
 			{'q', "quit",			0, quit_cmd},
 			{'h', "help",			0, help_cmd},
 		};
 		int *chosen;
 
+		if (!clean_flags) {
+			strncpy(flags_title, "toggle flags: none", sizeof(flags_title)/sizeof(char));
+		} else {
+			snprintf(flags_title, sizeof(flags_title)/sizeof(char),
+				 "toggle flags: -%s%s%s%s",
+				 clean_flags & CLEAN_OPTS_REMOVE_DIRECTORIES ? "d" : "",
+				 clean_flags & CLEAN_OPTS_SHOW_IGNORED ? "x" : "",
+				 clean_flags & CLEAN_OPTS_IGNORED_ONLY ? "X" : "",
+				 clean_flags & CLEAN_OPTS_REMOVE_NESTED_GIT ? "ff" : ""
+				);
+		}
+
 		menu_opts.header = _("*** Commands ***");
 		menu_opts.prompt = "What now";
 		menu_opts.flag = MENU_OPTS_SINGLETON;
@@ -925,13 +1004,25 @@ static void interactive_main_loop(void)
 		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);
+		if (cached_clean_flags != clean_flags) {
+			scan_clean_candidates(pathspec, exclude_list, prefix);
+			cached_clean_flags = clean_flags;
+		}
 
-		pretty_print_dels();
+		if (del_list.nr) {
+			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);
+
+			pretty_print_dels();
+		} else {
+			clean_print_color(CLEAN_COLOR_HEADER);
+			printf_ln(_("NOTE: no more files to clean; press \"t\" to toggle flags of git-clean."));
+			putchar('\n');
+			clean_print_color(CLEAN_COLOR_RESET);
+		}
 
 		chosen = list_and_choose(&menu_opts, &menu_stuff);
 
@@ -941,12 +1032,6 @@ static void interactive_main_loop(void)
 			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 {
@@ -1023,8 +1108,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	scan_clean_candidates(pathspec, exclude_list, prefix);
 
-	if (interactive && !dry_run && del_list.nr > 0)
-		interactive_main_loop();
+	if (interactive && !dry_run)
+		interactive_main_loop(pathspec, exclude_list, prefix);
 
 	for_each_string_list_item(item, &del_list) {
 		struct stat st;
-- 
1.8.3.rc1.341.g24a8a0f

[PATCH v8 12/12] git-clean: update document for interactive git-clean

[Jiang Xin]

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
---
 Documentation/git-clean.txt | 71 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 186e34..4512f7 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -39,8 +39,8 @@ OPTIONS
 
 -i::
 --interactive::
-	Show what would be done and the user must confirm before actually
-	cleaning.
+	Show what would be done and clean files interactively. See
+	``Interactive mode'' for details.
 
 -n::
 --dry-run::
@@ -69,6 +69,73 @@ 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: filter by pattern     3: select by numbers
+      4. ask each      5. toggle flags: none    6. quit
+      7: help
+    What now> 2
+------------
+
+You also could say `c` or `clean` above as long as the choice is unique.
+
+The main command loop has 7 subcommands.
+
+clean::
+
+   Start cleaning files and directories, and then quit.
+
+filter by pattern::
+
+   This shows the files and directories to be deleted and issues an
+   "Input ignore patterns>>" prompt. You can input 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.
+
+select 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.
+
+ask each::
+
+  This will start to clean, and you must confirm one by one in order
+  to delete items. Please note that this action is not as efficient
+  as the above two actions.
+
+toggle flags::
+
+  This lets you change the flags for git-clean, such as -x/-X/-d/-ff,
+  and refresh the cleaning candidates list automatically.
+
+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.341.g24a8a0f

Re: [PATCH v7 01/10] Add support for -i/--interactive to git-clean

[Matthieu Moy]

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

> +			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');


> +		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');

These two pieces of code are surprisingly similar ... Shouldn't they be
factored into a small helper function?

> +		/* Confirmation dialog */
> +		printf(_("Remove ([y]es/[n]o/[e]dit) ? "));

To be more consistent with "git add -p", this should use [] instead of
(), and have no space before "?".

> +			die(_("clean.requireForce defaults to true and neither -i, -n nor -f given; "
>  				  "refusing to clean"));

That makes it a 85 characters message, and we usually break lines before
80. Adding \n after ";" (instead of a space) would be better IMO.

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

Re: [PATCH v7 01/10] Add support for -i/--interactive to git-clean

[Jiang Xin]

2013/5/13 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
>> +             /* Confirmation dialog */
>> +             printf(_("Remove ([y]es/[n]o/[e]dit) ? "));
>
> To be more consistent with "git add -p", this should use [] instead of
> (), and have no space before "?".

Will be replaced with:

             printf(_("Remove [y/n]? "));

>
>> +                     die(_("clean.requireForce defaults to true and neither -i, -n nor -f given; "
>>                                 "refusing to clean"));
>
> That makes it a 85 characters message, and we usually break lines before
> 80. Adding \n after ";" (instead of a space) would be better IMO.
>

If die with multiple lines, it looks ugly. E.g.

    % git clean
    fatal: clean.requireForce defaults to true and neither -i, -n nor -f given;
    refusing to clean

I think because of this, some die messages are longer than 80 characters.
Such as:

    # builtin/apply.c:1496

    die(Q_("git diff header lacks filename information when removing "
                "%d leading pathname component (line %d)",
                "git diff header lacks filename information when removing "
                "%d leading pathname components (line %d)",

-- 
Jiang Xin
http://www.worldhello.net/

Re: [PATCH v7 00/10] interactive git clean

[Eric Sunshine]

On Wed, May 8, 2013 at 7:38 AM, Jiang Xin <worldhello.net@gmail.com> wrote:
> When the command enters the interactive mode, it shows the
> files and directories to be cleaned, and goes into its
> interactive command loop.

Your current implementation only allows excluding items from the list
of files to delete. If you accidentally exclude some file which you
actually want in the deletion list, there is no way to re-add it.
Would it make sense to change the behavior so that it lists all files
but stars those which are to be deleted. This is similar to how the
"edit by numbers" mode operates, but would apply to the deletion list
printed for the top-level menu as well. For example:

  Will remove starred items:
    file1  *file2  *file3
    *file4  file5  file6
  *** Commands ***
  ...
  What now> clean
  Removing file2
  Removing file3
  Removing file4

> 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: flags: none         6: quit                7: help
>     What now> 2
>
> You also could say `c` or `clean` above as long as the choice is unique.

It is not obvious by reading the menu that "edit by patterns" can be
abbreviated as 'p', and "edit by numbers" by 'n'. If you change the
names a bit, then the abbreviations become more obvious. For instance,
one possibility:

  2. filter by pattern
  3. select by number
  5. toggle flags: none

Also, the abbreviation 'i' for "rm -i" is not obvious.

> 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.

It seems odd that "edit by patterns" _excludes_ files from deletion,
but "edit by patterns" _includes_ them in the deletion list. These
opposing behaviors force the user to invert his mode of thought when
editing by patterns vs. numbers. While playing with "edit by numbers",
I repeatedly found myself incorrectly inputting numbers of items I
wanted to exclude rather than those I wanted to keep in the list,
since my brain had not made the 180 degree flip from "edit by
patterns".

More generally, there are cases when it is more convenient to filter
the list by exclusion, and other cases when inclusion is more
convenient. For example, in a list of 20 files, I may want to delete
18 but keep 2. In this case, it typically is easier to specify the two
I want to keep. On the other hand, if I want to delete 2 files but
keep 18, it may be easier to specify the two I want to delete. Would
it makes sense to support pattern negation (via '!' perhaps) in order
to make this possible?

One thing not mentioned here is that "edit by numbers" (as with
git-add-interactive) also accepts input "foo" to match item "foo" in
the list. This suggests that it might make sense to accept patterns
also, so that "*oo" can match "foo". This, together with negation and
the idea mentioned above of always listing all files and only deleting
starred ones, would allow you to combine "edit by patterns" and "edit
by numbers" into a single mode of operation.

> 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.

The name "rm -i" is rather Unixy, and Windows users might not
understand it, nor people who don't use rm's -i option. Other
potentially better names might be: "ask", "ask each", "confirm",
"confirm each", "prompt", or "prompt each".

Functionally, the current implementation of this mode makes it an
oddball. "edit by patterns" and "edit by numbers" both return to the
top-level menu, allowing the user to invoke "clean" or "quit" (or some
other option), but "rm -i" always finishes by running "clean"
unconditionally. This is strangely inconsistent. Shouldn't it also
return to the top-level menu to give the user an opportunity to review
the final selection before invoking "clean" or "quit"?

> flags::
>
>   This lets you change the flags for git-clean, such as -x/-X/-d/-ff,
>   and refresh the cleaning candidates list automatically.

Is your "edit by patterns" and "edit by numbers" input remembered
across flags changes? Should it be? In other words, if you spent some
time fine-tuning the deletion list but then realized that you forgot
the -d flag on the command line, you might want to toggle the -d flag
here without losing the fine-tuning you already did.

>
> quit::
>
>   This lets you quit without do cleaning.
>
> help::
>
>   Show brief usage of interactive git-clean.

Have you considered the idea of an additional menu item which launches
an editor with a list of files to be removed so the user can edit it
[*1*]? This might also be a convenient way to fine-tune the deletion
list, though it certainly doesn't need to be part of this series.

[*1*] http://thread.gmane.org/gmane.comp.version-control.git/223271/focus=223431

> Jiang Xin (10):
>   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
>   git-clean refactor: save some options in clean_flags
>   git-clean refactor: wrap in scan_clean_candidates
>   git-clean: change clean flags in interactive mode

The way this series is built up seems a bit odd for a couple reasons.

First, "edit by patterns" is just one of several selection modes
supported by --interactive. Why is it lumped in the first patch
whereas "edit by numbers" and "rm -i" are each added separately?
Shouldn't "edit by patterns", therefore, also be introduced in a
separate patch? Doing so might make review simpler.

Second, if you know that your intention is to emulate
git-add-interactive UI style, then implementing a "dumb" UI in patch 1
only to replace it entirely in patch 4 seems superfluous and
burdensome to reviewers. Wouldn't it make more sense to "do the right
thing" from the start by adding the desired UI early?

Given the above considerations, perhaps the series could be composed like this:

  ui: add a git-add-interactive-style UI for general use
  git-clean: add support for -i/--interactive to git-clean
    (with menu items "clean", "quit", "help")
  git-clean: interactive: show items in columns
  git-clean: interactive: add colors
  git-clean: interactive: add pattern-based fine-tuning
  git-clean: interactive: add number-based fine-turning
  git-clean: interactive: add "rm -i"-style fine-tuning
  git-clean: refactor: save some options in clean_flags
  git-clean: refactor: wrap in scan_clean_candidates
  git-clean: interactive: allow changing clean-flags
  doc: document interactive git-clean

Re: [PATCH v7 00/10] interactive git clean

[Eric Sunshine]

On Wed, May 8, 2013 at 11:15 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Wed, May 8, 2013 at 7:38 AM, Jiang Xin <worldhello.net@gmail.com> wrote:
>>     *** Commands ***
>>       1: clean               2: edit by patterns    3: edit by numbers     4: rm -i
>>       5: flags: none         6: quit                7: help
>>     What now> 2
>>
>> You also could say `c` or `clean` above as long as the choice is unique.
>
> It is not obvious by reading the menu that "edit by patterns" can be
> abbreviated as 'p', and "edit by numbers" by 'n'. If you change the
> names a bit, then the abbreviations become more obvious. For instance,
> one possibility:
>
>   2. filter by pattern
>   3. select by number
>   5. toggle flags: none

By "more obvious", I meant that the initial letter of each option is
now unique across the entire menu:

  2. [f]ilter by pattern
  3. [s]elect by number
  4. [t]oggle flags: none

Re: [PATCH v7 00/10] interactive git clean

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, May 8, 2013 at 7:38 AM, Jiang Xin <worldhello.net@gmail.com> wrote:
>> When the command enters the interactive mode, it shows the
>> files and directories to be cleaned, and goes into its
>> interactive command loop.
>
> Your current implementation only allows excluding items from the list
> of files to delete. If you accidentally exclude some file which you
> actually want in the deletion list, there is no way to re-add it.

If you accidentally dropped items from the list, is it such a big
deal?  I would imagine, if I were in that situation, that I would
simply continue and concentrate on making sure the remaining list
does not include anything I want to keep, clean them and then run
the command again, at which point the list of untracked paths in the
list will be much smaller.

We cannot make the same argument against the approach to first
present a list and _remove_ items that should not be dropped. That
is a genuine improvement, without which you cannot do a similar
incremental removal, whose first invocation removes only a subset
(but still a subset with no precious files you need to keep) of
files to allow the list subsequent invocations presents to *shrink*.

Cleaning all unneeded files inside a single interactive session
should *never* be the goal---that will lead to an over-engineered
design (e.g. switching "clean -x" flags in the middle? why?).  I
think Jiang's latest series is already way over-engineered, and I
suspect your suggestion leads it more into that direction.

Re: [PATCH v7 00/10] interactive git clean

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Cleaning all unneeded files inside a single interactive session
> should *never* be the goal---that will lead to an over-engineered
> design (e.g. switching "clean -x" flags in the middle? why?).  I
> think Jiang's latest series is already way over-engineered,

I didn't read the end of the series in detail, but I tend to agree with
you. Maybe it's time to remember the YAGNI principle, and keep the
number of options small at least for now (e.g. drop 11/12 git-clean: add
toggle flags interactive action). When users (including us) will have
used "git clean -i" for a while, we'll have a better idea of what is
really needed.

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

Re: [PATCH v7 00/10] interactive git clean

[Jiang Xin]

2013/5/13 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Cleaning all unneeded files inside a single interactive session
>> should *never* be the goal---that will lead to an over-engineered
>> design (e.g. switching "clean -x" flags in the middle? why?).  I
>> think Jiang's latest series is already way over-engineered,
>
> I didn't read the end of the series in detail, but I tend to agree with
> you. Maybe it's time to remember the YAGNI principle, and keep the
> number of options small at least for now (e.g. drop 11/12 git-clean: add
> toggle flags interactive action). When users (including us) will have
> used "git clean -i" for a while, we'll have a better idea of what is
> really needed.

My initial intention for flags toggle is for git newbies, who are not clear
about how to use -x/-d/-X/-ff options. I feel it may have values for these
people. But since most people feel it may bring trouble, I can withdraw
patch 9/12, 10/12, and 11/12.

Another improvement I want to hear your opinions. When the user execute
"git clean --interactive --dry-run", the current implement is ignore the
"--interactive" option, and do dry-run only. How about we combine them
together? First show the user the interactive interface, and when user
choose to clean, do cleaning in dry-run mode.

-- 
Jiang Xin
http://www.worldhello.net/

Re: [PATCH v7 00/10] interactive git clean

[Matthieu Moy]

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

> My initial intention for flags toggle is for git newbies, who are not clear
> about how to use -x/-d/-X/-ff options. I feel it may have values for these
> people.

It may have value for some of them, but throwing too many options in a
menu is usually counter-productive for a newbie. I never heard anyone
say "Git is super newbie-friendly because it has so many commands", but
quite often the opposite.

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