* [PATCH] Clean up use of ANSI color sequences @ 2009-02-12 20:37 Arjen Laarhoven 2009-02-12 23:03 ` Junio C Hamano 0 siblings, 1 reply; 7+ messages in thread From: Arjen Laarhoven @ 2009-02-12 20:37 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Remove the literal ANSI escape sequences and replace them by readable constants. Signed-off-by: Arjen Laarhoven <arjen@yaph.org> --- builtin-branch.c | 10 +++++----- color.c | 4 +--- color.h | 10 ++++++++++ diff.c | 16 ++++++++-------- pretty.c | 8 ++++---- wt-status.c | 10 +++++----- 6 files changed, 33 insertions(+), 25 deletions(-) diff --git a/builtin-branch.c b/builtin-branch.c index 56a1971..c154500 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -32,11 +32,11 @@ static unsigned char head_sha1[20]; static int branch_use_color = -1; static char branch_colors[][COLOR_MAXLEN] = { - "\033[m", /* reset */ - "", /* PLAIN (normal) */ - "\033[31m", /* REMOTE (red) */ - "", /* LOCAL (normal) */ - "\033[32m", /* CURRENT (green) */ + COLOR_RESET, + COLOR_NORMAL, /* PLAIN */ + COLOR_RED, /* REMOTE */ + COLOR_NORMAL, /* LOCAL */ + COLOR_GREEN, /* CURRENT */ }; enum color_branch { COLOR_BRANCH_RESET = 0, diff --git a/color.c b/color.c index db4dccf..5653667 100644 --- a/color.c +++ b/color.c @@ -1,8 +1,6 @@ #include "cache.h" #include "color.h" -#define COLOR_RESET "\033[m" - int git_use_color_default = 0; static int parse_color(const char *name, int len) @@ -54,7 +52,7 @@ void color_parse_mem(const char *value, int value_len, const char *var, int bg = -2; if (!strncasecmp(value, "reset", len)) { - strcpy(dst, "\033[m"); + strcpy(dst, COLOR_RESET); return; } diff --git a/color.h b/color.h index 5019df8..c4d2e53 100644 --- a/color.h +++ b/color.h @@ -4,6 +4,16 @@ /* "\033[1;38;5;2xx;48;5;2xxm\0" is 23 bytes */ #define COLOR_MAXLEN 24 +#define COLOR_NORMAL "" +#define COLOR_RESET "\033[m" +#define COLOR_BOLD "\033[1m" +#define COLOR_RED "\033[31m" +#define COLOR_GREEN "\033[32m" +#define COLOR_YELLOW "\033[33m" +#define COLOR_BLUE "\033[34m" +#define COLOR_CYAN "\033[36m" +#define COLOR_BG_RED "\033[41m" + /* * This variable stores the value of color.ui */ diff --git a/diff.c b/diff.c index a5a540f..1ca64d3 100644 --- a/diff.c +++ b/diff.c @@ -30,14 +30,14 @@ int diff_auto_refresh_index = 1; static int diff_mnemonic_prefix; static char diff_colors[][COLOR_MAXLEN] = { - "\033[m", /* reset */ - "", /* PLAIN (normal) */ - "\033[1m", /* METAINFO (bold) */ - "\033[36m", /* FRAGINFO (cyan) */ - "\033[31m", /* OLD (red) */ - "\033[32m", /* NEW (green) */ - "\033[33m", /* COMMIT (yellow) */ - "\033[41m", /* WHITESPACE (red background) */ + COLOR_RESET, + COLOR_NORMAL, /* PLAIN */ + COLOR_BOLD, /* METAINFO */ + COLOR_CYAN, /* FRAGINFO */ + COLOR_RED, /* OLD */ + COLOR_GREEN, /* NEW */ + COLOR_YELLOW, /* COMMIT */ + COLOR_BG_RED, /* WHITESPACE */ }; static void diff_filespec_load_driver(struct diff_filespec *one); diff --git a/pretty.c b/pretty.c index cc460b5..a8595f6 100644 --- a/pretty.c +++ b/pretty.c @@ -567,16 +567,16 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, return end - placeholder + 1; } if (!prefixcmp(placeholder + 1, "red")) { - strbuf_addstr(sb, "\033[31m"); + strbuf_addstr(sb, COLOR_RED); return 4; } else if (!prefixcmp(placeholder + 1, "green")) { - strbuf_addstr(sb, "\033[32m"); + strbuf_addstr(sb, COLOR_GREEN); return 6; } else if (!prefixcmp(placeholder + 1, "blue")) { - strbuf_addstr(sb, "\033[34m"); + strbuf_addstr(sb, COLOR_BLUE); return 5; } else if (!prefixcmp(placeholder + 1, "reset")) { - strbuf_addstr(sb, "\033[m"); + strbuf_addstr(sb, COLOR_RESET); return 6; } else return 0; diff --git a/wt-status.c b/wt-status.c index 96ff2f8..432d23a 100644 --- a/wt-status.c +++ b/wt-status.c @@ -15,11 +15,11 @@ int wt_status_relative_paths = 1; int wt_status_use_color = -1; int wt_status_submodule_summary; static char wt_status_colors[][COLOR_MAXLEN] = { - "", /* WT_STATUS_HEADER: normal */ - "\033[32m", /* WT_STATUS_UPDATED: green */ - "\033[31m", /* WT_STATUS_CHANGED: red */ - "\033[31m", /* WT_STATUS_UNTRACKED: red */ - "\033[31m", /* WT_STATUS_NOBRANCH: red */ + COLOR_NORMAL, /* WT_STATUS_HEADER */ + COLOR_GREEN, /* WT_STATUS_UPDATED */ + COLOR_RED, /* WT_STATUS_CHANGED */ + COLOR_RED, /* WT_STATUS_UNTRACKED */ + COLOR_RED, /* WT_STATUS_NOBRANCH */ }; enum untracked_status_type show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES; -- 1.6.2.rc0.186.g417c ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Clean up use of ANSI color sequences 2009-02-12 20:37 [PATCH] Clean up use of ANSI color sequences Arjen Laarhoven @ 2009-02-12 23:03 ` Junio C Hamano 2009-02-13 21:53 ` [PATCH 1/2] " Arjen Laarhoven 0 siblings, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2009-02-12 23:03 UTC (permalink / raw) To: Arjen Laarhoven; +Cc: git Arjen Laarhoven <arjen@yaph.org> writes: > diff --git a/color.h b/color.h > index 5019df8..c4d2e53 100644 > --- a/color.h > +++ b/color.h > @@ -4,6 +4,16 @@ > /* "\033[1;38;5;2xx;48;5;2xxm\0" is 23 bytes */ > #define COLOR_MAXLEN 24 > > +#define COLOR_NORMAL "" > +#define COLOR_RESET "\033[m" > +#define COLOR_BOLD "\033[1m" > +#define COLOR_RED "\033[31m" > +#define COLOR_GREEN "\033[32m" > +#define COLOR_YELLOW "\033[33m" > +#define COLOR_BLUE "\033[34m" > +#define COLOR_CYAN "\033[36m" > +#define COLOR_BG_RED "\033[41m" Sounds like a very sane thing to do in principle, but the choice of constant names are problematic. (1) There are COLOR_BRANCH_$category constants, that look very similar (they probably should be renamed to BRANCH_COLOR_$category). (2) These are ANSI constants so it might be better to call them ANSI_COLOR_$physical_attributes, or GIT_COLOR_$physical_attributes. I actually prefer the latter because then later we can potentially redefine these macros with something like: #define GIT_COLOR_RED ti_setf(COLOR_RED) #define GIT_COLOR_BG_RED ti_setb(COLOR_RED) and write a set of small wrappers to terminfo to support non ANSI terminals without changing the rest of the code. It is nicer to use GIT_COLOR_RED instead of COLOR_RED, because the latter are defined in ncurses.h like this: /* colors */ #define COLOR_BLACK 0 #define COLOR_RED 1 #define COLOR_GREEN 2 #define COLOR_YELLOW 3 #define COLOR_BLUE 4 #define COLOR_MAGENTA 5 #define COLOR_CYAN 6 #define COLOR_WHITE 7 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] Clean up use of ANSI color sequences 2009-02-12 23:03 ` Junio C Hamano @ 2009-02-13 21:53 ` Arjen Laarhoven 2009-02-13 21:53 ` [PATCH 2/2] builtin-branch.c: Rename branch category color names Arjen Laarhoven 2009-02-14 2:02 ` [PATCH 1/2] Clean up use of ANSI color sequences Junio C Hamano 0 siblings, 2 replies; 7+ messages in thread From: Arjen Laarhoven @ 2009-02-13 21:53 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Remove the literal ANSI escape sequences and replace them by readable constants. Signed-off-by: Arjen Laarhoven <arjen@yaph.org> --- builtin-branch.c | 10 +++++----- color.c | 8 +++----- color.h | 10 ++++++++++ diff.c | 16 ++++++++-------- pretty.c | 8 ++++---- wt-status.c | 10 +++++----- 6 files changed, 35 insertions(+), 27 deletions(-) diff --git a/builtin-branch.c b/builtin-branch.c index 56a1971..fe139e1 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -32,11 +32,11 @@ static unsigned char head_sha1[20]; static int branch_use_color = -1; static char branch_colors[][COLOR_MAXLEN] = { - "\033[m", /* reset */ - "", /* PLAIN (normal) */ - "\033[31m", /* REMOTE (red) */ - "", /* LOCAL (normal) */ - "\033[32m", /* CURRENT (green) */ + GIT_COLOR_RESET, + GIT_COLOR_NORMAL, /* PLAIN */ + GIT_COLOR_RED, /* REMOTE */ + GIT_COLOR_NORMAL, /* LOCAL */ + GIT_COLOR_GREEN, /* CURRENT */ }; enum color_branch { COLOR_BRANCH_RESET = 0, diff --git a/color.c b/color.c index db4dccf..62977f4 100644 --- a/color.c +++ b/color.c @@ -1,8 +1,6 @@ #include "cache.h" #include "color.h" -#define COLOR_RESET "\033[m" - int git_use_color_default = 0; static int parse_color(const char *name, int len) @@ -54,7 +52,7 @@ void color_parse_mem(const char *value, int value_len, const char *var, int bg = -2; if (!strncasecmp(value, "reset", len)) { - strcpy(dst, "\033[m"); + strcpy(dst, GIT_COLOR_RESET); return; } @@ -175,7 +173,7 @@ static int color_vfprintf(FILE *fp, const char *color, const char *fmt, r += fprintf(fp, "%s", color); r += vfprintf(fp, fmt, args); if (*color) - r += fprintf(fp, "%s", COLOR_RESET); + r += fprintf(fp, "%s", GIT_COLOR_RESET); if (trail) r += fprintf(fp, "%s", trail); return r; @@ -217,7 +215,7 @@ int color_fwrite_lines(FILE *fp, const char *color, char *p = memchr(buf, '\n', count); if (p != buf && (fputs(color, fp) < 0 || fwrite(buf, p ? p - buf : count, 1, fp) != 1 || - fputs(COLOR_RESET, fp) < 0)) + fputs(GIT_COLOR_RESET, fp) < 0)) return -1; if (!p) return 0; diff --git a/color.h b/color.h index 5019df8..6846be1 100644 --- a/color.h +++ b/color.h @@ -4,6 +4,16 @@ /* "\033[1;38;5;2xx;48;5;2xxm\0" is 23 bytes */ #define COLOR_MAXLEN 24 +#define GIT_COLOR_NORMAL "" +#define GIT_COLOR_RESET "\033[m" +#define GIT_COLOR_BOLD "\033[1m" +#define GIT_COLOR_RED "\033[31m" +#define GIT_COLOR_GREEN "\033[32m" +#define GIT_COLOR_YELLOW "\033[33m" +#define GIT_COLOR_BLUE "\033[34m" +#define GIT_COLOR_CYAN "\033[36m" +#define GIT_COLOR_BG_RED "\033[41m" + /* * This variable stores the value of color.ui */ diff --git a/diff.c b/diff.c index a5a540f..2513a54 100644 --- a/diff.c +++ b/diff.c @@ -30,14 +30,14 @@ int diff_auto_refresh_index = 1; static int diff_mnemonic_prefix; static char diff_colors[][COLOR_MAXLEN] = { - "\033[m", /* reset */ - "", /* PLAIN (normal) */ - "\033[1m", /* METAINFO (bold) */ - "\033[36m", /* FRAGINFO (cyan) */ - "\033[31m", /* OLD (red) */ - "\033[32m", /* NEW (green) */ - "\033[33m", /* COMMIT (yellow) */ - "\033[41m", /* WHITESPACE (red background) */ + GIT_COLOR_RESET, + GIT_COLOR_NORMAL, /* PLAIN */ + GIT_COLOR_BOLD, /* METAINFO */ + GIT_COLOR_CYAN, /* FRAGINFO */ + GIT_COLOR_RED, /* OLD */ + GIT_COLOR_GREEN, /* NEW */ + GIT_COLOR_YELLOW, /* COMMIT */ + GIT_COLOR_BG_RED, /* WHITESPACE */ }; static void diff_filespec_load_driver(struct diff_filespec *one); diff --git a/pretty.c b/pretty.c index cc460b5..a8595f6 100644 --- a/pretty.c +++ b/pretty.c @@ -567,16 +567,16 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, return end - placeholder + 1; } if (!prefixcmp(placeholder + 1, "red")) { - strbuf_addstr(sb, "\033[31m"); + strbuf_addstr(sb, COLOR_RED); return 4; } else if (!prefixcmp(placeholder + 1, "green")) { - strbuf_addstr(sb, "\033[32m"); + strbuf_addstr(sb, COLOR_GREEN); return 6; } else if (!prefixcmp(placeholder + 1, "blue")) { - strbuf_addstr(sb, "\033[34m"); + strbuf_addstr(sb, COLOR_BLUE); return 5; } else if (!prefixcmp(placeholder + 1, "reset")) { - strbuf_addstr(sb, "\033[m"); + strbuf_addstr(sb, COLOR_RESET); return 6; } else return 0; diff --git a/wt-status.c b/wt-status.c index 96ff2f8..dd87339 100644 --- a/wt-status.c +++ b/wt-status.c @@ -15,11 +15,11 @@ int wt_status_relative_paths = 1; int wt_status_use_color = -1; int wt_status_submodule_summary; static char wt_status_colors[][COLOR_MAXLEN] = { - "", /* WT_STATUS_HEADER: normal */ - "\033[32m", /* WT_STATUS_UPDATED: green */ - "\033[31m", /* WT_STATUS_CHANGED: red */ - "\033[31m", /* WT_STATUS_UNTRACKED: red */ - "\033[31m", /* WT_STATUS_NOBRANCH: red */ + GIT_COLOR_NORMAL, /* WT_STATUS_HEADER */ + GIT_COLOR_GREEN, /* WT_STATUS_UPDATED */ + GIT_COLOR_RED, /* WT_STATUS_CHANGED */ + GIT_COLOR_RED, /* WT_STATUS_UNTRACKED */ + GIT_COLOR_RED, /* WT_STATUS_NOBRANCH */ }; enum untracked_status_type show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES; -- 1.6.2.rc0.186.g417c ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] builtin-branch.c: Rename branch category color names 2009-02-13 21:53 ` [PATCH 1/2] " Arjen Laarhoven @ 2009-02-13 21:53 ` Arjen Laarhoven 2009-02-14 2:02 ` [PATCH 1/2] Clean up use of ANSI color sequences Junio C Hamano 1 sibling, 0 replies; 7+ messages in thread From: Arjen Laarhoven @ 2009-02-13 21:53 UTC (permalink / raw) To: Junio C Hamano; +Cc: git The branch color constants have the form COLOR_BRANCH_$category. Rename them to BRANCH_COLOR_$category as this conveys their meaning better. Signed-off-by: Arjen Laarhoven <arjen@yaph.org> --- builtin-branch.c | 32 ++++++++++++++++---------------- 1 files changed, 16 insertions(+), 16 deletions(-) diff --git a/builtin-branch.c b/builtin-branch.c index fe139e1..6d241c8 100644 --- a/builtin-branch.c +++ b/builtin-branch.c @@ -39,11 +39,11 @@ static char branch_colors[][COLOR_MAXLEN] = { GIT_COLOR_GREEN, /* CURRENT */ }; enum color_branch { - COLOR_BRANCH_RESET = 0, - COLOR_BRANCH_PLAIN = 1, - COLOR_BRANCH_REMOTE = 2, - COLOR_BRANCH_LOCAL = 3, - COLOR_BRANCH_CURRENT = 4, + BRANCH_COLOR_RESET = 0, + BRANCH_COLOR_PLAIN = 1, + BRANCH_COLOR_REMOTE = 2, + BRANCH_COLOR_LOCAL = 3, + BRANCH_COLOR_CURRENT = 4, }; static enum merge_filter { @@ -56,15 +56,15 @@ static unsigned char merge_filter_ref[20]; static int parse_branch_color_slot(const char *var, int ofs) { if (!strcasecmp(var+ofs, "plain")) - return COLOR_BRANCH_PLAIN; + return BRANCH_COLOR_PLAIN; if (!strcasecmp(var+ofs, "reset")) - return COLOR_BRANCH_RESET; + return BRANCH_COLOR_RESET; if (!strcasecmp(var+ofs, "remote")) - return COLOR_BRANCH_REMOTE; + return BRANCH_COLOR_REMOTE; if (!strcasecmp(var+ofs, "local")) - return COLOR_BRANCH_LOCAL; + return BRANCH_COLOR_LOCAL; if (!strcasecmp(var+ofs, "current")) - return COLOR_BRANCH_CURRENT; + return BRANCH_COLOR_CURRENT; die("bad config variable '%s'", var); } @@ -303,20 +303,20 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, switch (item->kind) { case REF_LOCAL_BRANCH: - color = COLOR_BRANCH_LOCAL; + color = BRANCH_COLOR_LOCAL; break; case REF_REMOTE_BRANCH: - color = COLOR_BRANCH_REMOTE; + color = BRANCH_COLOR_REMOTE; break; default: - color = COLOR_BRANCH_PLAIN; + color = BRANCH_COLOR_PLAIN; break; } c = ' '; if (current) { c = '*'; - color = COLOR_BRANCH_CURRENT; + color = BRANCH_COLOR_CURRENT; } if (verbose) { @@ -335,14 +335,14 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose, printf("%c %s%-*s%s %s %s%s\n", c, branch_get_color(color), maxwidth, item->name, - branch_get_color(COLOR_BRANCH_RESET), + branch_get_color(BRANCH_COLOR_RESET), find_unique_abbrev(item->commit->object.sha1, abbrev), stat.buf, sub); strbuf_release(&stat); strbuf_release(&subject); } else { printf("%c %s%s%s\n", c, branch_get_color(color), item->name, - branch_get_color(COLOR_BRANCH_RESET)); + branch_get_color(BRANCH_COLOR_RESET)); } } -- 1.6.2.rc0.186.g417c ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Clean up use of ANSI color sequences 2009-02-13 21:53 ` [PATCH 1/2] " Arjen Laarhoven 2009-02-13 21:53 ` [PATCH 2/2] builtin-branch.c: Rename branch category color names Arjen Laarhoven @ 2009-02-14 2:02 ` Junio C Hamano 2009-02-14 7:41 ` Arjen Laarhoven 1 sibling, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2009-02-14 2:02 UTC (permalink / raw) To: Arjen Laarhoven; +Cc: git Arjen Laarhoven <arjen@yaph.org> writes: > diff --git a/pretty.c b/pretty.c > index cc460b5..a8595f6 100644 > --- a/pretty.c > +++ b/pretty.c > @@ -567,16 +567,16 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder, > return end - placeholder + 1; > } > if (!prefixcmp(placeholder + 1, "red")) { > - strbuf_addstr(sb, "\033[31m"); > + strbuf_addstr(sb, COLOR_RED); > return 4; > } else if (!prefixcmp(placeholder + 1, "green")) { > - strbuf_addstr(sb, "\033[32m"); > + strbuf_addstr(sb, COLOR_GREEN); > return 6; > } else if (!prefixcmp(placeholder + 1, "blue")) { > - strbuf_addstr(sb, "\033[34m"); > + strbuf_addstr(sb, COLOR_BLUE); > return 5; > } else if (!prefixcmp(placeholder + 1, "reset")) { > - strbuf_addstr(sb, "\033[m"); > + strbuf_addstr(sb, COLOR_RESET); These four are obviously bad and not even compile tested. I'll fix them up when queuing. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Clean up use of ANSI color sequences 2009-02-14 2:02 ` [PATCH 1/2] Clean up use of ANSI color sequences Junio C Hamano @ 2009-02-14 7:41 ` Arjen Laarhoven 2009-02-14 7:53 ` Junio C Hamano 0 siblings, 1 reply; 7+ messages in thread From: Arjen Laarhoven @ 2009-02-14 7:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Fri, Feb 13, 2009 at 06:02:51PM -0800, Junio C Hamano wrote: [...] > These four are obviously bad and not even compile tested. > > I'll fix them up when queuing. Grmbl. Note to self: never patch and watch TV. Sorry about this. -- Arjen Laarhoven The presence of those seeking the truth is infinitely to be preferred to those who think they've found it. -- Terry Pratchett, "Monstrous Regiment" ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Clean up use of ANSI color sequences 2009-02-14 7:41 ` Arjen Laarhoven @ 2009-02-14 7:53 ` Junio C Hamano 0 siblings, 0 replies; 7+ messages in thread From: Junio C Hamano @ 2009-02-14 7:53 UTC (permalink / raw) To: Arjen Laarhoven; +Cc: git arjen@yaph.org (Arjen Laarhoven) writes: > On Fri, Feb 13, 2009 at 06:02:51PM -0800, Junio C Hamano wrote: > > [...] > >> These four are obviously bad and not even compile tested. >> >> I'll fix them up when queuing. > > Grmbl. Note to self: never patch and watch TV. Sorry about this. That's Ok, mistakes happen. And I sometimes watch TV while my machine is doing all the work for me ;-) ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-02-14 8:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-02-12 20:37 [PATCH] Clean up use of ANSI color sequences Arjen Laarhoven 2009-02-12 23:03 ` Junio C Hamano 2009-02-13 21:53 ` [PATCH 1/2] " Arjen Laarhoven 2009-02-13 21:53 ` [PATCH 2/2] builtin-branch.c: Rename branch category color names Arjen Laarhoven 2009-02-14 2:02 ` [PATCH 1/2] Clean up use of ANSI color sequences Junio C Hamano 2009-02-14 7:41 ` Arjen Laarhoven 2009-02-14 7:53 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).