pretty.c: Make user defined format honor color option
PATCH 1/1 pretty.c: Make user defined format honor color option
This patch fixes that the pretty-formats tformat and format ignore git's color option.
--- commit.h | 1 + log-tree.c | 1 + pretty.c | 29 +++++++++++++++++------------ 3 files changed, 19 insertions(+), 12 deletions(-)
commit.h +1 -0
@@ -78,6 +78,7 @@ struct pretty_print_contextint show_notes;struct reflog_walk_info *reflog_info;const char *output_encoding; + unsigned colorize:1; };struct userformat_want {
log-tree.c +1 -0
@@ -505,6 +505,7 @@ void show_log(struct rev_info *opt) ctx.abbrev = opt->diffopt.abbrev; ctx.after_subject = extra_headers; ctx.reflog_info = opt->reflog_info; + ctx.colorize = (DIFF_OPT_TST (&opt->diffopt,COLOR_DIFF ) !=0 );pretty_print_commit (opt->commit_format, commit, &msgbuf, &ctx);if (opt->add_signoff)
pretty.c +17 -12
@@ -743,7 +743,10 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,struct format_commit_context *c = context;const struct commit *commit = c->commit;const char *msg = c->message; +char cbuf[COLOR_MAXLEN ];struct commit_list *p; +size_t consumed =0 ; +char *color =NULL ;int h1, h2;/* these are independent of the commit */
@@ -751,29 +754,31 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,case 'C' :if (placeholder[1 ] =='(' ) {const char *end =strchr (placeholder +2 ,')' ); -char color[COLOR_MAXLEN ];if (!end)return 0 ;color_parse_mem (placeholder +2 , end - (placeholder +2 ), -"--pretty format" , color); -strbuf_addstr (sb, color); -return end - placeholder +1 ; +"--pretty format" , cbuf); + consumed = end - placeholder +1 ; + color = cbuf; }if (!prefixcmp (placeholder +1 ,"red" )) { -strbuf_addstr (sb,GIT_COLOR_RED ); -return 4 ; + color =GIT_COLOR_RED ; + consumed =4 ; }else if (!prefixcmp (placeholder +1 ,"green" )) { -strbuf_addstr (sb,GIT_COLOR_GREEN ); -return 6 ; + color =GIT_COLOR_GREEN ; + consumed =6 ; }else if (!prefixcmp (placeholder +1 ,"blue" )) { -strbuf_addstr (sb,GIT_COLOR_BLUE ); -return 5 ; + color =GIT_COLOR_BLUE ; + consumed =5 ; }else if (!prefixcmp (placeholder +1 ,"reset" )) { -strbuf_addstr (sb,GIT_COLOR_RESET ); -return 6 ; + color =GIT_COLOR_RESET ; + consumed =6 ; }else return 0 ; +if (color && (c->pretty_ctx->colorize ==1 )) +strbuf_addstr (sb, color); +return consumed;case 'n' :/* newline */ strbuf_addch (sb,'\n' );return 1 ;
Quoted 2 lines · Thomas Egerer
This patch fixes that the pretty-formats tformat and format ignore git's color option.
It is my understanding that this is intentional, the logic being: If you normally don't want color, but have specified it directly on the command-line, you probably want color.
It is arguable that this logic does not hold for format aliases, of course. (and personally I've never agreed with the above logic anyway)
iirc, there are a couple of other places beyond log-tree.c which need to propagate COLOR_DIFF into the pretty context if you want to respect the colour option in user-specified formats. Skimming my own diffs:rev-list.c and shortlog.c
Of course, explicitly propagating "diff_opts" settings like this just shows off how poorly named these things are if we're going to use them for this sort of thing. That's probably a place for another patch, though.
I've got a patch to add support for conditional formats, including such things as: %(opt-color: %Cred%h%Creset, %h). I'm currently in the process of re-rolling just the "long options" part of that into something more-manageable, but honestly that's the bulk of the change, so if that gets accepted then adding %(opt-color...) would be pretty trivial.
--Will
On 03/17/2011 10:39 AM, Will Palmer schrobtete:
Quoted 7 lines · Thomas Egerer
Quoted 2 lines
This patch fixes that the pretty-formats tformat and format ignore git's color option.
It is my understanding that this is intentional, the logic being: If you normally don't want color, but have specified it directly on the command-line, you probably want color.
I'm using the pretty format in the context of an alias. My global setting for colors is auto. I would expect git to not disregard this options. I usually use the alias to display a git log in a modified way, but I also do sometimes pipe it to grep. If there was a way to suppress output colorization (let's say by not using global options but the command line switch --color=never) that would work for me. But there is no wa and I find it inconvinient to have two different aliases doing the same thing one with color and one without while there would be a much simpler way.
Quoted 4 lines
iirc, there are a couple of other places beyond
log-tree.cwhich need to propagateCOLOR_DIFFinto the pretty context if you want to respect the colour option in user-specified formats. Skimming my own diffs:rev-list.candshortlog.c
You're right. If there's a chance to bring this upstream, I would include it in a revised versoin of my patch.
Thomas
Quoted 16 lines · Thomas Egerer
On 03/17/2011 10:39 AM, Will Palmer schrobtete:
Quoted 7 lines · Thomas Egerer
Quoted 2 lines
This patch fixes that the pretty-formats tformat and format ignore git's color option.
It is my understanding that this is intentional, the logic being: If you normally don't want color, but have specified it directly on the command-line, you probably want color.
I'm using the pretty format in the context of an alias. My global setting for colors is auto. I would expect git to not disregard this options. I usually use the alias to display a git log in a modified way, but I also do sometimes pipe it to grep. If there was a way to suppress output colorization (let's say by not using global options but the command line switch --color=never) that would work for me. But there is no wa and I find it inconvinient to have two different aliases doing the same thing one with color and one without while there would be a much simpler way.
...snip
Quoted 1 line
Thomas
Perhaps --color=auto, specified on the command-line, should behave differently to the various color options specified via config. That might make both sides happy, as one could always specify --color=auto to explicitly tell git to only color if it thinks it should.
Can anyone else refresh my memory regarding the use-case where hand-specified colors really should have an effect even with --color=never?
Quoted 8 lines · Will Palmer
Perhaps --color=auto, specified on the command-line, should behave differently to the various color options specified via config. That might make both sides happy, as one could always specify --color=auto to explicitly tell git to only color if it thinks it should.
Can anyone else refresh my memory regarding the use-case where hand-specified colors really should have an effect even with --color=never?
Without doing any digging on the list and just from the top of my head, I don't think it was ever really an intentional feature that format should ignore color settings. It simply wasn't bothered with because in the beginning, the only way to specify a format was on the command line. Now as we see them used in aliases, it probably makes sense to respect the color setting. If one wants the current behavior, they can always use --color=always.
So I think the intent of Thomas' patch is a good change, though I haven't really looked closely at the patch itself.
-Peff
This patch fixes that the pretty-formats tformat and format ignore git's color option.
--- builtin/log.c | 2 +- builtin/rev-list.c | 1 + builtin/shortlog.c | 5 +++-- commit.h | 1 + log-tree.c | 1 + pretty.c | 29 +++++++++++++++++------------ shortlog.h | 2 +- 7 files changed, 25 insertions(+), 16 deletions(-)
0001-pretty.c-Make-user-defined-format-honor-color-option.patch
builtin/log.c +1 -1
@@ -780,7 +780,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, log.in1 =2 ; log.in2 =4 ;for (i =0 ; i < nr; i++) -shortlog_add_commit (&log, list[i]); +shortlog_add_commit (rev, &log, list[i]);shortlog_output (&log);
builtin/rev-list.c +1 -0
@@ -108,6 +108,7 @@ static void show_commit(struct commit *commit, void *data)struct pretty_print_context ctx = {0 }; ctx.abbrev = revs->abbrev; ctx.date_mode = revs->date_mode; + ctx.colorize = (DIFF_OPT_TST (&revs->diffopt,COLOR_DIFF ) !=0 );pretty_print_commit (revs->commit_format, commit, &buf, &ctx);if (revs->graph) {if (buf.len) {
builtin/shortlog.c +3 -2
@@ -136,7 +136,7 @@ static void read_from_stdin(struct shortlog *log) } } -void shortlog_add_commit (struct shortlog *log,struct commit *commit) +void shortlog_add_commit (struct rev_info * rev,struct shortlog *log,struct commit *commit) {const char *author =NULL , *buffer;struct strbuf buf =STRBUF_INIT ;
@@ -166,6 +166,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit) ctx.subject ="" ; ctx.after_subject ="" ; ctx.date_mode =DATE_NORMAL ; + ctx.colorize = (DIFF_OPT_TST (&rev->diffopt,COLOR_DIFF ) !=0 );pretty_print_commit (CMIT_FMT_USERFORMAT , commit, &ufbuf, &ctx); buffer = ufbuf.buf; }else if (*buffer) {
@@ -183,7 +184,7 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)if (prepare_revision_walk (rev))die ("revision walk setup failed" );while ((commit =get_revision (rev)) !=NULL ) -shortlog_add_commit (log, commit); +shortlog_add_commit (rev, log, commit); }static int parse_uint (char const **arg,int comma,int defval)
commit.h +1 -0
@@ -78,6 +78,7 @@ struct pretty_print_contextint show_notes;struct reflog_walk_info *reflog_info;const char *output_encoding; + unsigned colorize:1; };struct userformat_want {
log-tree.c +1 -0
@@ -505,6 +505,7 @@ void show_log(struct rev_info *opt) ctx.abbrev = opt->diffopt.abbrev; ctx.after_subject = extra_headers; ctx.reflog_info = opt->reflog_info; + ctx.colorize = (DIFF_OPT_TST (&opt->diffopt,COLOR_DIFF ) !=0 );pretty_print_commit (opt->commit_format, commit, &msgbuf, &ctx); if (opt->add_signoff)This patch fixes that the pretty-formats tformat and format ignore
git's color option.
--- builtin/log.c | 2 +- builtin/rev-list.c | 1 + builtin/shortlog.c | 5 +++-- commit.h | 1 + log-tree.c | 1 + pretty.c | 29 +++++++++++++++++------------ shortlog.h | 2 +- 7 files changed, 25 insertions(+), 16 deletions(-)
0001-pretty.c-Make-user-defined-format-honor-color-option.patch
builtin/log.c +1 -1
@@ -780,7 +780,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, log.in1 =2 ; log.in2 =4 ;for (i =0 ; i < nr; i++) -shortlog_add_commit (&log, list[i]); +shortlog_add_commit (rev, &log, list[i]);shortlog_output (&log);
builtin/rev-list.c +1 -0
@@ -108,6 +108,7 @@ static void show_commit(struct commit *commit, void *data)struct pretty_print_context ctx = {0 }; ctx.abbrev = revs->abbrev; ctx.date_mode = revs->date_mode; + ctx.colorize = (DIFF_OPT_TST (&revs->diffopt,COLOR_DIFF ) !=0 );pretty_print_commit (revs->commit_format, commit, &buf, &ctx);if (revs->graph) {if (buf.len) {
builtin/shortlog.c +3 -2
@@ -136,7 +136,7 @@ static void read_from_stdin(struct shortlog *log) } } -void shortlog_add_commit (struct shortlog *log,struct commit *commit) +void shortlog_add_commit (struct rev_info * rev,struct shortlog *log,struct commit *commit) {const char *author =NULL , *buffer;struct strbuf buf =STRBUF_INIT ;
@@ -166,6 +166,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit) ctx.subject ="" ; ctx.after_subject ="" ; ctx.date_mode =DATE_NORMAL ; + ctx.colorize = (DIFF_OPT_TST (&rev->diffopt,COLOR_DIFF ) !=0 );pretty_print_commit (CMIT_FMT_USERFORMAT , commit, &ufbuf, &ctx); buffer = ufbuf.buf; }else if (*buffer) {
@@ -183,7 +184,7 @@ static void get_from_rev(struct rev_info *rev, struct shortlog *log)if (prepare_revision_walk (rev))die ("revision walk setup failed" );while ((commit =get_revision (rev)) !=NULL ) -shortlog_add_commit (log, commit); +shortlog_add_commit (rev, log, commit); }static int parse_uint (char const **arg,int comma,int defval)
commit.h +1 -0
@@ -78,6 +78,7 @@ struct pretty_print_contextint show_notes;struct reflog_walk_info *reflog_info;const char *output_encoding; + unsigned colorize:1; };struct userformat_want {
log-tree.c +1 -0
@@ -505,6 +505,7 @@ void show_log(struct rev_info *opt) ctx.abbrev = opt->diffopt.abbrev; ctx.after_subject = extra_headers; ctx.reflog_info = opt->reflog_info; + ctx.colorize = (DIFF_OPT_TST (&opt->diffopt,COLOR_DIFF ) !=0 );pretty_print_commit (opt->commit_format, commit, &msgbuf, &ctx);if (opt->add_signoff)
pretty.c +17 -12
@@ -743,7 +743,10 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,struct format_commit_context *c = context;const struct commit *commit = c->commit;const char *msg = c->message; +char cbuf[COLOR_MAXLEN ];struct commit_list *p; +size_t consumed =0 ; +char *color =NULL ;int h1, h2;/* these are independent of the commit */
@@ -751,29 +754,31 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,case 'C' :if (placeholder[1 ] =='(' ) {const char *end =strchr (placeholder +2 ,')' ); -char color[COLOR_MAXLEN ];if (!end)return 0 ;color_parse_mem (placeholder +2 , end - (placeholder +2 ), -"--pretty format" , color); -strbuf_addstr (sb, color); -return end - placeholder +1 ; +"--pretty format" , cbuf); + consumed = end - placeholder +1 ; + color = cbuf; }if (!prefixcmp (placeholder +1 ,"red" )) { -strbuf_addstr (sb,GIT_COLOR_RED ); -return 4 ; + color =GIT_COLOR_RED ; + consumed =4 ; }else if (!prefixcmp (placeholder +1 ,"green" )) { -strbuf_addstr (sb,GIT_COLOR_GREEN ); -return 6 ; + color =GIT_COLOR_GREEN ; + consumed =6 ; }else if (!prefixcmp (placeholder +1 ,"blue" )) { -strbuf_addstr (sb,GIT_COLOR_BLUE ); -return 5 ; + color =GIT_COLOR_BLUE ; + consumed =5 ; }else if (!prefixcmp (placeholder +1 ,"reset" )) { -strbuf_addstr (sb,GIT_COLOR_RESET ); -return 6 ; + color =GIT_COLOR_RESET ; + consumed =6 ; }else return 0 ; +if (color && (c->pretty_ctx->colorize ==1 )) +strbuf_addstr (sb, color); +return consumed;case 'n' :/* newline */ strbuf_addch (sb,'\n' );return 1 ;
shortlog.h +1 -1
@@ -21,7 +21,7 @@ struct shortlog {void shortlog_init (struct shortlog *log); -void shortlog_add_commit (struct shortlog *log,struct commit *commit); +void shortlog_add_commit (struct rev_info *rev,struct shortlog *log,struct commit *commit);void shortlog_output (struct shortlog *log);
pretty.c +17 -12
@@ -743,7 +743,10 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,struct format_commit_context *c = context;const struct commit *commit = c->commit;const char *msg = c->message; +char cbuf[COLOR_MAXLEN ];struct commit_list *p; +size_t consumed =0 ; +char *color =NULL ;int h1, h2;/* these are independent of the commit */
@@ -751,29 +754,31 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,case 'C' :if (placeholder[1 ] =='(' ) {const char *end =strchr (placeholder +2 ,')' ); -char color[COLOR_MAXLEN ];if (!end)return 0 ;color_parse_mem (placeholder +2 , end - (placeholder +2 ), -"--pretty format" , color); -strbuf_addstr (sb, color); -return end - placeholder +1 ; +"--pretty format" , cbuf); + consumed = end - placeholder +1 ; + color = cbuf; }if (!prefixcmp (placeholder +1 ,"red" )) { -strbuf_addstr (sb,GIT_COLOR_RED ); -return 4 ; + color =GIT_COLOR_RED ; + consumed =4 ; }else if (!prefixcmp (placeholder +1 ,"green" )) { -strbuf_addstr (sb,GIT_COLOR_GREEN ); -return 6 ; + color =GIT_COLOR_GREEN ; + consumed =6 ; }else if (!prefixcmp (placeholder +1 ,"blue" )) { -strbuf_addstr (sb,GIT_COLOR_BLUE ); -return 5 ; + color =GIT_COLOR_BLUE ; + consumed =5 ; }else if (!prefixcmp (placeholder +1 ,"reset" )) { -strbuf_addstr (sb,GIT_COLOR_RESET ); -return 6 ; + color =GIT_COLOR_RESET ; + consumed =6 ; }else return 0 ; +if (color && (c->pretty_ctx->colorize ==1 )) +strbuf_addstr (sb, color); +return consumed;case 'n' :/* newline */ strbuf_addch (sb,'\n' );return 1 ;
shortlog.h +1 -1
@@ -21,7 +21,7 @@ struct shortlog {void shortlog_init (struct shortlog *log); -void shortlog_add_commit (struct shortlog *log,struct commit *commit); +void shortlog_add_commit (struct rev_info *rev,struct shortlog *log,struct commit *commit);void shortlog_output (struct shortlog *log);