[v2,2/2] clean: refector to the interactive part of clean

Commit Message

Nadav Goldstein May 23, 2022, 6:12 a.m. UTC

From: Nadav Goldstein <[e-mail]>

moved the code that responsible for presenting the menu options
(interactive flag) to be handles by the add-menu lib that was added
in previous commit.

Signed-off-by: Nadav Goldstein <[e-mail]>
---
 add-menu.c      |  78 +++++----
 add-menu.h      |   8 +-
 builtin/clean.c | 413 ++----------------------------------------------
 3 files changed, 68 insertions(+), 431 deletions(-)

Comments

Derrick Stolee May 23, 2022, 7:45 p.m. UTC | #1

On 5/23/22 2:12 AM, Nadav Goldstein via GitGitGadget wrote:
> From: Nadav Goldstein <[e-mail]>
> 
> moved the code that responsible for presenting the menu options
> (interactive flag) to be handles by the add-menu lib that was added
> in previous commit.

Please see Documentation/SubmittingPatches, specifically the
"present tense" section [1]. 

[1] https://github.com/git/git/blob/f9b95943b68b6b8ca5a6072f50a08411c6449b55/Documentation/SubmittingPatches#L174-L179

> -static int clean_use_color = -1;
> -static char clean_colors[][COLOR_MAXLEN] = {
> -	[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
> -	[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
> -	[CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,
> -	[CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,
> -	[CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,
> -	[CLEAN_COLOR_RESET] = GIT_COLOR_RESET,
> -};
> -
> -
> -static const char *clean_get_color(enum color_clean ix)
> +static const char *clean_get_color(enum color_clean ix, clean_color_settings *clean_colors, int *clean_use_color)

I'm surprised to see changes to add-menu.c in this patch. I
would expect that add-menu.c was written in its correct form
in the first patch and then this patch could focus entirely
on deleting matching code from builtin/clean.c and calling
the API exposed in add-menu.h.

Perhaps this was just an interactive rebase issue? Fixed up
the wrong commit? That happens to me a lot.

There are also a lot of places that refer to "clean" when
you want this API to be abstracted away from 'git clean'.

> \ No newline at end of file
> diff --git a/add-menu.h b/add-menu.h
> index 52e5ccb1800..64f1cbdab9f 100644
> --- a/add-menu.h
> +++ b/add-menu.h
> @@ -1,3 +1,7 @@
> +#include "color.h"
> +
> +typedef char clean_color_settings[][COLOR_MAXLEN];

This typedef also shouldn't reference 'git clean'.

> +
>  #define MENU_OPTS_SINGLETON		01
>  #define MENU_OPTS_IMMEDIATE		02
>  #define MENU_OPTS_LIST_ONLY		04
> @@ -35,7 +39,7 @@ struct menu_stuff {
>  	void *stuff;
>  };
>  
> -void clean_print_color(enum color_clean ix);
> +void clean_print_color(enum color_clean ix, clean_color_settings *clean_colors, int *clean_use_color);
>  
>  /*
>   * Implement a git-add-interactive compatible UI, which is borrowed
> @@ -48,4 +52,4 @@ void clean_print_color(enum color_clean ix);
>   *   - The array ends with EOF.
>   *   - If user pressed CTRL-D (i.e. EOF), no selection returned.
>   */
> -int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff, void (*prompt_help_cmd)(int));
> \ No newline at end of file
> +int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff, clean_color_settings *clean_colors, int *clean_use_color, void (*prompt_help_cmd)(int));

There are a lot of instances where your lines are much too
wide. Documentation/CodingGuidelines has a lot of style
requirements, including trying to stay to an 80-character
width. There are a lot of examples in the codebase that
show how to break a method prototype across multiple lines
with tasteful vertical alignment.

> diff --git a/builtin/clean.c b/builtin/clean.c
> index 5466636e666..ef220869851 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -19,6 +19,7 @@
>  #include "pathspec.h"
>  #include "help.h"
>  #include "prompt.h"
> +#include "add-menu.h"
>  
>  static int force = -1; /* unset */
>  static int interactive;
> @@ -39,14 +40,6 @@ static const char *msg_warn_lstat_failed = N_("could not lstat %s\n");
>  static const char *msg_skip_cwd = N_("Refusing to remove current working directory\n");
>  static const char *msg_would_skip_cwd = N_("Would refuse to remove current working directory\n");
>  
> -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
> -};

This removal doesn't seem valuable. I think this color set
should remain in the builtin, especially because it's being
used further down.

Alternatively, the names can be renamed to "MENU_COLOR_..."
to apply to all commands in the add-menu.h API.

>  static const char *color_interactive_slots[] = {
>  	[CLEAN_COLOR_ERROR]  = "error",
> @@ -58,7 +51,7 @@ static const char *color_interactive_slots[] = {
>  };
>  
>  static int clean_use_color = -1;
> -static char clean_colors[][COLOR_MAXLEN] = {
> +static clean_color_settings clean_colors = {
>  	[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
>  	[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
>  	[CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,
> @@ -67,36 +60,8 @@ static char clean_colors[][COLOR_MAXLEN] = {
>  	[CLEAN_COLOR_RESET] = GIT_COLOR_RESET,
>  };
>  
> -#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 flags;
> -};
> -

Generally, the remainder of this patch is primarily
deletions. Although, we could make it be completely
deletions if the API method names are changed (not to
start with "clean_" and then all these calls are
modified in one go:

>  static void prompt_help_cmd(int singleton)
>  {
> -	clean_print_color(CLEAN_COLOR_HELP);
> +	clean_print_color(CLEAN_COLOR_HELP, &clean_colors, &clean_use_color);

Here, we would have something like "menu_print_color()"
instead. To avoid adding too much deletion noise when
making these important changes, we can add MAYBE_UNUSED
to all of the static methods that become unreachable.

After a patch that does that refactor, a diff that only
deletes lines (does not add any) would be very easy to
verify.

Thanks,
-Stolee

Patch

diff --git a/add-menu.c b/add-menu.c
index 6a1c125d113..becf4956917 100644
--- a/add-menu.c
+++ b/add-menu.c
@@ -13,24 +13,18 @@ 
 #include "help.h"
 #include "prompt.h"
 
-static int clean_use_color = -1;
-static char clean_colors[][COLOR_MAXLEN] = {
-	[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
-	[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
-	[CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,
-	[CLEAN_COLOR_PLAIN] = GIT_COLOR_NORMAL,
-	[CLEAN_COLOR_PROMPT] = GIT_COLOR_BOLD_BLUE,
-	[CLEAN_COLOR_RESET] = GIT_COLOR_RESET,
-};
-
-
-static const char *clean_get_color(enum color_clean ix)
+static const char *clean_get_color(enum color_clean ix, clean_color_settings *clean_colors, int *clean_use_color)
 {
-	if (want_color(clean_use_color))
-		return clean_colors[ix];
+	if (want_color(*clean_use_color))
+		return (*clean_colors)[ix];
 	return "";
 }
 
+void clean_print_color(enum color_clean ix, clean_color_settings *clean_colors, int *clean_use_color)
+{
+	printf("%s", clean_get_color(ix, clean_colors, clean_use_color));
+}
+
 static int find_unique(const char *choice, struct menu_stuff *menu_stuff)
 {
 	struct menu_item *menu_item;
@@ -80,10 +74,33 @@  static int find_unique(const char *choice, struct menu_stuff *menu_stuff)
 	return found;
 }
 
+/*
+ * 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)
+			int **chosen,
+			clean_color_settings *clean_colors,
+			int *clean_use_color)
 {
 	struct strbuf **choice_list, **ptr;
 	int nr = 0;
@@ -155,9 +172,9 @@  static int parse_choice(struct menu_stuff *menu_stuff,
 
 		if (top <= 0 || bottom <= 0 || top > menu_stuff->nr || bottom > top ||
 		    (is_single && bottom != top)) {
-			clean_print_color(CLEAN_COLOR_ERROR);
+			clean_print_color(CLEAN_COLOR_ERROR, clean_colors, clean_use_color);
 			printf(_("Huh (%s)?\n"), (*ptr)->buf);
-			clean_print_color(CLEAN_COLOR_RESET);
+			clean_print_color(CLEAN_COLOR_RESET, clean_colors, clean_use_color);
 			continue;
 		}
 
@@ -187,7 +204,7 @@  static void pretty_print_menus(struct string_list *menu_list)
 /*
  * display menu stuff with number prefix and hotkey highlight
  */
-static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen)
+static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen, clean_color_settings *clean_colors, int *clean_use_color)
 {
 	struct string_list menu_list = STRING_LIST_INIT_DUP;
 	struct strbuf menu = STRBUF_INIT;
@@ -210,9 +227,9 @@  static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen)
 			strbuf_addf(&menu, "%s%2d: ", (*chosen)[i] ? "*" : " ", i+1);
 			for (; *p; p++) {
 				if (!highlighted && *p == menu_item->hotkey) {
-					strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_PROMPT));
+					strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_PROMPT, clean_colors, clean_use_color));
 					strbuf_addch(&menu, *p);
-					strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_RESET));
+					strbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_RESET, clean_colors, clean_use_color));
 					highlighted = 1;
 				} else {
 					strbuf_addch(&menu, *p);
@@ -242,7 +259,7 @@  static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen)
 	string_list_clear(&menu_list, 0);
 }
 
-int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff, void (*prompt_help_cmd)(int))
+int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff, clean_color_settings *clean_colors, int *clean_use_color, void (*prompt_help_cmd)(int))
 {
 	struct strbuf choice = STRBUF_INIT;
 	int *chosen, *result;
@@ -258,23 +275,23 @@  int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff, void (*pr
 	for (;;) {
 		if (opts->header) {
 			printf_ln("%s%s%s",
-				  clean_get_color(CLEAN_COLOR_HEADER),
+				  clean_get_color(CLEAN_COLOR_HEADER, clean_colors, clean_use_color),
 				  _(opts->header),
-				  clean_get_color(CLEAN_COLOR_RESET));
+				  clean_get_color(CLEAN_COLOR_RESET, clean_colors, clean_use_color));
 		}
 
 		/* chosen will be initialized by print_highlight_menu_stuff */
-		print_highlight_menu_stuff(stuff, &chosen);
+		print_highlight_menu_stuff(stuff, &chosen, clean_colors, clean_use_color);
 
 		if (opts->flags & MENU_OPTS_LIST_ONLY)
 			break;
 
 		if (opts->prompt) {
 			printf("%s%s%s%s",
-			       clean_get_color(CLEAN_COLOR_PROMPT),
+			       clean_get_color(CLEAN_COLOR_PROMPT, clean_colors, clean_use_color),
 			       _(opts->prompt),
 			       opts->flags & MENU_OPTS_SINGLETON ? "> " : ">> ",
-			       clean_get_color(CLEAN_COLOR_RESET));
+			       clean_get_color(CLEAN_COLOR_RESET, clean_colors, clean_use_color));
 		}
 
 		if (git_read_line_interactively(&choice) == EOF) {
@@ -295,7 +312,9 @@  int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff, void (*pr
 		nr = parse_choice(stuff,
 				  opts->flags & MENU_OPTS_SINGLETON,
 				  choice,
-				  &chosen);
+				  &chosen,
+				  clean_colors,
+				  clean_use_color);
 
 		if (opts->flags & MENU_OPTS_SINGLETON) {
 			if (nr)
@@ -331,9 +350,4 @@  int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff, void (*pr
 	free(chosen);
 	strbuf_release(&choice);
 	return result;
-}
-
-void clean_print_color(enum color_clean ix)
-{
-	printf("%s", clean_get_color(ix));
 }
\ No newline at end of file
diff --git a/add-menu.h b/add-menu.h
index 52e5ccb1800..64f1cbdab9f 100644
--- a/add-menu.h
+++ b/add-menu.h
@@ -1,3 +1,7 @@ 
+#include "color.h"
+
+typedef char clean_color_settings[][COLOR_MAXLEN];
+
 #define MENU_OPTS_SINGLETON		01
 #define MENU_OPTS_IMMEDIATE		02
 #define MENU_OPTS_LIST_ONLY		04
@@ -35,7 +39,7 @@  struct menu_stuff {
 	void *stuff;
 };
 
-void clean_print_color(enum color_clean ix);
+void clean_print_color(enum color_clean ix, clean_color_settings *clean_colors, int *clean_use_color);
 
 /*
  * Implement a git-add-interactive compatible UI, which is borrowed
@@ -48,4 +52,4 @@  void clean_print_color(enum color_clean ix);
  *   - The array ends with EOF.
  *   - If user pressed CTRL-D (i.e. EOF), no selection returned.
  */
-int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff, void (*prompt_help_cmd)(int));
\ No newline at end of file
+int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff, clean_color_settings *clean_colors, int *clean_use_color, void (*prompt_help_cmd)(int));
diff --git a/builtin/clean.c b/builtin/clean.c
index 5466636e666..ef220869851 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -19,6 +19,7 @@ 
 #include "pathspec.h"
 #include "help.h"
 #include "prompt.h"
+#include "add-menu.h"
 
 static int force = -1; /* unset */
 static int interactive;
@@ -39,14 +40,6 @@  static const char *msg_warn_lstat_failed = N_("could not lstat %s\n");
 static const char *msg_skip_cwd = N_("Refusing to remove current working directory\n");
 static const char *msg_would_skip_cwd = N_("Would refuse to remove current working directory\n");
 
-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 const char *color_interactive_slots[] = {
 	[CLEAN_COLOR_ERROR]  = "error",
@@ -58,7 +51,7 @@  static const char *color_interactive_slots[] = {
 };
 
 static int clean_use_color = -1;
-static char clean_colors[][COLOR_MAXLEN] = {
+static clean_color_settings clean_colors = {
 	[CLEAN_COLOR_ERROR] = GIT_COLOR_BOLD_RED,
 	[CLEAN_COLOR_HEADER] = GIT_COLOR_BOLD,
 	[CLEAN_COLOR_HELP] = GIT_COLOR_BOLD_RED,
@@ -67,36 +60,8 @@  static char clean_colors[][COLOR_MAXLEN] = {
 	[CLEAN_COLOR_RESET] = GIT_COLOR_RESET,
 };
 
-#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 flags;
-};
-
 #define MENU_RETURN_NO_LOOP		10
 
-struct menu_item {
-	char hotkey;
-	const char *title;
-	int selected;
-	int (*fn)(void);
-};
-
-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;
-};
-
 define_list_config_array(color_interactive_slots);
 
 static int git_clean_config(const char *var, const char *value, void *cb)
@@ -130,18 +95,6 @@  static int git_clean_config(const char *var, const char *value, void *cb)
 	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)
 {
 	struct string_list *exclude_list = opt->value;
@@ -307,21 +260,9 @@  static void pretty_print_dels(void)
 	string_list_clear(&list, 0);
 }
 
-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);
+	clean_print_color(CLEAN_COLOR_HELP, &clean_colors, &clean_use_color);
 	printf(singleton ?
 		  _("Prompt help:\n"
 		    "1          - select a numbered item\n"
@@ -335,329 +276,7 @@  static void prompt_help_cmd(int singleton)
 		    "-...       - unselect specified items\n"
 		    "*          - choose all items\n"
 		    "           - (empty) finish selecting\n"));
