Re: /bin/ls formatting broken for non-C(?) locales

On 25.11.2015 3:15, Baptiste Daroussin wrote:
> On Sat, Nov 21, 2015 at 11:57:46PM +0300, Andrey Chernov wrote:
>> On 21.11.2015 15:18, Ed Schouten wrote:
>>> Hi Baptiste,
>>>
>>> I suppose you should use the wcswidth() function somewhere to compute
>>> the visible width of the month name. Some characters may be
>>> double-width, others may have no effective width at all.
>>>
>>
>> I agree. Checking error return of wide chars functions with some
>> fallback will be good too.
> 
> I have updated the code https://reviews.freebsd.org/D4239
> 
> Tested by modifying some locales to add double width and zero width unicode in
> the locales
> 
> Also added the error checking for the return of wide chars functions. For now I
> haven't added fallback, suggestions welcome if needed.

1) For just 1 char in wcswidth(&wab_months[i][j], 1); it is better to
use another function wcwidth(wab_months[i][j]);

2) By fallback I mean something which not stops ls working with
incorrect for some reason locale, like setting max_width_month to
MAX_ABMON_WIDTH on error return (from
mbstowcs/wcwidth/wcswidth/wcswidth) and exit from
populate_abbreviated_month().

3) wcwidth/wcswidth may return -1 too, it needs to be checked too.

4) The whole processing looks overcomplicated and not effective. What
about this instead?
for (i = 0; i < 12; i++) {
    count wcswidth() of each month and store it in wab_months_width[].
    count max_width_month.
}
for (i = 0; i < 12; i++) {
    if ((n = max_width_month - wab_months_width[i]) > 0)
	call wcscat(wab_months[i], L" ") n times.
}

5) If there is no %b is strftime() format, there is no sense to spend
CPU cycles on from populate_abbreviated_month(), so it should be called
only once inside ls_strftime() on first %b instead of calling it in
printtime() for all cases.

-- 
http://ache.vniz.net/