gettext: print_timeout

gettext: print_timeout

[Carles Pina i Estany]

Hi,

print_timeout is not working fine when the translated string contains
multi-byte characters, because it's locating where to write the seconds
when second_stage == 1 (so the seconds when second_stage == 1 are
miss-placed).

Tomorrow evening or Sunday I will fix it.

-- 
Carles Pina i Estany
	http://pinux.info

gettext: print_timeout

[Carles Pina i Estany]

[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]


Hello,

Find attached a patch about print_timeout.

Until now print_timeout had some problems with multi-byte / multi-width
characters. Now it's fixed, printing all sentence every time as
commented in the IRC.

I would like to comment two things:

+  char *msg_formatted = grub_malloc (sizeof (char) *
+                        (grub_strlen (msg) + 1 + 6));
+                        /* 5: max string length that `timeout' can
have.  */

timeout is a int, so the maximum string length that can do is -32768
(makes, no sense, I know). I could calculate the length of the string of
the int in base 10 dividing by 10 multiple times but I think that
allocating space for 6 position is more than enough, actually
grub_strlen already allocates 2 sizeof (char) (because %d). I mean, I
can add code to save some bytes in memory. Any better and simpler way?

Other thing to clarify:
+  print_spaces (GRUB_TERM_WIDTH - strwidth - 3);

The line is printing spaces from the end of the string until the end of
the line (GRUB_TERM_WIDTH). I'm not assuming that the new string with
the new timeout is one char shorter like it happens in English because
the plurals in some languages can break it.

If ok I would commit.

-- 
Carles Pina i Estany
	http://pinux.info

