Re: [PATCH 2/2] pretty: support placeholders %C+ and %C-
On Fri, Sep 21, 2012 at 12:47 AM, Junio C Hamano gits...@pobox.com wrote: +- '%C+': enable coloring on the following placeholders if supported +- '%C-': disable coloring on the following placeholders OK, so typically you replace some format placeholder %? in your format string with %C+%?%C-, because you cannot get away with replacing it with %C+%? and other things in the format you do not know if they support coloring%C-. If that is the case, does it really make sense to have %C-? %C+%? should work. if %? does not support coloring, the %C+ effect is simply ignored. In my use case, I don' really use %C- because I always want color wherever possible. Though I suspect a user might want to turn off coloring for certain part of the format string. Replacing every %? with %C+%?%C- is really annoying in my always color case.. It smells as if it makes more sense to make _all_ %? placeholder reset the effect of %C+ after they are done (even the ones that they themselves do not color their own output elements), so that you can mechanically replace %? with %C+%?. .. or even %C+%?. My format string would become %C+%h %C+%s%C+%d, much harder to read. Thinking about this a bit more, perhaps we would want a generic mechanism to give parameters to various %? placeholders. This is not limited to I can do color but there is no mechanism for the user to tell me that I should do color %H, %h and %d may want to say. An obvious and immediate example is that %h might want to be told how many hexdigits it should use. Yeah that'd be nice. We already use %?(..) for %C. Maybe we can generalize that. Still I'd like a way to define attributes for a group of placeholders instead of just individuals. Continuing with the %?(...) syntax above for specifying attributes for a specific placeholder, %(...) may be used to specify global attributes that affect all following placeholders until another %(...) stops the effect, or %?(...) overrides it. -- Duy -- 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 2/2] pretty: support placeholders %C+ and %C-
%C+ tells the next specifiers that color is preferred. %C- the
opposite. So far only %H, %h and %d support coloring.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/pretty-formats.txt | 2 ++
pretty.c | 13 -
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index e3d8a83..6e287d6 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -142,6 +142,8 @@ The placeholders are:
- '%Cblue': switch color to blue
- '%Creset': reset color
- '%C(...)': color specification, as described in color.branch.* config option
+- '%C+': enable coloring on the following placeholders if supported
+- '%C-': disable coloring on the following placeholders
- '%m': left, right or boundary mark
- '%n': newline
- '%%': a raw '%'
diff --git a/pretty.c b/pretty.c
index e910679..b1cec71 100644
--- a/pretty.c
+++ b/pretty.c
@@ -623,6 +623,7 @@ struct format_commit_context {
unsigned commit_header_parsed:1;
unsigned commit_message_parsed:1;
unsigned commit_signature_parsed:1;
+ unsigned use_color:1;
struct {
char *gpg_output;
char good_bad;
@@ -885,6 +886,12 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
--pretty format, color);
strbuf_addstr(sb, color);
return end - placeholder + 1;
+ } else if (placeholder[1] == '+') {
+ c-use_color = 1;
+ return 2;
+ } else if (placeholder[1] == '-') {
+ c-use_color = 0;
+ return 2;
}
if (!prefixcmp(placeholder + 1, red)) {
strbuf_addstr(sb, GIT_COLOR_RED);
@@ -945,13 +952,17 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
switch (placeholder[0]) {
case 'H': /* commit hash */
+ strbuf_addstr(sb, diff_get_color(c-use_color, DIFF_COMMIT));
strbuf_addstr(sb, sha1_to_hex(commit-object.sha1));
+ strbuf_addstr(sb, diff_get_color(c-use_color, DIFF_RESET));
return 1;
case 'h': /* abbreviated commit hash */
+ strbuf_addstr(sb, diff_get_color(c-use_color, DIFF_COMMIT));
if (add_again(sb, c-abbrev_commit_hash))
return 1;
strbuf_addstr(sb, find_unique_abbrev(commit-object.sha1,
c-pretty_ctx-abbrev));
+ strbuf_addstr(sb, diff_get_color(c-use_color, DIFF_RESET));
c-abbrev_commit_hash.len = sb-len - c-abbrev_commit_hash.off;
return 1;
case 'T': /* tree hash */
@@ -988,7 +999,7 @@ static size_t format_commit_one(struct strbuf *sb, const
char *placeholder,
strbuf_addstr(sb, get_revision_mark(NULL, commit));
return 1;
case 'd':
- format_decoration(sb, commit, 0);
+ format_decoration(sb, commit, c-use_color);
return 1;
case 'g': /* reflog info */
switch(placeholder[1]) {
--
1.7.12.1.383.gda6001e.dirty
--
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 2/2] pretty: support placeholders %C+ and %C-
Junio C Hamano gits...@pobox.com writes: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: %C+ tells the next specifiers that color is preferred. %C- the opposite. So far only %H, %h and %d support coloring. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/pretty-formats.txt | 2 ++ pretty.c | 13 - 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index e3d8a83..6e287d6 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -142,6 +142,8 @@ The placeholders are: - '%Cblue': switch color to blue - '%Creset': reset color - '%C(...)': color specification, as described in color.branch.* config option +- '%C+': enable coloring on the following placeholders if supported +- '%C-': disable coloring on the following placeholders OK, so typically you replace some format placeholder %? in your format string with %C+%?%C-, because you cannot get away with replacing it with %C+%? and other things in the format you do not know if they support coloring%C-. If that is the case, does it really make sense to have %C-? It smells as if it makes more sense to make _all_ %? placeholder reset the effect of %C+ after they are done (even the ones that they themselves do not color their own output elements), so that you can mechanically replace %? with %C+%?. I dunno. Thinking about this a bit more, perhaps we would want a generic mechanism to give parameters to various %? placeholders. This is not limited to I can do color but there is no mechanism for the user to tell me that I should do color %H, %h and %d may want to say. An obvious and immediate example is that %h might want to be told how many hexdigits it should use. -- 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
