Re: [evolution-patches] Patch for bug number 36247

[Jeffrey Stedfast <fejj ximian com>]

On Sun, 2004-06-06 at 13:36, Vardhman Jain wrote:
> Hi,
> 	Thanks Jeff, for the suggestions, I am submitting the modified 
> version again,
> 
> Vardhman

Just a few more nit-picks...


first up, we need a ChangeLog entry

> Index: gui/e-itip-control.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/calendar/gui/e-itip-control.c,v
> retrieving revision 1.145
> diff -u -r1.145 e-itip-control.c
> --- a/gui/e-itip-control.c      11 May 2004 22:05:03 -0000      1.145
> +++ b/gui/e-itip-control.c      6 Jun 2004 17:34:02 -0000
> @@ -679,6 +679,7 @@
>         EItipControlPrivate *priv;
>         ECalComponentDateTime datetime;
>         static char buffer[1024];
> +       gchar *str;
>         gboolean wrote = FALSE, task_completed = FALSE;
>         ECalComponentVType type;
>  
> @@ -689,20 +690,24 @@
>         buffer[0] = '\0';
>         e_cal_component_get_dtstart (comp, &datetime);
>         if (datetime.value) {
> +               str = g_strdup_printf ("<b> %s: </b>",_("Start"));

why the added whitespace around the %s: ?

>                 write_label_piece (itip, &datetime, buffer, 1024,
> -                                  _("<b>Starts:</b> "),
> +                                 str,
>                                    "<br>", FALSE);
>                 gtk_html_write (html, html_stream, buffer,
> strlen(buffer));
>                 wrote = TRUE;
> +               g_free(str);

this should probably be "g_free (str);" to keep with the style of the
surrounding code

>         }
>         e_cal_component_free_datetime (&datetime);
>  
>         buffer[0] = '\0';
>         e_cal_component_get_dtend (comp, &datetime);
>         if (datetime.value){
> -               write_label_piece (itip, &datetime, buffer, 1024,
> _("<b>Ends:</b> "), "<br>", FALSE);
> +               str = g_strdup_printf ("<b> %s: </b>",_("Ends"));

this has the extra whitespace in the string too. why?

> +               write_label_piece (itip, &datetime, buffer, 1024, str,
> "<br>", FALSE);
>                 gtk_html_write (html, html_stream, buffer, strlen
> (buffer));
>                 wrote = TRUE;
> +               g_free(str);

space before the (

>         }
>         e_cal_component_free_datetime (&datetime);
>  
> @@ -719,20 +724,24 @@
>         if (type == E_CAL_COMPONENT_TODO && datetime.value) {
>                 /* Pass TRUE as is_utc, so it gets converted to the
> current
>                    timezone. */
> +               str = g_strdup_printf ("<b> %s: </b>",_("Completed"));

whitespace in the string again? maybe this is intentional, but could you
explain why?

also put a space before the _("Completed")

>                 datetime.value->is_utc = TRUE;
> -               write_label_piece (itip, &datetime, buffer, 1024,
> _("<b>Completed:</b> "), "<br>", FALSE);
> +               write_label_piece (itip, &datetime, buffer, 1024, str,
> "<br>", FALSE);
>                 gtk_html_write (html, html_stream, buffer, strlen
> (buffer));
>                 wrote = TRUE;
>                 task_completed = TRUE;
> +               g_free(str);

g_free (str);

>         }
>         e_cal_component_free_datetime (&datetime);
>  
>         buffer[0] = '\0';
>         e_cal_component_get_due (comp, &datetime);
>         if (type == E_CAL_COMPONENT_TODO && !task_completed &&
> datetime.value) {
> -               write_label_piece (itip, &datetime, buffer, 1024,
> _("<b>Due:</b> "), "<br>", FALSE);
> +               str = g_strdup_printf ("<b> %s: </b>",_("Due"));

(whitespace in the string again)

> +               write_label_piece (itip, &datetime, buffer, 1024, str,
> "<br>", FALSE);
>                 gtk_html_write (html, html_stream, buffer, strlen
> (buffer));
>                 wrote = TRUE;
> +               g_free(str);

g_free (str);

>         }
>  
>         e_cal_component_free_datetime (&datetime);
> @@ -814,6 +823,7 @@
>         gchar *html;
>         const gchar *const_html;
>         gchar *filename;
> +       gchar *str;
>  
>         priv = itip->priv;
>  
> @@ -922,9 +932,12 @@
>  
>         /* Summary */
>         e_cal_component_get_summary (priv->comp, &text);
> -       html = text.value ? camel_text_to_html (text.value,
> CAMEL_MIME_FILTER_TOHTML_CONVERT_NL, 0) : _("<i>None</i>");
> +       str = g_strdup_printf ("<i> %s: </i>",_("None"));

should this one have the ':' ? it doesn't look like the original code
had one...

> +
> +       html = text.value ? camel_text_to_html (text.value,
> CAMEL_MIME_FILTER_TOHTML_CONVERT_NL, 0) : str;
>         gtk_html_stream_printf (html_stream,
> "<b>%s</b><br>%s<br><br>",
>                                 _("Summary:"), html);
> +       g_free(str);

g_free (str);

>         if (text.value)
>                 g_free (html);

Thanks for enduring my nit-picking :-)

Jeff

-- 
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
fejj ximian com  - www.ximian.com