[-- Attachment #2: print_timeout.patch --]
[-- Type: text/x-diff, Size: 3894 bytes --]

=== modified file 'ChangeLog'
--- ChangeLog	2009-12-12 00:43:32 +0000
+++ ChangeLog	2009-12-13 00:44:09 +0000
@@ -1,3 +1,10 @@
+2009-12-13  Carles Pina i Estany  <carles@pina.cat>
+
+	* normal/menu_text.c (utf8_to_ucs4): New definition.
+	(print_message_indented): Use `utf8_to_ucs'.
+	(print_timeout): Prints the whole string so avoid problems with
+	multi-byte and multi-width characters.
+
 2009-12-12  Robert Millan  <rmh@aybabtu.com>
 
 	* gendistlist.sh (EXTRA_DISTFILES): Add `genvideolist.sh'.

=== modified file 'normal/menu_text.c'
--- normal/menu_text.c	2009-12-08 00:08:52 +0000
+++ normal/menu_text.c	2009-12-13 01:26:09 +0000
@@ -77,35 +77,50 @@ getstringwidth (grub_uint32_t * str, con
   return width;
 }
 
+static int
+utf8_to_ucs4 (const char *msg, grub_uint32_t **unicode_msg,
+			grub_uint32_t **last_position)
+{
+  grub_ssize_t msg_len = grub_strlen (msg);
+
+  *unicode_msg = grub_malloc (grub_strlen (msg) * sizeof (grub_uint32_t));
+  
+  if (!*unicode_msg)
+    {
+      grub_printf ("utf8_to_ucs4 ERROR1: %s", msg);
+      return -1;
+    }
+
+  msg_len = grub_utf8_to_ucs4 (*unicode_msg, msg_len,
+  			      (grub_uint8_t *) msg, -1, 0);
+
+  *last_position = *unicode_msg + msg_len;
+
+  if (msg_len < 0)
+    {
+      grub_printf ("utf8_to_ucs4 ERROR2: %s", msg);
+      grub_free (*unicode_msg);
+    }
+  return msg_len;
+}
+
 static void
 print_message_indented (const char *msg)
 {
   const int line_len = GRUB_TERM_WIDTH - grub_getcharwidth ('m') * 15;
 
   grub_uint32_t *unicode_msg;
+  grub_uint32_t *last_position;
 
-  grub_ssize_t msg_len = grub_strlen (msg);
-
-  unicode_msg = grub_malloc (msg_len * sizeof (*unicode_msg));
+  int msg_len;
 
-  msg_len = grub_utf8_to_ucs4 (unicode_msg, msg_len,
-                              (grub_uint8_t *) msg, -1, 0);
-
-  if (!unicode_msg)
-    {
-      grub_printf ("print_message_indented ERROR1: %s", msg);
-      return;
-    }
+  msg_len = utf8_to_ucs4 (msg, &unicode_msg, &last_position);
 
   if (msg_len < 0)
     {
-      grub_printf ("print_message_indented ERROR2: %s", msg);
-      grub_free (unicode_msg);
       return;
     }
 
-  const grub_uint32_t *last_position = unicode_msg + msg_len;
-
   grub_uint32_t *current_position = unicode_msg;
 
   grub_uint32_t *next_new_line = unicode_msg;
@@ -368,22 +383,39 @@ get_entry_number (const char *name)
 }
 
 static void
-print_timeout (int timeout, int offset, int second_stage)
+print_timeout (int timeout, int offset,
+	       int second_stage __attribute__ ((unused)))
 {
   const char *msg =
     _("The highlighted entry will be booted automatically in %ds.");
-  const int msg_localized_len = grub_strlen (msg);
-  const int number_spaces = GRUB_TERM_WIDTH - msg_localized_len - 3;
 
-  char *msg_end = grub_strchr (msg, '%');
+  char *msg_formatted = grub_malloc (sizeof (char) *
+                        (grub_strlen (msg) + 1 + 6));
+                        /* 5: max string length that `timeout' can have.  */
+
+  grub_sprintf (msg_formatted, msg, timeout);
+
+  grub_uint32_t *unicode_msg;
+  grub_uint32_t *last_position;
+
+  int msg_len = utf8_to_ucs4 (msg_formatted, &unicode_msg, &last_position);
+  if (msg_len < 0)
+    {
+      return;
+    }
 
-  grub_gotoxy (second_stage ? (msg_end - msg + 3) : 3, GRUB_TERM_HEIGHT - 3);
-  grub_printf (second_stage ? msg_end : msg, timeout);
-  print_spaces (second_stage ? number_spaces : 0);
+  grub_gotoxy (3, GRUB_TERM_HEIGHT - 3);
+  grub_print_ucs4 (unicode_msg, last_position);
+
+  int strwidth = getstringwidth (unicode_msg, last_position);
+
+  print_spaces (GRUB_TERM_WIDTH - strwidth - 3);
+  
+  grub_free(unicode_msg);
 
   grub_gotoxy (GRUB_TERM_CURSOR_X, GRUB_TERM_FIRST_ENTRY_Y + offset);
   grub_refresh ();
-};
+}
 
 /* Show the menu and handle menu entry selection.  Returns the menu entry
    index that should be executed or -1 if no entry should be executed (e.g.,

Re: gettext: print_timeout

[Robert Millan]

On Sun, Dec 13, 2009 at 01:31:55AM +0000, Carles Pina i Estany wrote:
> 
> If ok I would commit.

Was this checked in?  ISTR having OKed it on IRC, but I don't see it
in ChangeLog.


> === modified file 'ChangeLog'
> --- ChangeLog	2009-12-12 00:43:32 +0000
> +++ ChangeLog	2009-12-13 00:44:09 +0000
> @@ -1,3 +1,10 @@
> +2009-12-13  Carles Pina i Estany  <carles@pina.cat>
> +
> +	* normal/menu_text.c (utf8_to_ucs4): New definition.
> +	(print_message_indented): Use `utf8_to_ucs'.
> +	(print_timeout): Prints the whole string so avoid problems with
> +	multi-byte and multi-width characters.
> +
>  2009-12-12  Robert Millan  <rmh@aybabtu.com>
>  
>  	* gendistlist.sh (EXTRA_DISTFILES): Add `genvideolist.sh'.
> 
> === modified file 'normal/menu_text.c'
> --- normal/menu_text.c	2009-12-08 00:08:52 +0000
> +++ normal/menu_text.c	2009-12-13 01:26:09 +0000
> @@ -77,35 +77,50 @@ getstringwidth (grub_uint32_t * str, con
>    return width;
>  }
>  
> +static int
> +utf8_to_ucs4 (const char *msg, grub_uint32_t **unicode_msg,
> +			grub_uint32_t **last_position)
> +{
> +  grub_ssize_t msg_len = grub_strlen (msg);
> +
> +  *unicode_msg = grub_malloc (grub_strlen (msg) * sizeof (grub_uint32_t));
> +  
> +  if (!*unicode_msg)
> +    {
> +      grub_printf ("utf8_to_ucs4 ERROR1: %s", msg);
> +      return -1;
> +    }
> +
> +  msg_len = grub_utf8_to_ucs4 (*unicode_msg, msg_len,
> +  			      (grub_uint8_t *) msg, -1, 0);
> +
> +  *last_position = *unicode_msg + msg_len;
> +
> +  if (msg_len < 0)
> +    {
> +      grub_printf ("utf8_to_ucs4 ERROR2: %s", msg);
> +      grub_free (*unicode_msg);
> +    }
> +  return msg_len;
> +}
> +
>  static void
>  print_message_indented (const char *msg)
>  {
>    const int line_len = GRUB_TERM_WIDTH - grub_getcharwidth ('m') * 15;
>  
>    grub_uint32_t *unicode_msg;
> +  grub_uint32_t *last_position;
>  
> -  grub_ssize_t msg_len = grub_strlen (msg);
> -
> -  unicode_msg = grub_malloc (msg_len * sizeof (*unicode_msg));
> +  int msg_len;
>  
> -  msg_len = grub_utf8_to_ucs4 (unicode_msg, msg_len,
> -                              (grub_uint8_t *) msg, -1, 0);
> -
> -  if (!unicode_msg)
> -    {
> -      grub_printf ("print_message_indented ERROR1: %s", msg);
> -      return;
> -    }
> +  msg_len = utf8_to_ucs4 (msg, &unicode_msg, &last_position);
>  
>    if (msg_len < 0)
>      {
> -      grub_printf ("print_message_indented ERROR2: %s", msg);
> -      grub_free (unicode_msg);
>        return;
>      }
>  
> -  const grub_uint32_t *last_position = unicode_msg + msg_len;
> -
>    grub_uint32_t *current_position = unicode_msg;
>  
>    grub_uint32_t *next_new_line = unicode_msg;
> @@ -368,22 +383,39 @@ get_entry_number (const char *name)
>  }
>  
>  static void
> -print_timeout (int timeout, int offset, int second_stage)
> +print_timeout (int timeout, int offset,
> +	       int second_stage __attribute__ ((unused)))
>  {
>    const char *msg =
>      _("The highlighted entry will be booted automatically in %ds.");
> -  const int msg_localized_len = grub_strlen (msg);
> -  const int number_spaces = GRUB_TERM_WIDTH - msg_localized_len - 3;
>  
> -  char *msg_end = grub_strchr (msg, '%');
> +  char *msg_formatted = grub_malloc (sizeof (char) *
> +                        (grub_strlen (msg) + 1 + 6));
> +                        /* 5: max string length that `timeout' can have.  */
> +
> +  grub_sprintf (msg_formatted, msg, timeout);
> +
> +  grub_uint32_t *unicode_msg;
> +  grub_uint32_t *last_position;
> +
> +  int msg_len = utf8_to_ucs4 (msg_formatted, &unicode_msg, &last_position);
> +  if (msg_len < 0)
> +    {
> +      return;
> +    }
>  
> -  grub_gotoxy (second_stage ? (msg_end - msg + 3) : 3, GRUB_TERM_HEIGHT - 3);
> -  grub_printf (second_stage ? msg_end : msg, timeout);
> -  print_spaces (second_stage ? number_spaces : 0);
> +  grub_gotoxy (3, GRUB_TERM_HEIGHT - 3);
> +  grub_print_ucs4 (unicode_msg, last_position);
> +
> +  int strwidth = getstringwidth (unicode_msg, last_position);
> +
> +  print_spaces (GRUB_TERM_WIDTH - strwidth - 3);
> +  
> +  grub_free(unicode_msg);
>  
>    grub_gotoxy (GRUB_TERM_CURSOR_X, GRUB_TERM_FIRST_ENTRY_Y + offset);
>    grub_refresh ();
> -};
> +}
>  
>  /* Show the menu and handle menu entry selection.  Returns the menu entry
>     index that should be executed or -1 if no entry should be executed (e.g.,
> 

> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi

Re: gettext: print_timeout

[Carles Pina i Estany]

Hi,

On Dec/24/2009, Robert Millan wrote:
> On Sun, Dec 13, 2009 at 01:31:55AM +0000, Carles Pina i Estany wrote:
> > 
> > If ok I would commit.
> 
> Was this checked in?  ISTR having OKed it on IRC, but I don't see it
> in ChangeLog.

it's already applied but not in the way that I was suggesting in the
patch. It's a commit from date 2009-12-19 (first one of that day).

        (print_timeout): Use `print_message_indented' to print the message.
        Deletes `second_stage' parameter.

because print_timeout doesn't need the utf8_to_ucs4 code, this code 
is inside prprint_message_indented. If some other function will 
need to use this code I will factorize.

(I could factorize now but then we will have an extra call 
to one function, etc.)

-- 
Carles Pina i Estany
	http://pinux.info