[PATCH PR/42686] Align the help text output
Shujing Zhao
pearly.zhao@oracle.com
Fri Mar 19 11:43:00 GMT 2010
On 03/19/2010 01:03 AM, Joseph S. Myers wrote:
> 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.)
Yes! Thanks again! The source codes are clearer and simpler while choose your
method to convert one character at a time. All static functions I have added are
removed. Only the main function get_aligned_len are left.
>> + {
>> + 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.
It is changed to *length--* at the new logic.
+ colwidth += wcwidth (wc[0]);
+ length--;
+ if (length == 0)
+ len = remaining;
I have tried to used length = length - nbytes to get the new length. But if
nbytes > 1 and the character is the last character of the string (length will be
0 if it decreases nbytes), it would not assign the last variable *len* to
*len_bk*, then the character that maybe put into the ROOM would not be printed.
That is to say, it presumes the nbytes is 1. If (nbytes > 1), it would be
handled specially (just to update the value of len_bk). At the other
conditionals, len_bk is always kept consistently with len.
>
>> + else if (nbytes > 1)
>
> Why this conditional? Whatever the number of bytes is, you need to update
> the width appropriately.
At the new logic, the width is always be updated every time when convert one
character.
>
> Indentation seems very confused. Make sure that new code consistently
> uses TABs for indenting source code lines.
Thanks again. All the lines of the updated patch is checked to use TABs for
indenting.
>
>> + 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.
Yes! You are right! The gcc_gettext_width would always return the width whatever
the string is. It seems the old argument ITEM_WIDTH is not needed anymore. I
removed it from the function wrap_help.
Is it ok?
Thanks
Pearly
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 03191800.patch
Type: text/x-patch
Size: 6082 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20100319/dc6a148d/attachment.bin>
More information about the Gcc-patches
mailing list