[PATCH] utf8: add utf8_strwidth()

From: Geoffrey Thomas <redacted>
I'm about to use this pattern more than once, so make it a common function.

Signed-off-by: Geoffrey Thomas <redacted>
---
 utf8.c |   12 ++++++++++++
 utf8.h |    1 +
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/utf8.c b/utf8.c
index dc37353..a2d888d 100644
--- a/utf8.c
+++ b/utf8.c

@@ -246,6 +246,18 @@ int utf8_width(const char **start, size_t *remainder_p)
 	return git_wcwidth(ch);
 }
 
+/*
+ * Returns the total number of columns required by a null-terminated
+ * string.
+ */
+size_t utf8_strwidth(const char *string)
+{
+	size_t width = 0;
+	while (string && *string)
+		width += utf8_width(&string, NULL);
+	return width;
+}
+
 int is_utf8(const char *text)
 {
 	while (*text) {

diff --git a/utf8.h b/utf8.h
index 98cce1b..1ae3450 100644
--- a/utf8.h
+++ b/utf8.h

@@ -5,6 +5,7 @@ typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
 ucs_char_t pick_one_utf8_char(const char **start, size_t *remainder_p);
 int utf8_width(const char **start, size_t *remainder_p);
+size_t utf8_strwidth(const char *string);
 int is_utf8(const char *text);
 int is_encoding_utf8(const char *name);
 

-- 
1.5.6.5

From: Geoffrey Thomas <redacted>
git blame misaligns output if a author's name has a differing display width and
strlen; for instance, an accented Latin letter that takes two bytes to encode
will cause the rest of the line to be shifted to the left by one. To fix this,
use utf8_strwidth instead of strlen (and compute the padding ourselves, since
printf doesn't know about UTF-8).

Signed-off-by: Geoffrey Thomas <redacted>
---
 builtin-blame.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index aae14ef..2941fc0 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c

@@ -19,6 +19,7 @@
 #include "string-list.h"
 #include "mailmap.h"
 #include "parse-options.h"
+#include "utf8.h"
 
 static char blame_usage[] = "git blame [options] [rev-opts] [rev] [--] file";
 

@@ -1619,9 +1620,9 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 				       ent->s_lno + 1 + cnt);
 
 			if (!(opt & OUTPUT_NO_AUTHOR))
-				printf(" (%-*.*s %10s",
-				       longest_author, longest_author,
+				printf(" (%s%*s %10s",
 				       ci.author,
+				       longest_author - utf8_strwidth(ci.author), "",
 				       format_time(ci.author_time,
 						   ci.author_tz,
 						   show_raw_time));

@@ -1755,7 +1756,7 @@ static void find_alignment(struct scoreboard *sb, int *option)
 		if (!(suspect->commit->object.flags & METAINFO_SHOWN)) {
 			suspect->commit->object.flags |= METAINFO_SHOWN;
 			get_commit_info(suspect->commit, &ci, 1);
-			num = strlen(ci.author);
+			num = utf8_strwidth(ci.author);
 			if (longest_author < num)
 				longest_author = num;
 		}

-- 
1.5.6.5

Hi,

On Fri, 30 Jan 2009, Geoffrey Thomas wrote:

From: Geoffrey Thomas <redacted>
git blame misaligns output if a author's name has a differing display width and
strlen; for instance, an accented Latin letter that takes two bytes to encode
will cause the rest of the line to be shifted to the left by one. To fix this,
use utf8_strwidth instead of strlen (and compute the padding ourselves, since
printf doesn't know about UTF-8).

Good point (even if your commit message has lines much longer than 72 
chars, ASCII ones at that).

But how certain are you at that point that the authors are in UTF-8 
format?  IOW what encoding conversions were possibly performed up to that 
point?

You might want to make it easier on possible reviewers by putting that 
discussion into the commit message.

Ciao,
Dscho

Good point (even if your commit message has lines much longer than 72
chars, ASCII ones at that).

Oops; I'll fix that.

But how certain are you at that point that the authors are in UTF-8
format?  IOW what encoding conversions were possibly performed up to that
point?

I don't believe there are any encoding conversions performed up to that 
point. IIRC git doesn't require any encoding but encourages UTF-8; if it's 
something obscure, I have no way of knowing how wide in screen columns the 
author field is because I likely don't have a library for it in git at 
all. I do have a utf8.c, though.

Currently, however, printf("%*.*s", width, width, author) is simply wrong, 
because printf only cares about bytes, not screen columns. Do you think I 
should fall back on the old behavior if i18n.commitencoding is set, or if 
at least one of the author names isn't parseable as UTF-8, or something? 
Or should I be doing this with iconv and assuming all commits are 
encoded in the current encoding specified via $LANG or $LC_whatever?

-- 
Geoffrey Thomas
geofft@mit.edu

On Fri, Jan 30, 2009 at 04:41:28AM -0500, Geoffrey Thomas wrote:

I'm about to use this pattern more than once, so make it a common function.

I know next to nothing about our encoding functions, but this seems
suspiciously similar to utf8_width in utf8.c. There is also a
git_wcwidth, but I don't know how they relate.

-Peff

On Fri, Jan 30, 2009 at 05:22:07PM -0500, Geoffrey Thomas wrote:

I don't believe there are any encoding conversions performed up to that  
point. IIRC git doesn't require any encoding but encourages UTF-8; if it's 
something obscure, I have no way of knowing how wide in screen columns the 
author field is because I likely don't have a library for it in git at  
all. I do have a utf8.c, though.

Don't we pull the author from the commit message after it has been
converted using reencode_commit_message (see get_commit_info)? That
should be respecting the log output encoding.

It looks like we just throw away the information on what we encoded _to_
(i.e., the second parameter of reencode_commit_message).  Probably we
need to remember that and use a generic "what is the width of this
string in this encoding" function.

-Peff

On Sat, 31 Jan 2009, Jeff King wrote:

I know next to nothing about our encoding functions, but this seems
suspiciously similar to utf8_width in utf8.c. There is also a
git_wcwidth, but I don't know how they relate.

git_wcwidth determines the screen columns of a single ucs_char_t. 
utf8_width returns the git_wcwidth of the first character in a string. 
utf8_strwidth (the function added by this patch) is a simple loop around 
utf8_width, because writing the loop every time would be silly.

On that note, there are probably more cases in the code that ought to use 
something like utf8_strwidth. I only noticed this one case because I'm 
working on a project with someone with an accented letter in his last 
name.

-- 
Geoffrey Thomas
geofft@mit.edu

On Sat, Jan 31, 2009 at 03:51:48AM -0500, Geoffrey Thomas wrote:

On Sat, 31 Jan 2009, Jeff King wrote:

quoted

I know next to nothing about our encoding functions, but this seems
suspiciously similar to utf8_width in utf8.c. There is also a
git_wcwidth, but I don't know how they relate.

git_wcwidth determines the screen columns of a single ucs_char_t.  
utf8_width returns the git_wcwidth of the first character in a string.  
utf8_strwidth (the function added by this patch) is a simple loop around  
utf8_width, because writing the loop every time would be silly.

Urgh. Sorry. If I had taken 3 seconds to actually _look_ at your patch,
I would have seen that (instead I thought "don't we already have
something that does this" and went straight to the existing code).

But no, it looks like we don't already have this, so your patch is fine.
Sorry for the noise.

-Peff

Hi,

On Fri, 30 Jan 2009, Geoffrey Thomas wrote:

Currently, however, printf("%*.*s", width, width, author) is simply 
wrong, because printf only cares about bytes, not screen columns. Do you 
think I should fall back on the old behavior if i18n.commitencoding is 
set, or if at least one of the author names isn't parseable as UTF-8, or 
something? Or should I be doing this with iconv and assuming all commits 
are encoded in the current encoding specified via $LANG or $LC_whatever?

I do not know what encoding the author is at that point, but if you cannot 
be sure that it is UTF-8, using utf8_strwidth() is just as wrong as the 
current code, IMHO.

Ciao,
Dscho