Re: [PATCH] branch -v: align even when the first column is in UTF-8

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:

> Branch names are usually in ASCII so they are not the problem. The
> problem most likely comes from "(no branch)" translation, which is in
> UTF-8 and makes length calculation just wrong.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
>  So far all git translations are utf-8 compatible. Branch names may
>  use filesystem encoding, but then packed-refs specifies no encoding.
>  Anyway branch names should be in utf-8.. at least internally, imo.

I agree with all of the above, but shouldn't you be computing the
"maxwidth" based on the strwidth in the first place?  The use of
maxwidth in strbuf_addf() here clearly wants "we know N columns is
sufficient to show all output items, so pad the string to N columns"
here.  Looking for assignment "item.len = xxx" in the same file
shows these are computed as byte length, so you are offsetting off
of an incorrectly computed value.

Giving fewer padding bytes when showing a string that will occupy
fewer columns than it has bytes is independently necessary, once we
have the correct maxwidth that is computed in terms of the strwidth,
so this patch is not wrong per-se, but it is incomplete without a
correct maxwidth, no?

>  builtin/branch.c | 8 +++++---
>  1 tập tin đã bị thay đổi, 5 được thêm vào(+), 3 bị xóa(-)
>
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 0e060f2..7c1ffa8 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -17,6 +17,7 @@
>  #include "revision.h"
>  #include "string-list.h"
>  #include "column.h"
> +#include "utf8.h"
>  
>  static const char * const builtin_branch_usage[] = {
>  	"git branch [options] [-r | -a] [--merged | --no-merged]",
> @@ -490,11 +491,12 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>  	}
>  
>  	strbuf_addf(&name, "%s%s", prefix, item->name);
> -	if (verbose)
> +	if (verbose) {
> +		int utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf);
>  		strbuf_addf(&out, "%c %s%-*s%s", c, branch_get_color(color),
> -			    maxwidth, name.buf,
> +			    maxwidth + utf8_compensation, name.buf,
>  			    branch_get_color(BRANCH_COLOR_RESET));
> -	else
> +	} else
>  		strbuf_addf(&out, "%c %s%s%s", c, branch_get_color(color),
>  			    name.buf, branch_get_color(BRANCH_COLOR_RESET));
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]