Merge lp://qastaging/~cszikszoy/do-plugins/pidgin-metacontacts into lp://qastaging/do-plugins

Proposed by Alex Launi
Status: Merged
Merged at revision: not available
Proposed branch: lp://qastaging/~cszikszoy/do-plugins/pidgin-metacontacts
Merge into: lp://qastaging/do-plugins
Diff against target: None lines
To merge this branch: bzr merge lp://qastaging/~cszikszoy/do-plugins/pidgin-metacontacts
Reviewer Review Type Date Requested Status
Alex Launi (community) Needs Fixing
Review via email: mp+4531@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
Chris S. (cszikszoy) wrote :

Lots of new features here.

* First and foremost, support for metacontacts
* Better alias assigning algorithm. Aliases are favored in the following order: Matacontact alias, Local alias, then Server alias. Should all of those be null, the contact's IM Handle will be used.
* Accounts shown in contact child items now show online status. "(Online)" or "(Offline") is shown.
* More readable descriptions for accounts. IE: Msn buddies now show up as "<name> (Msn)", not "<name> (prpl-msn)"
* contact childitem details now show buddy icon (if it exists) instead of the stock protocol (aim, msn) icon. If no icon exists, the msn/aim/etc icon will be shown.Fixed some issues here.
* Added an array length check when checking for an item.
* fixed formatting of ternary statements
* fixed another uint/int bug which caused pidgin to crash when trying to sign off an account
* added the ability to start a chat with a buddy with some initial text

This image: http://imagebin.org/41466 highlights a lot of the fixes.

Revision history for this message
Alex Launi (alexlauni) wrote :

diff line 45 should be >= 2, not > 2.
diff line 48 and 48 can just be one line, no need to break there.
In your dbus interface, arrange your methods a) logically, and b) by length. Having shortest first looks much nicer, put a line break when changing logical sets
in GetBuddyServerAlias, you should return empty string instead of null, make sure whoever is getting this method is using string.IsNullOrEmpty.
Stop using null in general when you mean empty string, you do it in OpenConversationWithBuddy too.
In InstanceIsRunning, why do you have all of that standard out redirection stuff? That looks like debugging info for writing this patch to me.
Constify "gtk-gaim" in PidginAccountActions.cs. Doesn't need to be global, just declare it at the top of that method.
diff line 145 and 160 I don't think you need to cast 1 to an int
In PidginChatAction.cs why do you have SupportsModifierItemForItems commented out? Also I don't think it's right. Why would your item be a text item? item should be a contact item.
diff line 246 I think you should use .OfType<Item> (), we used to use Cast, but if I recall correctly, OfType is safer.
You do buddy.Details.Where (d => d.StartsWith ("prpl-") && d.EndsWith ("-icon")) a couple of times, make yourself a DetailIsIcon (string detail) method instead of reusing variations of that line.
at diff line 264 you deleted a newline, add it back. it was easier to read
switch diff lines 284 and 285, whenever you're declaring things do the shorter first. You should go through your merge and looks for examples of this I missed, and fix them.
Your loop in CreateBuddy has some very deeply nested levels, can we try and keep it within 3 or 4 levels? Maybe make a method that reads the child nodes or something, I don't know. Take a look at it and try to make it a little bit cleaner. That's a huge method, it can definitely be refactored.
diff lines 429 through 429 can all be one line, I dont think the breaks are needed there
for PidginHandleContactDetailItem.cs, is there a way we can cache the correct icon instead of that conditional in Icon? Having logic in Name, Description, or Icon can cause noticable slow down when showing search results
In ReadableProto you should again check to make sure you dont walk off of the array, and if you DO, return just the proto string

review: Needs Fixing
573. By Chris S.

fixes as per Alex's review. Also ability to select multiple buddies for chat

574. By Chris S.

fixed some issues with contacts having no alias or name

575. By Chris Szikszoy <chris@dionysus>

pidgin path platform agnostic, use dbus to get path, not a hardcoded unix path, minor int/uint fix

576. By Chris Szikszoy <chris@dionysus>

remove some debugging stuff

577. By Chris Szikszoy <chris@dionysus>

platform independant combine path, figure out what's happening with set status action

578. By Chris Szikszoy <chris@dionysus>

some cleanup of debug lines

579. By Chris S.

rework set status action

580. By Chris S.

remove some comments, fix braces to conform to mono coding guidelines

581. By Chris S.

