[PATCH PR/42686] Align the help text output

[Joseph S. Myers]

On Mon, 15 Mar 2010, Shujing Zhao wrote:

> +/* Return the number of bytes when converts the wide character WC
> +   to its multibyte representation. */

"Return the number of bytes in the multibyte representation of the wide 
character WC.".

> +  char *pmb = (char *)xmalloc (MB_CUR_MAX);

Missing space between "(char *)" and "xmalloc".

> +/* Return the length of MSGSTR that can be printed within the width ROOM. */ 
> +
> +unsigned int
> +get_aligned_len (const char *msgstr, unsigned int room,
> +	         unsigned int remaining)

You need to say what REMAINING is.

> @@ -1158,12 +1158,31 @@ wrap_help (const char *help,

The comment at the top of this function should be changed to make explicit 
that ITEM_WIDTH is a count of bytes (if that is indeed the case).

> +      if (item_width == item_col_width
> +          || item_width == 0)
> +        printf ("  %-*.*s", col_width, item_width, item);
> +      /* Handle the item that includes wide character. */
> +      else

I think the code would be a lot clearer with fewer conditionals.  Why do 
you need to handle specially the case where item_width == item_col_width?  
Can't you just use the logic below unconditionally?

> +        {
> +          printf ("  %s", item);
> +          /* Output a certain number of spaces for alignment. */
> +          if (item_col_width < col_width) 
> +            { 
> +              unsigned int n_spaces = col_width - item_col_width; 
> +              const char *space = " ";
> +              for (i = 0; i < n_spaces; i++) 
> +                printf ("%s", space); 

You don't actually need a loop here (given a number of spaces 
representable as an "int", which I think you can assume); you can use 
"%*s" as a format with an empty string argument to have printf output the 
number of spaces you computed.

> +	  if (len != gcc_gettext_width (help))
> +            len = get_aligned_len (help, room, remaining);

Again, I'd like to avoid conditionals if possible.

> +              for (i = 0; help[i]; i++)
> +	        {
> +	          if (i >= room && len != remaining)
> +		    break;
> +	          if (help[i] == ' ')
> +		    len = i;
> +	          else if ((help[i] == '-' || help[i] == '/')
> +		           && help[i + 1] != ' '
> +		           && i > 0 && ISALPHA (help[i - 1]))
> +		    len = i + 1;
> +	        }

You now have two copies of line-splitting logic, one in get_aligned_len 
and one in opts.c.  It would be better for the logic to be just in 
get_aligned_len.  You might still need two copies - the one using wide 
character functions and a simpler one where they aren't available - but 
having both in the same file would be better than sometimes using logic 
from one file and sometimes from another file.  (This would mean that 
intl.h no longer has a dummy definition of get_aligned_len; there would 
always be a real function that works out a suitable point to break a 
line.)

> +	  buf = (char *)alloca (len + 1);

Missing space in cast again.

-- 
Joseph S. Myers
joseph@codesourcery.com