[PATCH] flexi-sized user/group/size fields for ls
Denys Vlasenko
vda.linux at googlemail.com
Mon Jan 18 00:21:23 UTC 2010
On Saturday 16 January 2010 08:24, Ian Wienand wrote:
> Thanks for the review; here's another go
>
> Denys Vlasenko wrote:
> > On Friday 15 January 2010 21:16, Ian Wienand wrote:
> > The code growth is somewhat bigt, the code is not Unicode friendly,
> > and patch introduces broken indentation.
>
> For unicode I used bb_mbstrlen()
Using bb_mbstrlen is ok for now, but recently it was brought
to my attention that Unicode has double-wide chars (Japanese
and such)... >>>8O
Apparently we'd need to migrate to wc[s]width() functions
to support those. Gosh...
In the patch, printf("%*s") with unicode does not work properly.
It pads the value based on its strlen, not unicode width.
There is code to play around it, for example, in dumpleases.c:
#if ENABLE_FEATURE_ASSUME_UNICODE
printf(" %-16s%s%*s", inet_ntoa(addr), lease.hostname,
20 - (int)bb_mbstrlen(lease.hostname), "");
#else
printf(" %-16s%-20s", inet_ntoa(addr), lease.hostname);
#endif
but I think we need to create and use a better support infrastructure.
Something like:
/* equivalent to printf("%-20.20s", str) */
char unicode_buffer[20 * MB_CUR_MAX];
printf("%s", unicode_exact(20, str, unicode_buffer);
/* no need to free */
/* equivalent to printf("%-20s", str) */
char *malloced = unicode_minimum(20, str);
printf("%s", malloced);
free(malloced); /* ugh */
/* equivalent to printf("%-20s", str), better one */
printf("%s%*s", str, unicode_pad_to_width(str, 20), "");
/* equivalent to printf("%20s", str) */
printf("%*s%s", unicode_pad_to_width(str, 20), "", str);
> Indentation; should be fixed
It is not fixed. Two space indents are still there.
> Fixed compliation when opts turned off and used the right groups function too.
>
> For code growth, I reworked it a little. For the case of the tiny
> system with ENABLE_FEATURE_AUTOWIDTH and ENABLE_FEATURE_LS_USERNAME
> disabled growth is 10 bytes
It should be 0 bytes. You needlessly use %*s format in this case.
> With both enabled, IMHO I'd prefer to use a bit more space and know
> that the output will look right
w = bb_mbstrlen(make_human_readable_str(dn[i]->dstat.st_size, 1, 0));
This one (1) is never in unicode, and
(2) it is known that it is always 7 chars wide or less,
can just use 7-byte field.
--
vda
More information about the busybox
mailing list