[PATCH 10/15] for-each-ref: introduce format specifier %(*) and %(*)

2013-07-09 Thread Ramkumar Ramachandra 
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com

Pretty placeholders %(N) and %(N) require a user provided width N,
which makes sense because the commit chain could be really long and the
user only needs to look at a few at the top, going to the end just to
calculate the best width wastes CPU cycles.

for-each-ref is different; the display set is small, and we display them
all at once. We even support sorting, which goes through all display
items anyway.  This patch introduces new %(*) and %(*), which are
supposed to be followed immediately by %(fieldname) (i.e. original
for-each-ref specifiers, not ones coming from pretty.c). They calculate
the best width for the %(fieldname), ignoring ansi escape sequences if
any.

[rr: documentation]

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Documentation/git-for-each-ref.txt |  7 +++
 builtin/for-each-ref.c | 38 ++
 t/t6300-for-each-ref.sh| 20 
 3 files changed, 65 insertions(+)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index d8ad758..8cbc08c 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -47,6 +47,10 @@ OPTIONS
are hex digits interpolates to character with hex code
`xx`; for example `%00` interpolates to `\0` (NUL),
`%09` to `\t` (TAB) and `%0a` to `\n` (LF).
++
+Placeholders `%(*)` and `%(*)` work like `%(N)` and `%(N)`
+respectively, except that the width of the next placeholder is
+calculated.
 
 pretty::
A format string with supporting placeholders described in the
@@ -68,6 +72,9 @@ Caveats:
 3. Only the placeholders inherited from `format` will respect
quoting settings.
 
+3. Only the placeholders inherited from `format` will work with the
+   alignment placeholders `%(*)` and '%(*)`.
+
 pattern...::
If one or more patterns are given, only refs are shown that
match against at least one pattern, either using fnmatch(3) or
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 39454fb..da479d1 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 utf8.h
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -966,10 +967,30 @@ static void show_refs(struct refinfo **refs, int maxcount,
 }
 
 struct format_one_atom_context {
+   struct refinfo **refs;
+   int maxcount;
+
struct refinfo *info;
int quote_style;
 };
 
+static unsigned int get_atom_width(struct format_one_atom_context *ctx,
+  const char *start, const char *end)
+{
+   struct strbuf sb = STRBUF_INIT;
+   int i, atom = parse_atom(start, end);
+   unsigned int len = 0, sb_len;
+   for (i = 0; i  ctx-maxcount; i++) {
+   print_value(sb, ctx-refs[i], atom, ctx-quote_style);
+   sb_len = utf8_strnwidth(sb.buf, sb.len, 1);
+   if (sb_len  len)
+   len = sb_len;
+   strbuf_reset(sb);
+   }
+   strbuf_release(sb);
+   return len;
+}
+
 static size_t format_one_atom(struct strbuf *sb, const char *placeholder,
  void *format_context, void *user_data,
  struct strbuf *subst)
@@ -982,6 +1003,21 @@ static size_t format_one_atom(struct strbuf *sb, const 
char *placeholder,
return 1;
}
 
