[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