Merge lp:~seb128/xchat-indicator/use-libmessaging-menu into lp:xchat-indicator

Proposed by Sebastien Bacher
Status: Merged
Approved by: Ken VanDine
Approved revision: 45
Merged at revision: 42
Proposed branch: lp:~seb128/xchat-indicator/use-libmessaging-menu
Merge into: lp:xchat-indicator
Diff against target: 526 lines (+123/-217)
2 files modified
configure.ac (+1/-2)
indicator.c (+122/-215)
To merge this branch: bzr merge lp:~seb128/xchat-indicator/use-libmessaging-menu
Reviewer Review Type Date Requested Status
Ken VanDine Pending
Review via email: mp+121441@code.launchpad.net

Description of the change

Use libmessaging-menu rather than libindicate

To post a comment you must log in.
Revision history for this message
Lars Karlitski (larsu) wrote :

Looks good to me from a libmessaging-menu point of view. A couple of comments:

* 342: Is xchat_plugin_deinit called every time the xchat-gnome process is stopped, or only when disabling the plugin? messaging_menu_app_unregister doesn't need to be called on app exit if the app should stay in the messaging menu. g_object_unref (mmapp) should be called, though.

* There's still a LIBINDICATE_REQUIRED variable in configure.ac.

These are not really important, just to let you know:

* 141, 166: append_source_with_time (mmapp, id, icon, label, g_get_real_time()) can be written as append_source (mmapp, id, icon, label) (i.e. current time is the default if nothing else is given)

* 102: has_source is not necessary before remove_source

Thanks for porting!

43. By Sebastien Bacher

updated to fix some of the issues pointed by the first code review

44. By Sebastien Bacher

use consistant tab spacing

Revision history for this message
Sebastien Bacher (seb128) wrote :

> * 342: Is xchat_plugin_deinit called every time the xchat-gnome process is stopped, or only when disabling the plugin?
> messaging_menu_app_unregister doesn't need to be called on app exit if the app should stay in the messaging menu.
> g_object_unref (mmapp) should be called, though.

good point, I changed for an unref call

> * There's still a LIBINDICATE_REQUIRED variable in configure.ac.

cleaned

> * 141, 166: append_source_with_time (mmapp, id, icon, label, g_get_real_time()) can be written as append_source
> (mmapp, id, icon, label) (i.e. current time is the default if nothing else is given)
> * 102: has_source is not necessary before remove_source

good to know, fixed as well, thanks

45. By Sebastien Bacher

try to handle focus_win_cb correctly...

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Has this been merged already ?!
/me avid xchat user

Revision history for this message
Sebastien Bacher (seb128) wrote :