-	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)
-{
-	struct string_list menu_list = STRING_LIST_INIT_DUP;
-	struct strbuf menu = STRBUF_INIT;
-	struct menu_item *menu_item;
-	struct string_list_item *string_list_item;
-	int i;
-
-	switch (stuff->type) {
-	default:
-		die("Bad type of menu_stuff when print menu");
-	case MENU_STUFF_TYPE_MENU_ITEM:
-		menu_item = (struct menu_item *)stuff->stuff;
-		for (i = 0; i < stuff->nr; i++, menu_item++) {
-			const char *p;
-			int highlighted = 0;
-
-			p = menu_item->title;
-			if ((*chosen)[i] < 0)
-				(*chosen)[i] = menu_item->selected ? 1 : 0;
-			strbuf_addf(&menu, "%s%2d: ", (*chosen)[i] ? "*" : " ", i+1);
-			for (; *p; p++) {
-				if (!highlighted && *p == menu_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);
-		}
-		break;
-	case MENU_STUFF_TYPE_STRING_LIST:
-		i = 0;
-		for_each_string_list_item(string_list_item, (struct string_list *)stuff->stuff) {
-			if ((*chosen)[i] < 0)
-				(*chosen)[i] = 0;
-			strbuf_addf(&menu, "%s%2d: %s",
-				    (*chosen)[i] ? "*" : " ", i+1, string_list_item->string);
-			string_list_append(&menu_list, menu.buf);
-			strbuf_reset(&menu);
-			i++;
-		}
-		break;
-	}
-
-	pretty_print_menus(&menu_list);
-
-	strbuf_release(&menu);
-	string_list_clear(&menu_list, 0);
-}
-
-static int find_unique(const char *choice, struct menu_stuff *menu_stuff)
-{
-	struct menu_item *menu_item;
-	struct string_list_item *string_list_item;
-	int i, len, found = 0;
-
-	len = strlen(choice);
-	switch (menu_stuff->type) {
-	default:
-		die("Bad type of menu_stuff when parse choice");
-	case MENU_STUFF_TYPE_MENU_ITEM:
-
-		menu_item = (struct menu_item *)menu_stuff->stuff;
-		for (i = 0; i < menu_stuff->nr; i++, menu_item++) {
-			if (len == 1 && *choice == menu_item->hotkey) {
-				found = i + 1;
-				break;
-			}
-			if (!strncasecmp(choice, menu_item->title, len)) {
-				if (found) {
-					if (len == 1) {
-						/* continue for hotkey matching */
-						found = -1;
-					} else {
-						found = 0;
-						break;
-					}
-				} else {
-					found = i + 1;
-				}
-			}
-		}
-		break;
-	case MENU_STUFF_TYPE_STRING_LIST:
-		string_list_item = ((struct string_list *)menu_stuff->stuff)->items;
-		for (i = 0; i < menu_stuff->nr; i++, string_list_item++) {
-			if (!strncasecmp(choice, string_list_item->string, len)) {
-				if (found) {
-					found = 0;
-					break;
-				}
-				found = i + 1;
-			}
-		}
-		break;
-	}
-	return found;
-}
-
-/*
- * 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, **ptr;
-	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 (ptr = choice_list; *ptr; ptr++) {
-		char *p;
-		int choose = 1;
-		int bottom = 0, top = 0;
-		int is_range, is_number;
-
-		strbuf_trim(*ptr);
-		if (!(*ptr)->len)
-			continue;
-
-		/* Input that begins with '-'; unchoose */
-		if (*(*ptr)->buf == '-') {
-			choose = 0;
-			strbuf_remove((*ptr), 0, 1);
-		}
-
-		is_range = 0;
-		is_number = 1;
-		for (p = (*ptr)->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((*ptr)->buf);
-			top = bottom;
-		} else if (is_range) {
-			bottom = atoi((*ptr)->buf);
-			/* a range can be specified like 5-7 or 5- */
-			if (!*(strchr((*ptr)->buf, '-') + 1))
-				top = menu_stuff->nr;
-			else
-				top = atoi(strchr((*ptr)->buf, '-') + 1);
-		} else if (!strcmp((*ptr)->buf, "*")) {
-			bottom = 1;
-			top = menu_stuff->nr;
-		} else {
-			bottom = find_unique((*ptr)->buf, menu_stuff);
-			top = bottom;
-		}
-
-		if (top <= 0 || bottom <= 0 || top > menu_stuff->nr || bottom > top ||
-		    (is_single && bottom != top)) {
-			clean_print_color(CLEAN_COLOR_ERROR);
-			printf(_("Huh (%s)?\n"), (*ptr)->buf);
-			clean_print_color(CLEAN_COLOR_RESET);
-			continue;
-		}
-
-		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;
-
-	ALLOC_ARRAY(chosen, 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));
-		}
-
-		/* chosen will be initialized by print_highlight_menu_stuff */
-		print_highlight_menu_stuff(stuff, &chosen);
-
-		if (opts->flags & MENU_OPTS_LIST_ONLY)
-			break;
-
-		if (opts->prompt) {
-			printf("%s%s%s%s",
-			       clean_get_color(CLEAN_COLOR_PROMPT),
-			       _(opts->prompt),
-			       opts->flags & MENU_OPTS_SINGLETON ? "> " : ">> ",
-			       clean_get_color(CLEAN_COLOR_RESET));
-		}
-
-		if (git_read_line_interactively(&choice) == EOF) {
-			eof = 1;
-			break;
-		}
-
-		/* help for prompt */
-		if (!strcmp(choice.buf, "?")) {
-			prompt_help_cmd(opts->flags & MENU_OPTS_SINGLETON);
-			continue;
-		}
-
-		/* for a multiple-choice menu, press ENTER (empty) will return back */
-		if (!(opts->flags & MENU_OPTS_SINGLETON) && !choice.len)
-			break;
-
-		nr = parse_choice(stuff,
-				  opts->flags & MENU_OPTS_SINGLETON,
-				  choice,
-				  &chosen);
-
-		if (opts->flags & MENU_OPTS_SINGLETON) {
-			if (nr)
-				break;
-		} else if (opts->flags & MENU_OPTS_IMMEDIATE) {
-			break;
-		}
-	}
-
-	if (eof) {
-		result = xmalloc(sizeof(int));
-		*result = EOF;
-	} else {
-		int j = 0;
-
-		/*
-		 * recalculate nr, if return back from menu directly with
-		 * default selections.
-		 */
-		if (!nr) {
-			for (i = 0; i < stuff->nr; i++)
-				nr += chosen[i];
-		}
-
-		CALLOC_ARRAY(result, st_add(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;
+	clean_print_color(CLEAN_COLOR_RESET, &clean_colors, &clean_use_color);
 }
 
 static int clean_cmd(void)
@@ -681,9 +300,9 @@  static int filter_by_patterns_cmd(void)
 		if (changed)
 			pretty_print_dels();
 
-		clean_print_color(CLEAN_COLOR_PROMPT);
+		clean_print_color(CLEAN_COLOR_PROMPT, &clean_colors, &clean_use_color);
 		printf(_("Input ignore patterns>> "));
-		clean_print_color(CLEAN_COLOR_RESET);
+		clean_print_color(CLEAN_COLOR_RESET, &clean_colors, &clean_use_color);
 		if (git_read_line_interactively(&confirm) == EOF)
 			putchar('\n');
 
@@ -715,9 +334,9 @@  static int filter_by_patterns_cmd(void)
 		if (changed) {
 			string_list_remove_empty_items(&del_list, 0);
 		} else {
-			clean_print_color(CLEAN_COLOR_ERROR);
+			clean_print_color(CLEAN_COLOR_ERROR, &clean_colors, &clean_use_color);
 			printf_ln(_("WARNING: Cannot find items matched by: %s"), confirm.buf);
-			clean_print_color(CLEAN_COLOR_RESET);
+			clean_print_color(CLEAN_COLOR_RESET, &clean_colors, &clean_use_color);
 		}
 
 		strbuf_list_free(ignore_list);
@@ -744,7 +363,7 @@  static int select_by_numbers_cmd(void)
 	menu_stuff.stuff = &del_list;
 	menu_stuff.nr = del_list.nr;
 
-	chosen = list_and_choose(&menu_opts, &menu_stuff);
+	chosen = list_and_choose(&menu_opts, &menu_stuff, &clean_colors, &clean_use_color, prompt_help_cmd);
 	items = del_list.items;
 	for (i = 0, j = 0; i < del_list.nr; i++) {
 		if (i < chosen[j]) {
@@ -807,7 +426,7 @@  static int quit_cmd(void)
 
 static int help_cmd(void)
 {
-	clean_print_color(CLEAN_COLOR_HELP);
+	clean_print_color(CLEAN_COLOR_HELP, &clean_colors, &clean_use_color);
 	printf_ln(_(
 		    "clean               - start cleaning\n"
 		    "filter by pattern   - exclude items from deletion\n"
@@ -817,7 +436,7 @@  static int help_cmd(void)
 		    "help                - this screen\n"
 		    "?                   - help for prompt selection"
 		   ));
-	clean_print_color(CLEAN_COLOR_RESET);
+	clean_print_color(CLEAN_COLOR_RESET, &clean_colors, &clean_use_color);
 	return 0;
 }
 
@@ -844,15 +463,15 @@  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);
+		clean_print_color(CLEAN_COLOR_HEADER, &clean_colors, &clean_use_color);
 		printf_ln(Q_("Would remove the following item:",
 			     "Would remove the following items:",
 			     del_list.nr));
-		clean_print_color(CLEAN_COLOR_RESET);
+		clean_print_color(CLEAN_COLOR_RESET, &clean_colors, &clean_use_color);
 
 		pretty_print_dels();
 
-		chosen = list_and_choose(&menu_opts, &menu_stuff);
+		chosen = list_and_choose(&menu_opts, &menu_stuff, &clean_colors, &clean_use_color, prompt_help_cmd);
 
 		if (*chosen != EOF) {
 			int ret;
@@ -860,9 +479,9 @@  static void interactive_main_loop(void)
 			if (ret != MENU_RETURN_NO_LOOP) {
 				FREE_AND_NULL(chosen);
 				if (!del_list.nr) {
-					clean_print_color(CLEAN_COLOR_ERROR);
+					clean_print_color(CLEAN_COLOR_ERROR, &clean_colors, &clean_use_color);
 					printf_ln(_("No more files to clean, exiting."));
-					clean_print_color(CLEAN_COLOR_RESET);
+					clean_print_color(CLEAN_COLOR_RESET, &clean_colors, &clean_use_color);
 					break;
 				}
 				continue;