+   /*
+* Substitute %(*)%(atom) and friends with real width.
+*/
+   if (*placeholder == '' || *placeholder == '') {
+   const char *star = placeholder + 1;
+   if (!prefixcmp(star, (*)%() 
+   ((ep = strchr(star + strlen((*)%(), ')')) != NULL)) {
+   star++;
+   strbuf_addf(subst, %c(%u),
+   *placeholder,
+   get_atom_width(ctx, star + strlen(*)%(), 
ep));
+   return 1 + strlen((*));
+   }
+   }
+
if (*placeholder != '(')
return 0;
 
@@ -1008,6 +1044,8 @@ static void show_pretty_refs(struct refinfo **refs, int 
maxcount,
ctx.abbrev = DEFAULT_ABBREV;
ctx.format = format_one_atom;
ctx.user_data = fctx;
+   fctx.refs = refs;
+   fctx.maxcount = maxcount;
fctx.quote_style = quote_style;
for (i = 0; i  maxcount; i++) {
struct commit *commit = NULL;
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index d39e0b4..160018c 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -196,6 +196,26 @@ test_pretty head '%(20)%(committername) end' 'C O Mitter  
 end'
 test_pretty head '%(20)%(committername) end' '  C O Mitter end'
 test_pretty head '%(20)%(committername) end' ' C O Mitter

Re: [PATCH 10/15] for-each-ref: introduce format specifier %(*) and %(*)

2013-06-05 Thread Ramkumar Ramachandra 
Duy Nguyen wrote:
 I mentioned it before and I do it again. This is not optimal.

Yeah, I'll attempt to fix this, but it's not urgent.

 But I guess it's ok in this
 shape unless you run this over hundreds of refs.

Oh, you can run over a hundred refs just fine, for scripting purposes;
but why would you want to run over a hundred refs with a pretty that
includes %(*) or %(*)?
--
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 10/15] for-each-ref: introduce format specifier %(*) and %(*)

2013-06-05 Thread Duy Nguyen 
On Wed, Jun 5, 2013 at 3:14 PM, Ramkumar Ramachandra artag...@gmail.com wrote:
 Duy Nguyen wrote:
 I mentioned it before and I do it again. This is not optimal.

 Yeah, I'll attempt to fix this, but it's not urgent.

Agreed it's not urgent.

 But I guess it's ok in this
 shape unless you run this over hundreds of refs.

 Oh, you can run over a hundred refs just fine, for scripting purposes;
 but why would you want to run over a hundred refs with a pretty that
 includes %(*) or %(*)?

Good point. git for-each-ref|wc -l on my git repo says I have 673
refs. A naive use for-each-ref --pretty without any ref patterns could
happen. But I guess people will quickly learn to limit the ref
selection soon after doing that :-)
--
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 10/15] for-each-ref: introduce format specifier %(*) and %(*)

2013-06-04 Thread Ramkumar Ramachandra 
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com

Pretty placeholders %(N) and %(N) require a user provided width N,
which makes sense because the commit chain could be really long and the
user only needs to look at a few at the top, going to the end just to
calculate the best width wastes CPU cycles.

for-each-ref is different; the display set is small, and we display them
all at once. We even support sorting, which goes through all display
items anyway.  This patch introduces new %(*) and %(*), which are
supposed to be followed immediately by %(fieldname) (i.e. original
for-each-ref specifiers, not ones coming from pretty.c). They calculate
the best width for the %(fieldname), ignoring ansi escape sequences if
any.

[rr: documentation]

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

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 6135812..7d6db7f 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -47,6 +47,10 @@ OPTIONS
are hex digits interpolates to character with hex code
`xx`; for example `%00` interpolates to `\0` (NUL),
`%09` to `\t` (TAB) and `%0a` to `\n` (LF).
++
+Placeholders `%(*)` and `%(*)` work like `%(N)` and `%(N)`
+respectively, except that the width of the next placeholder is
+calculated.
 
 pretty::
A format string with supporting placeholders described in the
@@ -67,6 +71,9 @@ Caveats:
 3. Only the placeholders inherited from `format` will respect
quoting settings.
 
+3. Only the placeholders inherited from `format` will work with the
+   alignment placeholders `%(*)` and '%(*)`.
+
 pattern...::
If one or more patterns are given, only refs are shown that
match against at least one pattern, either using fnmatch(3) or
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index f8a3880..053a622 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 utf8.h
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -966,10 +967,30 @@ static void show_refs(struct refinfo **refs, int maxcount,
 }
 
 struct format_one_atom_context {
+   struct refinfo **refs;
+   int maxcount;
+
struct refinfo *info;
int quote_style;
 };
 
+static unsigned int get_atom_width(struct format_one_atom_context *ctx,
+  const char *start, const char *end)
+{
+   struct strbuf sb = STRBUF_INIT;
+   int i, atom = parse_atom(start, end);
+   unsigned int len = 0, sb_len;
+   for (i = 0; i  ctx-maxcount; i++) {
+   print_value(sb, ctx-refs[i], atom, ctx-quote_style);
+   sb_len = utf8_strnwidth(sb.buf, sb.len, 1);
+   if (sb_len  len)
+   len = sb_len;
+   strbuf_reset(sb);
+   }
+   strbuf_release(sb);
+   return len;
+}
+
 static size_t format_one_atom(struct strbuf *sb, const char *placeholder,
  void *format_context, void *user_data,
  struct strbuf *subst)
@@ -982,6 +1003,21 @@ static size_t format_one_atom(struct strbuf *sb, const 
char *placeholder,
return 1;
}
 
+   /*
+* Substitute %(*)%(atom) and friends with real width.
+*/
+   if (*placeholder == '' || *placeholder == '') {
+   const char *star = placeholder + 1;
+   if (!prefixcmp(star, (*)%() 
+   ((ep = strchr(star + strlen((*)%(), ')')) != NULL)) {
+   star++;
+   strbuf_addf(subst, %c(%u),
+   *placeholder,
+   get_atom_width(ctx, star + strlen(*)%(), 
ep));
+   return 1 + strlen((*));
+   }
+   }
+
if (*placeholder != '(')
return 0;
 
@@ -1003,6 +1039,8 @@ static void show_pretty_refs(struct refinfo **refs, int 
maxcount,
 
ctx.format = format_one_atom;
ctx.user_data = fctx;
+   fctx.refs = refs;
+   fctx.maxcount = maxcount;
fctx.quote_style = quote_style;
for (i = 0; i  maxcount; i++) {
struct commit *commit = NULL;
-- 
1.8.3.GIT

--
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 10/15] for-each-ref: introduce format specifier %(*) and %(*)

2013-06-04 Thread Duy Nguyen 
On Tue, Jun 4, 2013 at 7:35 PM, Ramkumar Ramachandra artag...@gmail.com wrote:
 +static unsigned int get_atom_width(struct format_one_atom_context *ctx,
 +  const char *start, const char *end)
 +{
 +   struct strbuf sb = STRBUF_INIT;
 +   int i, atom = parse_atom(start, end);
 +   unsigned int len = 0, sb_len;
 +   for (i = 0; i  ctx-maxcount; i++) {
 +   print_value(sb, ctx-refs[i], atom, ctx-quote_style);
 +   sb_len = utf8_strnwidth(sb.buf, sb.len, 1);
 +   if (sb_len  len)
 +   len = sb_len;
 +   strbuf_reset(sb);
 +   }
 +   strbuf_release(sb);
 +   return len;
 +}
 +

I mentioned it before and I do it again. This is not optimal. We
should cache the result of get_atom_width() after the first
calculation. I haven't done it yet because I'm still not sure if it's
worth supporting %(*)%non-atom at this stage. Caching for atoms only
is much easier because atom is indexed. But I guess it's ok in this
shape unless you run this over hundreds of refs.
--
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