it didn't get merged but got uploaded to Ubuntu for quantal

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configure.ac'
2--- configure.ac 2012-06-12 14:51:50 +0000
3+++ configure.ac 2012-08-30 11:13:25 +0000
4@@ -11,7 +11,6 @@
5
6 GLIB_REQUIRED=2.18.0
7 GTK_REQUIRED=2.14.0
8-LIBINDICATE_REQUIRED=0.6.90
9 UNITY_REQUIRED=3.4.0
10
11 AC_PROG_CC
12@@ -34,8 +33,8 @@
13
14 # Hard dependencies
15 PKG_CHECK_MODULES([DEPENDENCIES], [
16- indicate-0.7 >= $LIBINDICATE_REQUIRED
17 gtk+-2.0 >= $GTK_REQUIRED
18+ messaging-menu
19 ])
20 AC_SUBST([DEPENDENCIES_CFLAGS])
21 AC_SUBST([DEPENDENCIES_LIBS])
22
23=== modified file 'indicator.c'
24--- indicator.c 2012-06-12 08:12:19 +0000
25+++ indicator.c 2012-08-30 11:13:25 +0000
26@@ -2,7 +2,7 @@
27 * indicator.c - xchat plugin to support the Messaging Indicator
28 *
29 * Copyright (C) 2009 Scott Parkerson <scott.parkerson@gmail.com>
30- * Copyright (C) 2009-2011 Canonical Ltd.
31+ * Copyright (C) 2009-2012 Canonical Ltd.
32 *
33 * This program is free software; you can redistribute it and/or
34 * modify it under the terms of the GNU General Public License
35@@ -21,9 +21,7 @@
36 */
37
38 #include <config.h>
39-#include <libindicate/server.h>
40-#include <libindicate/indicator-messages.h>
41-#include <libindicate/indicator.h>
42+#include <messaging-menu.h>
43 #include <string.h>
44 #include <glib/gi18n.h>
45 #include <gtk/gtk.h>
46@@ -40,8 +38,7 @@
47 int xchat_plugin_deinit (void);
48
49 static xchat_plugin *ph;
50-static GHashTable *indicators = NULL;
51-static IndicateServer *indicate_server = NULL;
52+static MessagingMenuApp *mmapp;
53 #ifdef HAVE_UNITY
54 static UnityLauncherEntry *launcher = NULL;
55 #endif
56@@ -54,41 +51,43 @@
57 static GtkWindow *win;
58 static gchar *focused;
59 static gboolean run = FALSE;
60-static void hide_indicator (const gchar *channel, gpointer indicator, gpointer data);
61-static void remove_indicator (const gchar *channel, gpointer indicator);
62-static void add_indicator (const gchar *channel);
63+static gint count = 0;
64+static void remove_indsource (gchar *sourceid);
65+static void add_indsource (gchar *sourceid);
66 static void really_activate_window (GtkWindow *win);
67 gboolean focus_win_cb (GtkWindow *win, GParamSpec *pspec, gpointer data);
68-
69+static xchat_context *stored_ctx = NULL;
70
71 static void
72-indicator_display (gpointer indicator, guint timestamp, const gchar *channel)
73+source_display (MessagingMenuApp *mmapp,
74+ const gchar *channel,
75+ gpointer user_data)
76 {
77 GtkWindow *win;
78
79- if (channel) {
80- xchat_context *current_ctx;
81- current_ctx = xchat_get_context(ph);
82- xchat_context *ctx;
83- ctx = xchat_find_context (ph, NULL, channel);
84- /*
85- g_debug ("INDICATOR: Changing to channel %s", channel);
86- */
87- win = (GtkWindow *)xchat_get_info (ph, "win_ptr");
88- if (xchat_set_context (ph, ctx)) {
89+ if (channel) {
90+ xchat_context *current_ctx;
91+ current_ctx = xchat_get_context(ph);
92+ xchat_context *ctx;
93+ ctx = xchat_find_context (ph, NULL, channel);
94 /*
95- g_debug ("xchat-indicator: Showing window");
96+ g_debug ("INDICATOR: Changing to channel %s", channel);
97 */
98- xchat_command (ph, "GUI SHOW");
99- xchat_command (ph, "GUI FOCUS");
100- gtk_widget_show (GTK_WIDGET (win));
101- really_activate_window (GTK_WINDOW (win));
102- /* gtk_window_present_with_time(GTK_WINDOW (win), timestamp); */
103+ win = (GtkWindow *)xchat_get_info (ph, "win_ptr");
104+ if (xchat_set_context (ph, ctx)) {
105+ /*
106+ g_debug ("xchat-indicator: Showing window");
107+ */
108+ xchat_command (ph, "GUI SHOW");
109+ xchat_command (ph, "GUI FOCUS");
110+ gtk_widget_show (GTK_WIDGET (win));
111+ really_activate_window (GTK_WINDOW (win));
112+ /* gtk_window_present_with_time(GTK_WINDOW (win), timestamp); */
113 } else {
114 g_warning ("xchat-indicator: context change fail");
115 }
116- if (indicator != NULL) {
117- remove_indicator (channel, indicator);
118+ if (channel != NULL) {
119+ remove_indsource (channel);
120 }
121 xchat_set_context (ph, current_ctx);
122 }
123@@ -98,51 +97,17 @@
124 }
125
126 static void
127-server_display (IndicateServer * indicate_server, guint timestamp, gpointer data)
128-{
129- GtkWindow *win = (GtkWindow *)xchat_get_info (ph, "win_ptr");
130- really_activate_window (GTK_WINDOW (win));
131- xchat_command (ph, "GUI SHOW");
132- /* gtk_window_present_with_time(GTK_WINDOW (win), timestamp); */
133- /*
134- g_debug ("xchat-indicator: Showing the window");
135- */
136- /* XXX: Should clear all indicators */
137-}
138-
139-static void
140-hide_indicator (const gchar *channel, gpointer indicator, gpointer data)
141-{
142- /*
143- g_debug ("xchat-indicator: hiding indicator");
144- */
145- indicate_indicator_hide (INDICATE_INDICATOR (indicator));
146-}
147-
148-static void
149-remove_indicator (const gchar *channel, gpointer indicator)
150-{
151- gpointer orig_channel, orig_indicator;
152-
153- if (g_hash_table_lookup_extended (indicators, channel,
154- &orig_channel, &orig_indicator)
155- && orig_indicator == indicator)
156+remove_indsource (gchar *channel)
157+{
158+ count = count - 1;
159+ messaging_menu_app_remove_source (MESSAGING_MENU_APP (mmapp), channel);
160+
161+#ifdef HAVE_UNITY
162+ if (launcher == NULL)
163 {
164- /*
165- g_debug ("xchat-indicator: removing indicator for channel %s", channel);
166- */
167- indicate_indicator_hide (INDICATE_INDICATOR (indicator));
168- g_hash_table_remove (indicators, channel);
169- g_object_unref (indicator);
170- g_free (orig_channel);
171+ return;
172 }
173-#ifdef HAVE_UNITY
174- if (launcher == NULL)
175- {
176- return;
177- }
178
179- gint count = g_hash_table_size (indicators);
180 /*
181 g_debug ("LAUNCHER: count is %d", count);
182 */
183@@ -164,35 +129,28 @@
184 }
185
186 static void
187-update_indicator (gpointer indicator)
188+update_indicator (gchar *sourceid)
189 {
190- GTimeVal time;
191- if (indicator != NULL)
192+ if (sourceid != NULL)
193 {
194 /*
195 g_debug ("xchat-indicator: found existing indicator");
196 */
197- g_get_current_time (&time);
198- indicate_indicator_set_property_time (INDICATE_INDICATOR (indicator),
199- "time", &time);
200+ messaging_menu_app_set_source_time (MESSAGING_MENU_APP (mmapp),
201+ sourceid, g_get_real_time ());
202 }
203
204 }
205
206 static void
207-add_indicator (const gchar *channel)
208+add_indsource (gchar *sourceid)
209 {
210- IndicateIndicator *indicator = NULL;
211- GTimeVal time;
212- gpointer chan;
213-
214- indicator = g_hash_table_lookup (indicators, channel);
215- if (indicator != NULL) {
216- update_indicator (indicator);
217+ if (messaging_menu_app_has_source(MESSAGING_MENU_APP (mmapp), sourceid)) {
218+ update_indicator (sourceid);
219 return;
220 }
221
222- if (!run)
223+ if (!run)
224 {
225 GtkWindow *win = (GtkWindow *)xchat_get_info (ph, "win_ptr");
226 if (GTK_IS_WINDOW (win))
227@@ -202,26 +160,9 @@
228 }
229 }
230
231- indicator = indicate_indicator_new ();
232- indicate_indicator_set_property (INDICATE_INDICATOR (indicator),
233- "subtype", "im");
234- indicate_indicator_set_property (INDICATE_INDICATOR (indicator),
235- "sender", channel);
236- indicate_indicator_set_property (INDICATE_INDICATOR (indicator),
237- "draw-attention", "true");
238-
239- chan = g_strdup (channel);
240-
241- g_signal_connect (G_OBJECT (indicator),
242- INDICATE_INDICATOR_SIGNAL_DISPLAY,
243- G_CALLBACK (indicator_display), chan);
244-
245- g_get_current_time (&time);
246- indicate_indicator_set_property_time (INDICATE_INDICATOR (indicator),
247- "time", &time);
248- indicate_indicator_show (INDICATE_INDICATOR (indicator));
249-
250- g_hash_table_insert(indicators, chan, indicator);
251+ count = count + 1;
252+ messaging_menu_app_append_source (MESSAGING_MENU_APP (mmapp), sourceid, NULL, sourceid);
253+ messaging_menu_app_draw_attention (MESSAGING_MENU_APP (mmapp), sourceid);
254
255 #ifdef HAVE_UNITY
256 if (launcher == NULL)
257@@ -229,7 +170,6 @@
258 return;
259 }
260
261- gint count = g_hash_table_size (indicators);
262 /*
263 g_debug ("LAUNCHER: count is %d", count);
264 */
265@@ -244,26 +184,17 @@
266 static int
267 nick_change_cb (char *word[], void *data)
268 {
269- gpointer indicator, key;
270- gboolean indicator_present;
271- xchat_context *ctx;
272 char *old_nick, *new_nick;
273
274 old_nick = word[1];
275 new_nick = word[2];
276
277- indicator_present = g_hash_table_lookup_extended (indicators,
278- old_nick, &key, &indicator);
279-
280- if (!indicator_present)
281- return XCHAT_EAT_NONE;
282-
283- g_hash_table_remove (indicators, old_nick);
284- g_free (key);
285- key = g_strdup (new_nick);
286- g_hash_table_insert (indicators, key, indicator);
287- indicate_indicator_set_property (INDICATE_INDICATOR (indicator),
288- "sender", new_nick);
289+ /* TODO: libmessaging doesn't allow renaming a source, so removing
290+ the old nick one and adding a new entry, that reset the time though */
291+ if (messaging_menu_app_has_source (MESSAGING_MENU_APP (mmapp), old_nick)) {
292+ messaging_menu_app_remove_source (MESSAGING_MENU_APP (mmapp), old_nick);
293+ messaging_menu_app_append_source (MESSAGING_MENU_APP (mmapp), new_nick, NULL, new_nick);
294+ }
295
296 return XCHAT_EAT_NONE;
297 }
298@@ -271,46 +202,45 @@
299 static int
300 indicate_msg_cb (char *word[], gpointer data)
301 {
302- GtkWindow *win;
303- xchat_context *ctx;
304- ctx = xchat_find_context (ph, NULL, word[2]);
305- xchat_set_context (ph, ctx);
306- const gchar *channel = xchat_get_info (ph, "channel");
307- win = (GtkWindow *)xchat_get_info (ph, "win_ptr");
308-
309+ GtkWindow *win;
310+ xchat_context *ctx;
311+ ctx = xchat_find_context (ph, NULL, word[2]);
312+ xchat_set_context (ph, ctx);
313+ gchar *channel = xchat_get_info (ph, "channel");
314+ win = (GtkWindow *)xchat_get_info (ph, "win_ptr");
315+
316 if (focused == channel && GTK_WINDOW (win)->has_toplevel_focus)
317 return XCHAT_EAT_NONE;
318
319- /*
320- g_debug ("xchat-indicator: channel %s is not focused, adding indicator", channel);
321- */
322- add_indicator (channel);
323+ add_indsource (channel);
324 return XCHAT_EAT_NONE;
325 }
326
327 static int
328 focus_tab_cb (char *word[], gpointer data)
329 {
330- gpointer indicator;
331- const gchar *channel;
332+ gchar *channel;
333 channel = xchat_get_info (ph, "channel");
334 /*
335 g_debug ("xchat-indicator: tab focused for channel %s", channel);
336 */
337- indicator = g_hash_table_lookup(indicators, channel);
338- if (indicator != NULL) {
339+ if (messaging_menu_app_has_source(MESSAGING_MENU_APP (mmapp), channel)) {
340 /*
341 g_debug ("xchat-indicator: found indicator for %s", channel);
342 */
343- remove_indicator (channel, indicator);
344- }
345+ remove_indsource (channel);
346+ }
347 focused = channel;
348- return XCHAT_EAT_NONE;
349+ /* store context to workaround focus_win_cb issues */
350+ stored_ctx = xchat_get_context (ph);
351+ return XCHAT_EAT_NONE;
352 }
353
354 gboolean
355 focus_win_cb (GtkWindow *win, GParamSpec *pspec, gpointer data)
356 {
357+ gchar *channel;
358+
359 if (!GTK_WINDOW (win)->has_toplevel_focus)
360 {
361 return FALSE;
362@@ -318,20 +248,26 @@
363 /*
364 g_debug ("xchat-indicator: window focused");
365 */
366- gpointer indicator;
367- const gchar *channel;
368- channel = xchat_get_info (ph, "channel");
369+
370+ /* restore the context of the previously focussed tab, not sure if xchat
371+ is buggy or doing it on purpose but it sets the current context,
372+ when receiving a message, to the channel which received the event and
373+ doesn't change it back to the selected tab on focus events.
374+ The focus_tab stored value should give us what we want though */
375+ if (stored_ctx)
376+ xchat_set_context (ph, stored_ctx);
377+
378+ channel = xchat_get_info (ph, "channel");
379 /*
380- g_debug ("xchat-indicator: tab focused for channel %s", channel);
381+ g_debug ("xchat-indicator: tab focused for channel %s", channel);
382 */
383- indicator = g_hash_table_lookup(indicators, channel);
384- if (indicator != NULL) {
385+ if (messaging_menu_app_has_source(MESSAGING_MENU_APP (mmapp), channel)) {
386 /*
387- g_debug ("xchat-indicator: found indicator for %s", channel);
388+ g_debug ("xchat-indicator: found indicator for %s", channel);
389 */
390- remove_indicator (channel, indicator);
391- }
392- focused = channel;
393+ remove_indsource (channel);
394+ }
395+ focused = channel;
396 return FALSE;
397 }
398
399@@ -339,35 +275,35 @@
400 static void
401 really_activate_window (GtkWindow *window)
402 {
403- Screen *screen;
404- Time timestamp;
405- XEvent xev;
406-
407- g_return_if_fail (GTK_IS_WINDOW (window));
408-
409- screen = GDK_SCREEN_XSCREEN (gtk_widget_get_screen (GTK_WIDGET (window)));
410- timestamp = GDK_CURRENT_TIME;
411-
412- xev.xclient.type = ClientMessage;
413- xev.xclient.serial = 0;
414- xev.xclient.send_event = True;
415- xev.xclient.display = GDK_DISPLAY ();
416- xev.xclient.window = GDK_WINDOW_XWINDOW (GTK_WIDGET (window)->window);
417- xev.xclient.message_type = gdk_x11_get_xatom_by_name ("_NET_ACTIVE_WINDOW");
418- xev.xclient.format = 32;
419- xev.xclient.data.l[0] = 2; /* Pager client type */
420- xev.xclient.data.l[1] = timestamp;
421- xev.xclient.data.l[2] = 0;
422- xev.xclient.data.l[3] = 0;
423- xev.xclient.data.l[4] = 0;
424-
425- gdk_error_trap_push ();
426- XSendEvent (GDK_DISPLAY (),
427- RootWindowOfScreen (screen),
428- False,
429- SubstructureRedirectMask | SubstructureNotifyMask,
430- &xev);
431- gdk_error_trap_pop ();
432+ Screen *screen;
433+ Time timestamp;
434+ XEvent xev;
435+
436+ g_return_if_fail (GTK_IS_WINDOW (window));
437+
438+ screen = GDK_SCREEN_XSCREEN (gtk_widget_get_screen (GTK_WIDGET (window)));
439+ timestamp = GDK_CURRENT_TIME;
440+
441+ xev.xclient.type = ClientMessage;
442+ xev.xclient.serial = 0;
443+ xev.xclient.send_event = True;
444+ xev.xclient.display = GDK_DISPLAY ();
445+ xev.xclient.window = GDK_WINDOW_XWINDOW (GTK_WIDGET (window)->window);
446+ xev.xclient.message_type = gdk_x11_get_xatom_by_name ("_NET_ACTIVE_WINDOW");
447+ xev.xclient.format = 32;
448+ xev.xclient.data.l[0] = 2; /* Pager client type */
449+ xev.xclient.data.l[1] = timestamp;
450+ xev.xclient.data.l[2] = 0;
451+ xev.xclient.data.l[3] = 0;
452+ xev.xclient.data.l[4] = 0;
453+
454+ gdk_error_trap_push ();
455+ XSendEvent (GDK_DISPLAY (),
456+ RootWindowOfScreen (screen),
457+ False,
458+ SubstructureRedirectMask | SubstructureNotifyMask,
459+ &xev);
460+ gdk_error_trap_pop ();
461 }
462
463 void
464@@ -385,19 +321,10 @@
465
466 ph = plugin_handle;
467 const gchar *desktop_id = g_strconcat(g_get_prgname(), ".desktop", NULL);
468- const gchar *desktop_file = g_strconcat("/usr/share/applications/", desktop_id, NULL);
469-
470- indicate_server = indicate_server_ref_default ();
471- indicate_server_set_type (indicate_server, "message.im");
472-
473- indicate_server_set_desktop_file (indicate_server,
474- desktop_file);
475- indicate_server_show (indicate_server);
476- indicators = g_hash_table_new(g_str_hash, g_str_equal);
477-
478- g_signal_connect (G_OBJECT (indicate_server),
479- INDICATE_SERVER_SIGNAL_SERVER_DISPLAY,
480- G_CALLBACK (server_display), NULL);
481+
482+ mmapp = messaging_menu_app_new (desktop_id);
483+ messaging_menu_app_register (MESSAGING_MENU_APP (mmapp));
484+ g_signal_connect (mmapp, "activate-source", G_CALLBACK (source_display), NULL);
485
486 #ifdef HAVE_UNITY
487 launcher = unity_launcher_entry_get_for_desktop_id (desktop_id);
488@@ -413,8 +340,8 @@
489 XCHAT_PRI_NORM, indicate_msg_cb, NULL);
490 tab_focus = xchat_hook_print (ph, "Focus Tab",
491 XCHAT_PRI_NORM, focus_tab_cb, NULL);
492- ch_nick = xchat_hook_print (ph, "Change Nick",
493- XCHAT_PRI_NORM, nick_change_cb, NULL);
494+ ch_nick = xchat_hook_print (ph, "Change Nick",
495+ XCHAT_PRI_NORM, nick_change_cb, NULL);
496
497 xchat_print (ph, (g_strjoin (" ", MESSAGING_INDICATOR_PLUGIN_NAME, MESSAGING_INDICATOR_PLUGIN_VERSION, _("plugin loaded."), NULL)));
498
499@@ -430,27 +357,7 @@
500 xchat_unhook (ph, pm_dialog);
501 xchat_unhook (ph, tab_focus);
502
503-
504- /* XXX: Need to clear all indicators */
505- GHashTableIter iter;
506- gpointer channel, indicator;
507-
508- g_hash_table_iter_init (&iter, indicators);
509- while (g_hash_table_iter_next (&iter, &channel, &indicator)) {
510- /*
511- g_debug ("xchat-indicator: removing indicator for channel %p", channel);
512- */
513- indicate_indicator_hide (INDICATE_INDICATOR (indicator));
514- g_free (channel);
515- }
516-
517-
518- /* Kill the hash table */
519- g_hash_table_destroy(indicators);
520-
521- g_print ("xchat-indicator: Hiding the indicator server");
522-
523- indicate_server_hide (indicate_server);
524+ g_object_unref (mmapp);
525
526 xchat_print (ph, (g_strjoin (" ", MESSAGING_INDICATOR_PLUGIN_NAME, MESSAGING_INDICATOR_PLUGIN_VERSION, _("plugin unloaded."), NULL)));
527

Subscribers

People subscribed via source and target branches

to all changes: