[PATCH PR/42686] Align the help text output
Joseph S. Myers
joseph@codesourcery.com
Thu Mar 18 17:54:00 GMT 2010
On Thu, 18 Mar 2010, Shujing Zhao wrote:
>
> +static wchar_t *
> +get_sub_wstr (wchar_t *wstr, int len)
Needs comment explaining semantics of parameters and return value.
> +/* Return the number of bytes in the multibyte representation of the wide
> + character WC. */
> +
> +static int
> +wctomb_nbytes (wchar_t wc)
> +{
This code is heavily assuming that converting from multibyte to wide
characters and back again yields exactly the same multibyte characters.
This is more likely in the Unicode world than it used to be, but it's not
clear we should be presuming it. I think code without this presumption
would be simpler than code with it; rather than converting the whole
string at once and then repeatedly extracting substrings, you'd convert
one character at a time, keeping track of the width at each point
(computing the width of one character at a time) and of the byte count to
the last possible point for a line break. (That way, you could also
easily allow for encodings with shift sequences, keeping an mbstate_t in
the course of processing the string.)
> +/* Return the length of multibyte string MSGSTR that can be printed within the
"length" = number of bytes?
> + {
> + nbytes = wctomb_nbytes (wmsgstr [i]);
> + colwidth ++;
Why this increment? Characters may have width 0 (combining characters) or
more than 1 and it's not clear why adding 1 should be the correct thing to
do here.
> + else if (nbytes > 1)
Why this conditional? Whatever the number of bytes is, you need to update
the width appropriately.
> + {
> + if (colwidth > room)
> + len = len_bk;
> + break;
Indentation seems very confused. Make sure that new code consistently
uses TABs for indenting source code lines.
> + int n_spaces = item_width <= item_col_width
> + ? col_width - item_width
> + : col_width - item_col_width;
I don't see why this conditional is here. For printing spaces, it's
always the number of columns that's relevant; the number of bytes is
irrelevant.
--
Joseph S. Myers
joseph@codesourcery.com
More information about the Gcc-patches
mailing list