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

2013-05-13 Thread Matthieu Moy
Jiang Xin worldhello@gmail.com writes:

 2013/5/13 Matthieu Moy matthieu@grenoble-inp.fr:
 Jiang Xin worldhello@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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-05-12 Thread Matthieu Moy
Jiang Xin worldhello@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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

2013-05-12 Thread Jiang Xin
2013/5/13 Matthieu Moy matthieu@grenoble-inp.fr:
 Jiang Xin worldhello@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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-05-12 Thread Junio C Hamano
Jiang Xin worldhello@gmail.com writes:

 2013/5/13 Matthieu Moy matthieu@grenoble-inp.fr:
 Jiang Xin worldhello@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?

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-05-08 Thread 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@gmail.com
Comments-by: Matthieu Moy matthieu@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 {
+