[PATCH PR/42686] Align the help text output

Shujing Zhao pearly.zhao@oracle.com
Tue Apr 13 11:32:00 GMT 2010


On 04/07/2010 12:55 AM, Joseph S. Myers wrote:
Thanks. Fix the typo and all the comments. Change the confused variable 
cur_len_to_print to len_to_print.
>> +      /* Compute the point to break the string. */
>> +      if (left_len == 0)
>> +	{
>> +	  pre_len_to_print = cur_len_to_print;
>> +	  cur_len_to_print = remaining;
> 
> I think it might be clearer to have the loop in the form
> 
>   while (left_len > 0)
> 
> which at least would avoid questions of what mbrtowc is meant to do when 
> given a length of 0.  After the loop, if cur_len_to_print is 0 or the 
> entire string would fit in the given number of columns, set 
> cur_len_to_print accordingly to indicate that the whole string should be 
> printed.  Then make the loop mimic that in the non-i18n version of the 
> function as much as possible: if the previous columns occupy at least the 
> full width and a point to break has been found, then break; otherwise 
> decode a character; if a space then breaking *before* the space is OK; if 
> '-' or '/' (and the other conditions apply) then breaking *after* that 
> character is OK.
> 
> Incidentally, it would be a lot clearer if you had a single variable for 
> the byte index into the string (like the non-i18n version) instead of both 
> increasing msgstr and decreasing left_len - so the loop would be "while (i 
> < remaining)".  You wouldn't need to do "remaining - left_len" 
> computations that way.
Both pre_len_to_print and len_to_print are initialized REMAINING now. If no 
suitable line-breaking position has been found, the whole line will be printed.
a variable i is added to index the string to decode and use

while (i < remaining)

to control the loop. It looks clear. Thanks.
> 
>> +	}
>> +      else if (iswspace (wc[0]))
>> +	pre_len_to_print = cur_len_to_print = remaining - left_len - nbytes;
>> +      else  if ((wc[0] == L'-' || wc[0] == L'/')
>> +	        && iswalpha (pre_wc))
>> +	pre_len_to_print = cur_len_to_print = remaining - left_len;
> 
> Will pre_wc always be initialized here, if the string starts with '-' or 
> '/'?
It is changed to
+      else if ((wc[0] == L'-' || wc[0] == L'/')
+              && i > nbytes
+              && iswalpha (pre_wc))
(i > nbytes) would ensure the pre_wc is initialized. I found this solution from 
the non-i18n version.
> 
>> +      else if (nbytes > 1)
> 
> I still see no reason you should need conditionals on the number of bytes 
> in a character, instead of always working with characters regardless of 
> the number of bytes in them.
This condition is to make the string can be break after a multi bytes wide 
alphabetic or punctuations. The line can be break after every multi bytes wide 
alphabetic or punctuations. I think the nbytes can distinguish if it is a wide 
character before decode. The letter 'a' is recognized wide character after the 
decoding, but the line can't be broken after a 'a' at the string "after". I 
think the difference between one byte wide character and the multi bytes wide 
character is the nbytes.

Is it better changed to

+      else if ((iswalpha (wc[0]) || iswpunct (wc[0])) && nbytes > 1)

better?
> 
>> +	{
>> +	  pre_len_to_print = cur_len_to_print;
>> +	  cur_len_to_print = remaining - left_len;
>> +	  pre_wc = wc[0];
>> +	  pre_nbytes = nbytes;
>> +	  /* Keep the puncts printed with the previous word. */
> 
> I don't understand what issue you are dealing with here, but punctuation 
> other than '/' or '-' isn't considered a valid line-breaking point anyway 
> so you shouldn't need special code for it.
> 
Since the line can be broken after a multi bytes alphabetic, the following 
punctuation maybe be printed as the first character at the new line if the 
punctuation didn't be dealing with. I also found at some special cases, only a 
"full stop" printed at a new line. It looks strange. So I design to print these 
punctuation with the previous alphabetic.
That is to say, if a wide character followed by a wide character punctuation, 
the possible line-breaking position would not be set after this wide character. 
It would be set after the punctuation.
yeah, the '/' and '-' must be considered. How about used (nbytes > 1) again to 
distinguish the multi bytes wide character punctuation? It at least ensure the 
Chinese multi bytes punctuations are handled. The other punctuation will be 
handle just like the non-i18n version.

Thanks
Pearly
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0413.patch
Type: text/x-patch
Size: 8546 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20100413/b743c574/attachment.bin>


More information about the Gcc-patches mailing list