rework set status action (for the last time! :))

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Pidgin/Resources/Pidgin.addin.xml'
2--- Pidgin/Resources/Pidgin.addin.xml 2009-01-15 19:07:08 +0000
3+++ Pidgin/Resources/Pidgin.addin.xml 2009-03-16 02:41:13 +0000
4@@ -1,10 +1,10 @@
5 <Addin
6 id="Pidgin"
7 namespace="Do"
8- version="1.6"
9+ version="2.0"
10 name="Pidgin"
11 description="Search buddies and initiate chats in Pidgin."
12- author="David Siegel, Alex Launi"
13+ author="David Siegel, Alex Launi, Chris Szikszoy"
14 category="Official"
15 >
16
17
18=== modified file 'Pidgin/src/Pidgin.cs'
19--- Pidgin/src/Pidgin.cs 2009-03-01 20:28:07 +0000
20+++ Pidgin/src/Pidgin.cs 2009-03-16 04:22:47 +0000
21@@ -62,6 +62,9 @@
22 string PurpleAccountGetAlias (int account);
23 void PurpleAccountSetEnabled (int account, string ui, int value);
24 string PurpleAccountGetUsername (int account);
25+ string PurpleBuddyGetServerAlias (int buddy);
26+ void PurpleConvImSend (int im, string message);
27+ int PurpleConversationGetImData (int conv);
28
29 #region Pidgin < 2.5.4 compatibility methods
30
31@@ -78,13 +81,17 @@
32
33 public static string GetProtocolIcon (string proto)
34 {
35- string icon;
36+ string icon = null;
37
38 proto = proto.ToLower ();
39- if (proto.StartsWith ("prpl-"))
40- proto = proto.Substring ("prpl-".Length);
41- icon = Path.Combine (
42- "/usr/share/pixmaps/pidgin/protocols/48", proto + ".png");
43+ string[] parts = proto.Split ('-');
44+
45+ if (parts.Length > 2) {
46+ if (parts[0] == "prpl")
47+ proto = parts[1];
48+ icon = Path.Combine (
49+ "/usr/share/pixmaps/pidgin/protocols/48", proto + ".png");
50+ }
51 return File.Exists (icon) ? icon : Pidgin.ChatIcon;
52 }
53
54@@ -114,6 +121,24 @@
55 return connected.ToArray ();
56 }
57 }
58+
59+ public static string GetBuddyServerAlias (string name)
60+ {
61+ IPurpleObject prpl = GetPurpleObject ();
62+ int buddy;
63+ string alias;
64+
65+ if (!InstanceIsRunning)
66+ return null;
67+
68+ foreach (int account in ConnectedAccounts) {
69+ buddy = prpl.PurpleFindBuddy (account, name);
70+ if (buddy == 0) continue;
71+ alias = prpl.PurpleBuddyGetServerAlias (buddy);
72+ return (string.IsNullOrEmpty (alias)) ? null : alias;
73+ }
74+ return null;
75+ }
76
77 public static bool BuddyIsOnline (string name)
78 {
79@@ -166,8 +191,8 @@
80 } catch {
81 }
82 }
83-
84- public static void OpenConversationWithBuddy (string name)
85+
86+ public static void OpenConversationWithBuddy (string name, string message)
87 {
88 int account, conversation;
89 IPurpleObject prpl;
90@@ -183,6 +208,10 @@
91 catch {
92 conversation = prpl.PurpleConversationNew ((uint) 1, account, name);
93 }
94+ if (!string.IsNullOrEmpty (message)) {
95+ int im = prpl.PurpleConversationGetImData (conversation);
96+ prpl.PurpleConvImSend (im, message);
97+ }
98 prpl.PurpleConversationPresent (conversation);
99 } catch (Exception e) {
100 Log<Pidgin>.Error ("Could not create new Pidgin conversation: {0}", e.Message);
101@@ -190,17 +219,26 @@
102 }
103 }
104
105+ public static void OpenConversationWithBuddy (string name)
106+ {
107+ OpenConversationWithBuddy (name, null);
108+ }
109+
110 public static bool InstanceIsRunning
111 {
112 get {
113 Process pidof;
114-
115+ ProcessStartInfo pidofInfo = new ProcessStartInfo ("pidof", "pidgin");
116+ pidofInfo.UseShellExecute = true;
117+ pidofInfo.RedirectStandardError = true;
118+ pidofInfo.RedirectStandardOutput = true;
119+
120 try {
121 // Use pidof command to look for pidgin process. Exit
122 // status is 0 if at least one matching process is found.
123 // If there's any error, just assume some Purple client
124 // is running.
125- pidof = Process.Start ("pidof", "pidgin");
126+ pidof = Process.Start (pidofInfo);
127 pidof.WaitForExit ();
128 return pidof.ExitCode == 0;
129 } catch {
130
131=== modified file 'Pidgin/src/PidginAccountActions.cs'
132--- Pidgin/src/PidginAccountActions.cs 2009-03-01 20:28:07 +0000
133+++ Pidgin/src/PidginAccountActions.cs 2009-03-16 04:22:47 +0000
134@@ -63,15 +63,15 @@
135 public override IEnumerable<Item> Perform (IEnumerable<Item> items, IEnumerable<Item> modItems)
136 {
137 Pidgin.IPurpleObject prpl;
138+ PidginAccountItem account = items.First () as PidginAccountItem;
139+
140 try {
141 prpl = Pidgin.GetPurpleObject ();
142 try {
143- prpl.PurpleAccountSetEnabled ((items.First () as PidginAccountItem).Id,
144- "gtk-gaim", (int) 1);
145+ prpl.PurpleAccountSetEnabled (account.Id, "gtk-gaim", (int) 1);
146 }
147 catch {
148- prpl.PurpleAccountSetEnabled ((items.First () as PidginAccountItem).Id,
149- "gtk-gaim", (uint) 1);
150+ prpl.PurpleAccountSetEnabled (account.Id, "gtk-gaim", (uint) 1);
151 }
152 } catch (Exception e) {
153 Log<PidginEnableAccount>.Error ("Could not disable Pidgin account: {0}", e.Message);
154@@ -114,7 +114,12 @@
155
156 try {
157 prpl = Pidgin.GetPurpleObject ();
158- prpl.PurpleAccountSetEnabled (account.Id, "gtk-gaim", 0);
159+ try {
160+ prpl.PurpleAccountSetEnabled (account.Id, "gtk-gaim", (int) 0);
161+ }
162+ catch {
163+ prpl.PurpleAccountSetEnabled (account.Id, "gtk-gaim", (uint) 0);
164+ }
165 } catch (Exception e) {
166 Log<PidginDisableAccount>.Error ("Could not disable Pidgin account: {0}", e.Message);
167 Log<PidginDisableAccount>.Debug (e.StackTrace);
168
169=== modified file 'Pidgin/src/PidginChatAction.cs'
170--- Pidgin/src/PidginChatAction.cs 2009-01-15 19:07:08 +0000
171+++ Pidgin/src/PidginChatAction.cs 2009-03-16 04:22:47 +0000
172@@ -63,10 +63,29 @@
173 return true;
174 }
175
176+ public override IEnumerable<Type> SupportedModifierItemTypes {
177+ get { yield return typeof (ITextItem); }
178+ }
179+
180+ public override bool ModifierItemsOptional {
181+ get { return true; }
182+ }
183+
184+ /*
185+ public override bool SupportsModifierItemForItems (IEnumerable<Item> items, Item modItem)
186+ {
187+ return items.First () is ITextItem;
188+ }
189+ */
190+
191 public override IEnumerable<Item> Perform (IEnumerable<Item> items, IEnumerable<Item> modItems)
192 {
193 Item item = items.First ();
194- string name = null;
195+ string name, message;
196+ name = message = null;
197+
198+ if (modItems.Any ())
199+ message = (modItems.First () as ITextItem).Text;
200
201 if (item is ContactItem) {
202 // Just grab the first protocol we see.
203@@ -85,9 +104,12 @@
204 if (name != null) {
205 Services.Application.RunOnThread (() => {
206 Pidgin.StartIfNeccessary ();
207- Services.Application.RunOnMainThread (() =>
208- Pidgin.OpenConversationWithBuddy (name)
209- );
210+ Services.Application.RunOnMainThread (() => {
211+ if (!string.IsNullOrEmpty (message))
212+ Pidgin.OpenConversationWithBuddy (name, message);
213+ else
214+ Pidgin.OpenConversationWithBuddy (name);
215+ });
216 });
217 }
218 yield break;
219
220=== modified file 'Pidgin/src/PidginContactItemSource.cs'
221--- Pidgin/src/PidginContactItemSource.cs 2009-01-15 22:39:03 +0000
222+++ Pidgin/src/PidginContactItemSource.cs 2009-03-16 04:22:47 +0000
223@@ -71,13 +71,23 @@
224 get { return buddies; }
225 }
226
227+ private PidginHandleContactDetailItem MakeChildren (ContactItem buddy, string proto, IEnumerable<string> icons)
228+ {
229+ return (icons.Contains (proto+"-icon"))
230+ ? new PidginHandleContactDetailItem (proto, buddy[proto], buddy[proto+"-icon"])
231+ : new PidginHandleContactDetailItem (proto, buddy[proto]);
232+ }
233+
234 public override IEnumerable<Item> ChildrenOfItem (Item item)
235 {
236 ContactItem buddy = item as ContactItem;
237+
238+ IEnumerable<string> icons = buddy.Details.Where (d => d.StartsWith ("prpl-") && d.EndsWith ("-icon"));
239+
240 return buddy.Details
241- .Where (d => d.StartsWith ("prpl-"))
242- .Select (d => new PidginHandleContactDetailItem (d, buddy [d]))
243- .Cast<Item> ();
244+ .Where (d => d.StartsWith ("prpl-") && !d.EndsWith ("-icon"))
245+ .Select (d => MakeChildren (buddy, d, icons))
246+ .Cast<Item> ();
247 }
248
249 public override void UpdateItems ()
250@@ -92,15 +102,13 @@
251 try {
252 blist.Load (BuddyListFile);
253
254- foreach (XmlNode contact_node in blist.GetElementsByTagName ("contact"))
255- foreach (XmlNode buddy_node in contact_node.ChildNodes) {
256- ContactItem buddy;
257-
258- buddy = ContactItemFromBuddyXmlNode (buddy_node);
259- if (buddy == null) continue;
260- buddies_seen[buddy] = true;
261- }
262-
263+ foreach (XmlNode contact_node in blist.GetElementsByTagName ("contact")) {
264+ ContactItem buddy;
265+ buddy = CreateBuddy (contact_node);
266+ if (buddy == null) continue;
267+ buddies_seen[buddy] = true;
268+ }
269+
270 } catch (Exception e) {
271 Log<PidginContactItemSource>.Error ("Could not read Pidgin buddy list file: {0}", e.Message);
272 Log<PidginContactItemSource>.Debug (e.StackTrace);
273@@ -110,45 +118,82 @@
274 }
275 }
276
277- ContactItem ContactItemFromBuddyXmlNode (XmlNode buddy_node)
278+
279+ ContactItem CreateBuddy(XmlNode buddyNode)
280 {
281 ContactItem buddy;
282- string proto, name, alias, icon;
283+ string name, alias, icon, proto;
284+ Dictionary<string, string> protos = new Dictionary<string, string> ();
285+ Dictionary<string, string> icons = new Dictionary<string, string> ();
286
287+ alias = icon = name = null;
288+ //metacontact alias
289 try {
290- name = alias = icon = null;
291- // The messaging protocol (e.g. "prpl-jabber" for Jabber).
292- proto = buddy_node.Attributes.GetNamedItem ("proto").Value;
293- foreach (XmlNode attr in buddy_node.ChildNodes) {
294- switch (attr.Name) {
295- // The screen name.
296- case "name":
297- name = attr.InnerText;
298- break;
299- // The alias, or real name.
300- case "alias":
301- alias = attr.InnerText;
302- break;
303- // Buddy icon image file.
304- case "setting":
305- if (attr.Attributes.GetNamedItem ("name").Value == "buddy_icon") {
306- icon = Path.Combine (BuddyIconDirectory, attr.InnerText);
307+ alias = buddyNode.Attributes.GetNamedItem ("alias").Value;
308+ }
309+ catch {}
310+
311+ foreach (XmlNode node in buddyNode.ChildNodes)
312+ {
313+ switch (node.Name) {
314+ case "buddy":
315+ proto = node.Attributes.GetNamedItem ("proto").Value;
316+ //metacontacts
317+ int similarProtos = protos.Keys.Where (k => k.StartsWith (proto)).Count ();
318+ if (similarProtos > 0)
319+ proto = string.Format ("{0}-{1}", proto, similarProtos.ToString ());
320+ foreach (XmlNode attr in node.ChildNodes) {
321+ switch (attr.Name) {
322+ // The screen name.
323+ case "name":
324+ protos[proto] = attr.InnerText;
325+ break;
326+ // The alias, or real name, only if one isn't set yet.
327+ case "alias":
328+ if (string.IsNullOrEmpty (alias))
329+ alias = attr.InnerText;
330+ break;
331+ // Buddy icon image file.
332+ case "setting":
333+ if (attr.Attributes.GetNamedItem ("name").Value == "buddy_icon") {
334+ icons[proto+"-icon"] = Path.Combine (BuddyIconDirectory, attr.InnerText);
335+ if (!icons.Keys.Contains ("default"))
336+ icons["default"] = icons[proto+"-icon"];
337+ }
338+ break;
339 }
340- break;
341- }
342+ }
343+ //we favor aliases in this order: metacontact alias, local alias, server alias
344+ //if the alias is still null, let's try to get the server alias
345+ if (string.IsNullOrEmpty (alias))
346+ alias = Pidgin.GetBuddyServerAlias (protos[proto]) ?? null;
347+ break;
348+ //let's pick up the custom icon as the metacontact's icon
349+ case "setting":
350+ if (node.Attributes.GetNamedItem ("name").Value == "custom_buddy_icon") {
351+ icons["default"] = Path.Combine (BuddyIconDirectory, node.InnerText);
352+ }
353+ break;
354 }
355- } catch {
356- // Bad buddy.
357- return null;
358 }
359+
360+ //in case we don't have an alias, take one of the proto values for the name
361+ name = alias ?? protos.Values.FirstOrDefault ();
362+
363 // If crucial details are missing, we can't make a buddy.
364- if (name == null || proto == null) return null;
365+ if (name == null || protos.Values.Count () < 0) return null;
366
367 // Create a new buddy, add the details we have.
368 buddy = ContactItem.Create (alias ?? name);
369- if (icon != null)
370- buddy["photo"] = icon;
371- buddy[proto] = name;
372+ if (icons.Keys.Contains ("default"))
373+ buddy["photo"] = icons["default"];
374+
375+ foreach (string k in protos.Keys)
376+ buddy[k] = protos[k];
377+
378+ //add the icons keys to create individual icons for childitems
379+ foreach (string k in icons.Keys.Where (k => k != "default"))
380+ buddy[k] = icons[k];
381
382 return buddy;
383 }
384
385=== modified file 'Pidgin/src/PidginHandleContactDetailItem.cs'
386--- Pidgin/src/PidginHandleContactDetailItem.cs 2009-01-15 19:07:08 +0000
387+++ Pidgin/src/PidginHandleContactDetailItem.cs 2009-03-16 04:22:47 +0000
388@@ -19,6 +19,7 @@
389 //
390
391 using System;
392+using Mono.Unix;
393
394 using Do.Universe;
395
396@@ -28,12 +29,17 @@
397 class PidginHandleContactDetailItem : Item, IContactDetailItem
398 {
399
400- string proto, handle;
401-
402- public PidginHandleContactDetailItem (string proto, string handle)
403+ string proto, handle, custom_icon;
404+
405+ public PidginHandleContactDetailItem (string proto, string handle) : this (proto, handle, null)
406+ {
407+ }
408+
409+ public PidginHandleContactDetailItem (string proto, string handle, string custom_icon)
410 {
411 this.proto = proto;
412 this.handle = handle;
413+ this.custom_icon = custom_icon;
414 }
415
416 public override string Name {
417@@ -44,14 +50,21 @@
418
419 public override string Description {
420 get {
421- return ReadableProto (proto) + " " + "Handle" ;
422+ string online = (Pidgin.BuddyIsOnline (handle))
423+ ? Catalog.GetString ("Online")
424+ : Catalog.GetString ("Offline");
425+
426+ return string.Format ("{0} {1} ({2})",
427+ ReadableProto (proto),
428+ Catalog.GetString ("Handle"),
429+ online);
430 }
431 }
432
433 public override string Icon {
434- get { return Pidgin.GetProtocolIcon (proto); }
435+ get { return custom_icon ?? Pidgin.GetProtocolIcon (proto); }
436 }
437-
438+
439 public string Key {
440 get { return proto; }
441 }
442@@ -62,11 +75,8 @@
443
444 string ReadableProto (string proto)
445 {
446- switch (proto) {
447- case "prpl-aim": return "AIM";
448- case "prpl-jabber": return "Jabber";
449- default: return proto;
450- }
451+ string[] parts = proto.Split ('-');
452+ return char.ToUpper (parts[1][0]) + parts[1].Substring(1);
453 }
454 }
455 }

Subscribers

People subscribed via source and target branches