Re: [evolution-patches] Patch for bug/bounty #127536

[Not Zed <notzed ximian com>]

Hi, just some comments below to deal with our analities.

Hint ... please attach patches with 'view inline' set, then its easier to reply and quote them.

On Sun, 2003-11-30 at 10:24, ERDI Gergo wrote:
> Index: mail/ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
> retrieving revision 1.2898
> diff -u -r1.2898 ChangeLog
> --- mail/ChangeLog	24 Nov 2003 01:38:11 -0000	1.2898
> +++ mail/ChangeLog	24 Nov 2003 15:47:42 -0000
> @@ -1,3 +1,8 @@
> +2003-11-24  ERDI Gergo  <cactus cactus rulez org>
> +
> +	* em-folder-view.c (emfv_on_url_cb): Send a BonoboUI status
> +	message when the user mouses over a URL
> +

Need to include changelog entries for the other changes too.

>  2003-11-24  Radek Doulik  <rodo ximian com>
>  
>  	* em-format-html.c (efh_text_plain): use new colors
> Index: mail/em-folder-view.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-folder-view.c,v
> retrieving revision 1.14
> diff -u -r1.14 em-folder-view.c
> --- mail/em-folder-view.c	14 Nov 2003 16:20:17 -0000	1.14
> +++ mail/em-folder-view.c	24 Nov 2003 15:47:45 -0000
> @@ -102,6 +102,11 @@
>  
>  static void emfv_setting_setup(EMFolderView *emfv);
>  
> +static void emfv_on_url_cb(GObject *emitter, const gchar *url, EMFolderView *emfv);
> +static gchar * get_url_hint (const gchar *url);
> +
> +
> +


^^ use basic c types for basic c types.  char, int, etc.

Also, only add 1 blank line, not three.

>  static const EMFolderViewEnable emfv_enable_map[];
>  
>  struct _EMFolderViewPrivate {
> @@ -147,6 +152,7 @@
>  	emfv->preview = (EMFormatHTMLDisplay *)em_format_html_display_new();
>  	g_signal_connect(emfv->preview, "link_clicked", G_CALLBACK(emfv_format_link_clicked), emfv);
>  	g_signal_connect(emfv->preview, "popup_event", G_CALLBACK(emfv_format_popup_event), emfv);
> +	g_signal_connect (G_OBJECT (emfv->preview->formathtml.html), "on_url",G_CALLBACK (emfv_on_url_cb), emfv);

Do not use the G_OBJECT() cast here (its actually expicitly not needed
by g_signal anymore).  You might've noticed its not used above, stick
to the same style.

>  	p->invisible = gtk_invisible_new();
>  	g_object_ref(p->invisible);
> @@ -1545,6 +1551,7 @@
>  		e_charset_picker_bonobo_ui_populate (uic, "/menu/View", _("Default"), emfv_charset_changed, emfv);
>  
>  		emfv_enable_menus(emfv);
> +		bonobo_ui_component_set_translate (uic, "/", "<status><item name=\"main\"/></status>", NULL);
>  	} else {
>  		const BonoboUIVerb *v;
>  
> @@ -2012,4 +2019,46 @@
>  								(GConfClientNotifyFunc)emfv_setting_notify,
>  								emfv, NULL, NULL);
>  	g_object_unref(gconf);
> +}
> +
> +

again, only 1 line, not 2.

> +static void
> +emfv_on_url_cb(GObject *emitter, const gchar *url, EMFolderView *emfv)
> +{
> +	char *nice_url = url ? get_url_hint (url) : 0;

there's really no need to split out get_url_hint, its 3 lines of code
(including an else) if the changes below are made.

> +	if (emfv->uic)
> +	{

use proper k&r style indenting please.

> +		bonobo_ui_component_set_status (emfv->uic, nice_url, NULL);
> +		/* Make sure the node keeps existing if nice_url == 0 */
> +		if (!nice_url)
> +			bonobo_ui_component_set_translate (emfv->uic, "/",
> +							   "<status>\n"
> +							   "  <item name=\"main\"/>\n"
> +							   "</status>",
> +							   NULL);

Shouldn't it just set the the string to "" rather than NULL, and avoid this set_translate stuff?

> +	}
> +	
> +	g_free (nice_url);
> +}
> +
> +static gchar *
> +get_url_hint (const gchar *url)
> +{
> +	gchar       *scheme = 0, *scheme_end, *retval;
> +	
> +	g_return_val_if_fail (url != 0, g_strdup (""));
> +	
> +	scheme_end = index (url, ':');

use strchr(), index() is bsd, and less portable.

actually you don't need to do any of this, just use

if (strncmp(url, "mailto:", 7) == 0)
 ...

as it performs the identical logic, and is how it's done in many other places.

> +	if (scheme_end)	
> +		scheme = g_strndup (url, scheme_end - url);
> +	
> +	if (scheme && !strcmp (scheme, "mailto"))
> +		retval = g_strdup_printf (_("Click to mail %s"), scheme_end + 1);
> +	else
> +		retval = g_strdup_printf (_("Click to open %s"), url);
> +	
> +	g_free (scheme);
> +	
> +	return retval;
>  }
>