Re: [PATCH 2/7] Add column layout

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

 



2011/2/9 Jonathan Nieder <jrnieder@xxxxxxxxx>:
> Nguyán ThÃi Ngác Duy wrote:
>
>> +++ b/column.c
>> @@ -0,0 +1,177 @@
> [...]
>> +static int item_length(const struct column_layout *c, const char *s)
>> +{
>> + Â Â int a_len = 0;
>> +
>> + Â Â if (!(c->mode & COL_ANSI))
>> + Â Â Â Â Â Â return strlen(s);
>> +
>> + Â Â while ((s = strstr(s, "\033[")) != NULL) {
>> + Â Â Â Â Â Â int len = strspn(s+2, "0123456789;");
>> + Â Â Â Â Â Â s += len+3; /* \033[<len><func char> */
>> + Â Â Â Â Â Â a_len += len+3;
>> + Â Â }
>> + Â Â return a_len;
>> +}
>
> I think you mean "return strlen(orig_s) - a_len".
>
> Something like the following could be more obvious, though
> it is unfortunately verbose.
>
> Â Â Â Âint len = 0;
> Â Â Â Âwhile (*s) {
> Â Â Â Â Â Â Â Âconst char *next;
>
> Â Â Â Â Â Â Â Â/* \033[<len><func char> */
> Â Â Â Â Â Â Â Âif (!prefixcmp(s, "\033[")) {
> Â Â Â Â Â Â Â Â Â Â Â Âs += strlen("\033[");
> Â Â Â Â Â Â Â Â Â Â Â Âs += strspn(s, "0123456789;");
> Â Â Â Â Â Â Â Â Â Â Â Âif (!*s)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â... handle somehow ...
> Â Â Â Â Â Â Â Â Â Â Â Âs++;
> Â Â Â Â Â Â Â Â}
>
> Â Â Â Â Â Â Â Ânext = strchrnul(s, '\033');
> Â Â Â Â Â Â Â Âlen += next - s;
> Â Â Â Â Â Â Â Âs = next;
> Â Â Â Â}
>
> Both miscompute the width of "DÃpÃt". ÂSomething like this can do ok
> if the string is modifiable and we know it is UTF-8:
>
> Â Â Â Â Â Â Â Âchar save;
> Â Â Â Â Â Â Â Â...
> Â Â Â Â Â Â Â Ânext = strchrnul(s, '\033');
>
> Â Â Â Â Â Â Â Â/*
> Â Â Â Â Â Â Â Â * NEEDSWORK: a utf8_strwidth variant that
> Â Â Â Â Â Â Â Â * accepts a memory area not terminated by \0
> Â Â Â Â Â Â Â Â * would avoid this ugliness.
> Â Â Â Â Â Â Â Â */
> Â Â Â Â Â Â Â Âsave = *next;
> Â Â Â Â Â Â Â Â*next = '\0';
> Â Â Â Â Â Â Â Âlen += utf8_strwidth(s);
> Â Â Â Â Â Â Â Â*next = save;
>
> POSIX does not provide a strwidth function, so if we want to handle
> encodings like SHIFT-JIS then something uglier[1] might come to life.

I think UTF-8 is enough. Git does not produce complex ansi escape
codes. Perhaps checking them at the beginning and reset code at the
end is enough.

> [...]
>> +static void relayout(const struct column_layout *c,
>> + Â Â Â Â Â Â Â Â Âint padding, int spare,
>> + Â Â Â Â Â Â Â Â Âint *initial_width, int **width,
>> + Â Â Â Â Â Â Â Â Âint *rows, int *cols)
>> +{
>> + Â Â int new_rows, new_cols, new_initial_width;
>> + Â Â int i, *new_width, new_spare, total_width;
>> +
>> + Â Â /*
>> + Â Â Â* Assume all columns have same width, we would need
>> + Â Â Â* initial_width*cols. But then after squeezing, we have
>> + Â Â Â* "spare" more chars. Assume a new total_width with
>> + Â Â Â* additional chars, then re-squeeze to see if it fits
>> + Â Â Â* c->width.
>> + Â Â Â*/
>
> Might be easier to debug if this part were deferred to a separate
> patch. :)
>
>> + Â Â total_width = (*initial_width)*(*cols) + spare;
>
> An odd heuristic. ÂDoes it work well in practice?

It seems so. If it does not, I guess I would need to look into how GNU
ls does it.
-- 
Duy
ÿô.nlj·Ÿ®‰­†+%ŠË±é¥Šwÿº{.nlj· ŠßžØn‡r¡öë¨è&£ûz¹Þúzf£¢·hšˆ§~†­†Ûÿÿïÿ‘ê_èæ+v‰¨þ)ßø

[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]