Comment on attachment 144536 [details] [review]
Switch to dbus

<snip>
>+			gtk_tree_store_set (store, iter,
>+					    0, icon,
>+					    1, alias_e->str, 
>+					    2, dat->alias,
>+					    -1);
> 		}
> 		g_string_free(alias_e, TRUE);
> 	}
>@@ -207,7 +294,7 @@ get_contacts_widget (NstPlugin *plugin)
> 
> 	iter = g_malloc (sizeof(GtkTreeIter));
> 	iter2 = g_malloc (sizeof(GtkTreeIter));
>-	store = gtk_tree_store_new (2, GDK_TYPE_PIXBUF, G_TYPE_STRING);
>+	store = gtk_tree_store_new (3, GDK_TYPE_PIXBUF, G_TYPE_STRING, G_TYPE_STRING);

Could you change the magic numbers by an enum?

enum {
   COL_ICON,
   COL_ALIAS,
   COL_WHATEVER,
   NUM_COLS
};

<snip>
>-static gboolean
>-send_files (NstPlugin *plugin,
>-	    GtkWidget *contact_widget,
>+static
>+gboolean send_file(int account, const char *who, const char *filename)

Can the code not send groups of files, as opposed to a single file?

<snip>
>+	for(file_iter = file_list; file_iter != NULL; 
>+	    file_iter = g_list_next(file_iter)) {
>+		error= NULL;
>+		file_path = g_filename_from_uri(file_iter->data, NULL, &error);

Don't do that, you'd lose the ability to send files located on remote network shares.

Do something like:
GFile *file;
file = g_file_new_for_uri (file_iter->data);
path = g_file_get_path (file);
g_object_unref (file);
/* check if file is null and print a warning */

>+		if(error != NULL) {
>+			g_warning("[Pidgin] %d Unable to convert URI `%s' to absolute file path",
>+				  error->code, (gchar *)file_iter->data);
>+			g_error_free(error);
>+			continue;
>+		}

Rest looks fine.

If the functionality is at least as good as it used to be, then it's fine to work on other bugs incrementally.

(In reply to comment #1)
> Could you change the magic numbers by an enum?
> 
> enum {
>    COL_ICON,
>    COL_ALIAS,
>    COL_WHATEVER,
>    NUM_COLS
> };
> 
yes (I did in my other version :) )

> <snip>
> >-static gboolean
> >-send_files (NstPlugin *plugin,
> >-	    GtkWidget *contact_widget,
> >+static
> >+gboolean send_file(int account, const char *who, const char *filename)
> 
> Can the code not send groups of files, as opposed to a single file?

yes, I still have send_files later in the code 

> <snip>
> >+	for(file_iter = file_list; file_iter != NULL; 
> >+	    file_iter = g_list_next(file_iter)) {
> >+		error= NULL;
> >+		file_path = g_filename_from_uri(file_iter->data, NULL, &error);
> 
> Don't do that, you'd lose the ability to send files located on remote network
> shares.

I think original code had the same issue :)
 
> Do something like:
> GFile *file;
> file = g_file_new_for_uri (file_iter->data);
> path = g_file_get_path (file);
> g_object_unref (file);
> /* check if file is null and print a warning */

OK

(In reply to comment #2)
> > <snip>
> > >-static gboolean
> > >-send_files (NstPlugin *plugin,
> > >-	    GtkWidget *contact_widget,
> > >+static
> > >+gboolean send_file(int account, const char *who, const char *filename)
> > 
> > Can the code not send groups of files, as opposed to a single file?
> 
> yes, I still have send_files later in the code 

I meant, isn't there a Pidgin API to send multiple files at once, rather than sending them one-by-one?

No they have a "transfer" object with a filename field

OK, would be fine with the other changes mentioned above then, thanks.

Created attachment 144550 [details] [review]
Switch to dbus, second try

OK, here is the full patch

Looks fine to me. Out of curiosity, why did the previous patch's tree store have 3 columns when this one only has 2?

Because I had fixed the crash in a stupid way, the model used to contain manually ellipsized alias which did not allow to identify the contact, so I added the full one.

I only thought afterwards of only storing it, and setting the ellipsize operation on the renderer...

There's 2 other bugs related to Pidgin in nautilus-sendto, would be nice if you could look into those, thanks!