From: SourceForge.net <no...@so...> - 2005-03-07 18:14:17
|
Patches item #1091978, was opened at 2004-12-27 20:07 Message generated for change (Comment added) made by seanegan You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300235&aid=1091978&group_id=235 Category: None Group: None >Status: Closed >Resolution: Accepted Priority: 5 Submitted By: Richard Laager (rlaager) Assigned to: Sean Egan (seanegan) Summary: Log Viewing Enhancements Initial Comment: This patch adds a View Contact Log option to contact nodes. This option collates all the logs for a given contact's buddies. If accepted, this patch will obsolete track #1051898 <http://sourceforge.net/tracker/index.php?func=detail&aid=1051898&group_id=235&atid=390395> (NOTE: I haven't actually used that plug-in but according to the description, it does the same stuff.) ---------------------------------------------------------------------- >Comment By: Sean Egan (seanegan) Date: 2005-03-07 13:14 Message: Logged In: YES user_id=199625 Thanks, Richard! ---------------------------------------------------------------------- Comment By: Richard Laager (rlaager) Date: 2005-03-07 01:40 Message: Logged In: YES user_id=156487 Here's a new version of the patch. This version addresses several issues: (1) The GtkListStore is gone in favor of Sean's suggested approach. (2) It's "View Log" everywhere. No more of my "View Contact Log" crud. Buddies have a "View Log" that shows the logs for that buddy. Contacts have a "View Log" that shows the logs for that contact. Buddy sub-menus do not have any "View Log" option. Buddies under an expanded contact have a "View Log" option that allows one to view the logs for just that buddy. The expanded contact itself still has a "View Log" that shows contact logs. If this doesn't make sense now, it will if you apply this patch and test. (3) Intertwined into this code is a tiny new method and one signal hook installation that allows the user to double-click the months in the log viewer and have them expand/collapse. This matches how groups expand and collapse in the buddy list. This change was requested by Luke. (4) Also intertwined into this code is a tiny change to the buddy list that takes care of this comment by Mark: "I don't like how when I right-click on a buddy inside an expanded contact, the menu has both 'Alias Buddy' and 'Alias Contact'" (5) Related to (2) and (4), I tweaked the contact's right-click menu a bit to make it match the ordering and separator style of the other right-click menus. (6) The current log viewer looks bad when there are no logs because the tree on the left side collapses into almost zero space. Also, there's not much use in pulling up a log viewer that's completely empty. The system log viewer had the size of that tree hard-coded to avoid this issue. That caused other problems because then the tree doesn't auto-expand. It was also weird to special case that scenario now that I've combined the system log viewer display code with the display code for the other log viewers. Therefore, I've modified the log viewer to display nice messages when no logs exist. If the user has any particular logging disabled, it provides helpful advise on which preference to enable to start saving logs. (7) The log viewer window title is now one of "System Log", "Conversations with X" (for IMs), or "Conversations in X" (for chats." (8) When selecting a log, the descriptive label at the top of the log viewer is changed instead of the title. (9) I don't think the current log viewer takes into account chat aliases when making labels. This patch fixes that. (10) As I was coding today, I cleaned up some if statements by adding != NULL or == NULL where appropriate. This made it a lot easier for me to see what was happening. (11) I realigned stuff in gtklog.h. I realize this is mostly patch noise, but the end result is easier to follow. Things should either line up or not line up; having things that once lined up but don't anymore is frustrating. ---------------------------------------------------------------------- Comment By: Mark Doliner (thekingant) Date: 2005-03-01 23:35 Message: Logged In: YES user_id=20979 I agree with Stu and Sean. I wouldn't even mind getting rid of the right-click a contact->select-a-buddy submenus, but it's probably better to keep them. ---------------------------------------------------------------------- Comment By: Sean Egan (seanegan) Date: 2005-03-01 20:07 Message: Logged In: YES user_id=199625 I guess my thought is to completely get rid of "View buddy log" and replace it with "View contact log," calling it "View log," as "View contact log" is a direct superset of "view buddy log." I don't like having both labels in the menu, especially since nobody outside us knows what we mean by "contact." ---------------------------------------------------------------------- Comment By: Luke Schierer (lschiere) Date: 2005-03-01 19:43 Message: Logged In: YES user_id=28833 I don't particularly *want* to see a single buddy's log at all. but having "View Log" get a contact at the top level menu and a buddy at a submenu will confuse people. ---------------------------------------------------------------------- Comment By: Stu Tomlinson (nosnilmot) Date: 2005-03-01 19:31 Message: Logged In: YES user_id=309779 luke: why would you want to do that? or put another way, if you have people grouped into contacts, that implies they're the same person, and you'd want to view the contact log anyway. If you don't, then you'd want to expand them and view the specific log anyway. ---------------------------------------------------------------------- Comment By: Luke Schierer (lschiere) Date: 2005-03-01 17:31 Message: Logged In: YES user_id=28833 problem: if we do that, there is no way to view just the log of just the top buddy without expanding the contact and then right clicking, whereas any secondary buddy can be accessed via the submenu set up for it. ---------------------------------------------------------------------- Comment By: Mark Doliner (thekingant) Date: 2005-03-01 17:18 Message: Logged In: YES user_id=20979 I haven't been following this extremely closely. Do we all agree that the following is best? 1. Right clicking on a contact should show only one "View Log" option, that views the combined logs of all buddies in that contact 2. Right clicking on a buddy inside a contact should show only one "View Log" option, that views the logs of that buddy ---------------------------------------------------------------------- Comment By: Richard Laager (rlaager) Date: 2005-03-01 00:57 Message: Logged In: YES user_id=156487 You may be on to something with the suggestion that could eliminate the GtkListStore. I'll look into it. After more consideration, I think that just doing View Log is fine. I can see how the confusion to users outweighs the very minimal benefits of being able to view logs by both buddy and contact. I'll make that change. ---------------------------------------------------------------------- Comment By: Sean Egan (seanegan) Date: 2005-03-01 00:21 Message: Logged In: YES user_id=199625 Is the GtkListStore just for the hash table? I'd like to see that not exist. How about: char *screenname GaimAccount *account GaimContact *contact where screenname/account get used if contact is NULL. Maybe I'm being too picky, but it just seems like the patch could be a lot simpler. As for "View log" vs. "View Contact Log," maybe we should just get rid of "View log" altogether and say that "View Contact Log" supercedes it as a direct superset of its functionality. Thoughts? ---------------------------------------------------------------------- Comment By: Sean Egan (seanegan) Date: 2005-02-28 14:52 Message: Logged In: YES user_id=199625 Mark: His patch creates two menu options: "View log" and "View contact log," where the former views the log for the top-priority buddy. Richard: I'll look at this tonight. ---------------------------------------------------------------------- Comment By: Mark Doliner (thekingant) Date: 2005-01-29 17:02 Message: Logged In: YES user_id=20979 I got the impression he thought you should right-click on a contact and select "View Log" and it would show you all logs for all buddies within that contact. So the only difference is the name of the menu item ("View Log" verse "View Contact Log"). ---------------------------------------------------------------------- Comment By: Richard Laager (rlaager) Date: 2005-01-29 04:44 Message: Logged In: YES user_id=156487 I'm reopening this tracker item with a completely reworked patch. You should like this patch a lot more. I went with adding: gaim_gtk_log_show_contact(GaimContact *contact) Everything ugly about the last patch should be fixed now. As far as data storage goes, this version uses a GtkListStore. I was initially against this, but I finally realized that if I did as you said and called gaim_log_get_logs multiple times, I wouldn't need to mess with the core at all. Therefore, using a GTK object for data storage was totally acceptable. In the course of creating a helper function for the common code between gaim_gtk_log_show and gaim_gtk_log_show_contact, I was able to eliminate a lot of duplicated code in the system log function. This passes every test I can throw at it. I even implemented sorting (under the hood) in this one. This means you can open a log for a contact, reorder the buddies under that contact (while the log viewer is still open), and click View Contact Log again on that contact and it will simply raise the old log viewer instead of opening a new one. The only instruction of yours that I didn't follow was to name the option "View Log" instead of "View Contact Log". I think it's useful to be able to view logs by buddy and by contact. This also matches the current approach with Alias and Remove (as you noted). If you feel those need to go, I'm anxious to hear how you would do that without reducing functionality. Hypothetically, if that change were to be made, the log stuff could be changed at the same time. ---------------------------------------------------------------------- Comment By: Sean Egan (seanegan) Date: 2005-01-22 00:07 Message: Logged In: YES user_id=199625 I looked through it and failed to understand what it's doing. I don't like the crazy things you're doing with those three GLists, nor do I understand what their purpose is. From the skimming I did, I didn't even see any place where any of those lists would have more than one node. Also, I'm pretty sure that not only do your three lists leak, but you're not freeing the strings in "names," which you're strduping. For some reason. Also, you're attempting to identify a contact by its alias. That's way wrong. Really, this should be something straight-forward and simple. Make the log viewer contact-aware and give it a function to show a log belonging to a contact. Add this to the blist menu for contacts (Just as "View log," not as "View Contact Log," we try to keep the word "contact" out of the UI, and two "View Log" choices are confusing---I just noticed "Alias Buddy," "Alias Contact," "Remove Buddy," "Remove Contact." That's gotta go.). Of course, you can try to explain to me what these GLists are all about also. -s. ---------------------------------------------------------------------- Comment By: Richard Laager (rlaager) Date: 2005-01-21 18:05 Message: Logged In: YES user_id=156487 Attaching a new patch which is against HEAD instead of gaim-1.1.1. FYI, this feature addition breaks (GTK UI) API compatibility, so I recommend it be considered prior to the release of 2.0.0. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=300235&aid=1091978&group_id=235 |