Re: [PATCH v2 3/3] for-each-ref: introduce %(color:...) for color

[Junio C Hamano]

Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 Perhaps like this (obviously not tested as these three patches did
 not add any tests ;-)

 Sorry about that. I didn't notice t6300-for-each-ref.sh. Will fix in
 the next round.

 I also think that there should be a mechanism to do color:reset
 after each record is issued automatically, and also have the color
 output honor --color=auto from the command line, i.e.

 git for-each-ref --color=auto --format='%(color:blue)%(subject)' | 
 cat

 should turn the coloring off.

 We can add --color=auto later, but I'm wondering about auto-reset
 color after each token. What happens if I do:

   $ git for-each-ref --format='%(subject)%(color:blue)'

 No color, right? So, it should be auto-reset color after each token
 _and_ at end of format-string.

If you are saying, by after each token, that

--format='%(color:blue)%(A)literal string%(B)'

should result in

color blue value for A color reset literal string value for B

then I would disagree.  I was suggesting it to instead produce

color blue value for A literal string value for B color reset

where the color reset always comes when some color is used and we
hit the end of the format string. A bonus point if we can make it so
that we emit the final reset only when the last %(color:some) is
not %(color:reset), but unconditional reset if we ever used
color is fine.

Thanks.

--
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 v2 3/3] for-each-ref: introduce %(color:...) for color

[Ramkumar Ramachandra]

Enhance 'git for-each-ref' with color formatting options.  You can now
use the following format in for-each-ref:

  %(color:green)%(refname:short)%(color:reset)

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Documentation/git-for-each-ref.txt |  4 
 builtin/for-each-ref.c | 13 +++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index c9b192e..2f3ac22 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,6 +101,10 @@ HEAD::
Used to indicate the currently checked out branch.  Is '*' if
HEAD points to the current ref, and ' ' otherwise.
 
+color::
+   Used to change color of output.  Followed by `:colorname`,
+   where names are described in `color.branch.*`.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index ed81407..c59fffe 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
 #include quote.h
 #include parse-options.h
 #include remote.h
+#include color.h
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -76,6 +77,7 @@ static struct {
{ symref },
{ flag },
{ HEAD },
+   { color },
 };
 
 /*
@@ -662,8 +664,9 @@ static void populate_value(struct refinfo *ref)
!branch-merge[0]-dst)
continue;
refname = branch-merge[0]-dst;
-   }
-   else if (!strcmp(name, flag)) {
+   } else if (!prefixcmp(name, color)) {
+   ;
+   } else if (!strcmp(name, flag)) {
char buf[256], *cp = buf;
if (ref-flag  REF_ISSYMREF)
cp = copy_advance(cp, ,symref);
@@ -729,6 +732,12 @@ static void populate_value(struct refinfo *ref)
else
v-s = ;
continue;
+   } else if (!prefixcmp(name, color)) {
+   char color[COLOR_MAXLEN] = ;
+
+   color_parse(formatp, --format, color);
+   v-s = xstrdup(color);
+   continue;
} else
die(unknown %.*s format %s,
(int)(formatp - name), name, formatp);
-- 
1.8.5.rc0.3.g914176d.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 v2 3/3] for-each-ref: introduce %(color:...) for color

[Junio C Hamano]

Ramkumar Ramachandra artag...@gmail.com writes:

 + } else if (!prefixcmp(name, color)) {
 + ;

How is %(color:short) parsed with this code?

This part says, Yeah, I see something starting with color, and
then later strchr(name, ':') will point formatp to short.

Luckily, %(colorgarbage:short) does not even come this far because
parse_atom() would not have allowed the codeflow to, but comparing
with color: here may be a lot more defensive and safe, I think.

And find the color-value here, stuffing v-s inside this else if,
continue, without letting the formatp part work on refname this
piece of code does not even set.  Just like how we handle flag
without falling thru to the formatp code.


 + } else if (!strcmp(name, flag)) {
   char buf[256], *cp = buf;
   if (ref-flag  REF_ISSYMREF)
   cp = copy_advance(cp, ,symref);
 @@ -729,6 +732,12 @@ static void populate_value(struct refinfo *ref)
   else
   v-s = ;
   continue;
 + } else if (!prefixcmp(name, color)) {
 + char color[COLOR_MAXLEN] = ;
 +
 + color_parse(formatp, --format, color);
 + v-s = xstrdup(color);
 + continue;
   } else
   die(unknown %.*s format %s,
   (int)(formatp - name), name, formatp);
--
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 v2 3/3] for-each-ref: introduce %(color:...) for color

[Junio C Hamano]

Junio C Hamano gits...@pobox.com writes:

 Ramkumar Ramachandra artag...@gmail.com writes:

 +} else if (!prefixcmp(name, color)) {
 +;

 How is %(color:short) parsed with this code?

 This part says, Yeah, I see something starting with color, and
 then later strchr(name, ':') will point formatp to short.

 Luckily, %(colorgarbage:short) does not even come this far because
 parse_atom() would not have allowed the codeflow to, but comparing
 with color: here may be a lot more defensive and safe, I think.

 And find the color-value here, stuffing v-s inside this else if,
 continue, without letting the formatp part work on refname this
 piece of code does not even set.  Just like how we handle flag
 without falling thru to the formatp code.

Perhaps like this (obviously not tested as these three patches did
not add any tests ;-)

I also think that there should be a mechanism to do color:reset
after each record is issued automatically, and also have the color
output honor --color=auto from the command line, i.e.

git for-each-ref --color=auto --format='%(color:blue)%(subject)' | cat

should turn the coloring off.

So I think this patch may be a first step in the right direction,
but there are quite a lot more work that is needed before it gets
ready for production use.

Thanks.

 builtin/for-each-ref.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 9e07571..07a9385 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -664,8 +664,12 @@ static void populate_value(struct refinfo *ref)
!branch-merge[0]-dst)
continue;
refname = branch-merge[0]-dst;
-   } else if (!prefixcmp(name, color)) {
-   ;
+   } else if (!prefixcmp(name, color:)) {
+   char color[COLOR_MAXLEN] = ;
+
+   color_parse(name + 6, --format, color);
+   v-s = xstrdup(color);
+   continue;
} else if (!strcmp(name, flag)) {
char buf[256], *cp = buf;
if (ref-flag  REF_ISSYMREF)
@@ -733,12 +737,6 @@ static void populate_value(struct refinfo *ref)
else
v-s = ;
continue;
-   } else if (!prefixcmp(name, color)) {
-   char color[COLOR_MAXLEN] = ;
-
-   color_parse(formatp, --format, color);
-   v-s = xstrdup(color);
-   continue;
} else
die(unknown %.*s format %s,
(int)(formatp - name), name, formatp);
--
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 v2 3/3] for-each-ref: introduce %(color:...) for color

[Ramkumar Ramachandra]

Junio C Hamano wrote:
 Perhaps like this (obviously not tested as these three patches did
 not add any tests ;-)

Sorry about that. I didn't notice t6300-for-each-ref.sh. Will fix in
the next round.

 I also think that there should be a mechanism to do color:reset
 after each record is issued automatically, and also have the color
 output honor --color=auto from the command line, i.e.

 git for-each-ref --color=auto --format='%(color:blue)%(subject)' | cat

 should turn the coloring off.

We can add --color=auto later, but I'm wondering about auto-reset
color after each token. What happens if I do:

  $ git for-each-ref --format='%(subject)%(color:blue)'

No color, right? So, it should be auto-reset color after each token
_and_ at end of format-string.
--
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