[Pidgin] #13134: New Log Viewer GUI

[Pidgin]

#13134: New Log Viewer GUI
-----------------------------------------+---------------------------
 Reporter:  wyuka                        |       Owner:  rlaager
     Type:  patch                        |      Status:  new
Milestone:  Patches Needing Improvement  |   Component:  pidgin (gtk)
  Version:  2.7.9                        |  Resolution:
 Keywords:  log viewer logs              |
-----------------------------------------+---------------------------
Changes (by QuLogic):

 * milestone:  Patches Needing Review => Patches Needing Improvement


Comment:

 While I like the idea behind this plugin, I'm not sure the implementation
 is 100% there yet.
 * "For:" is not a very good description for the first entry on the Search
 tab.
 * It's not totally discoverable what those entries mean on the
 Conversations tab.
 * The Find image on the right of the entry doesn't mean anything to me,
 plus it doesn't even seem to be a button.
 * The Delete button is in a not-too-obvious place.
 * Would be nice to have a 'skip-to-next/previous-conversation' button
 somewhere, especially if you've not chatted in more than a month or two.
 * There are warnings printed in the debug window when you open the log
 window (and maybe some other times).
 * The code is indented haphazardly. This is a symptom of mixing tabs and
 spaces for indent. Please use only tabs for indent. Spaces are fine for
 alignment (of continued lines or enums or similar).
 * `uint` is not a standard type.
 * There are several times you do something like this:
 {{{
 for buddy in contact:
   all_logs += buddy->logs;

 for log in all_logs:
   <do something>
 }}}
  That's inefficient since you need to traverse `all_logs` several times.
 It'd be better to do:
 {{{
 for buddy in contact:
   for log in buddy->logs:
     <do something>
 }}}
  Of course, that won't work when you need to sort things.
 * There are a lot of calls to `localtime` in ternary operators. Surely
 this calls for a temporary variable there instead. Or maybe a function.
 * `g_strup` is deprecated. It's not UTF-8 compatible. I'm sure we have a
 better buddy matching function used in the New Instant Message dialog (and
 View User Log).
 * FYI, you don't need to put braces around an `#if` block. It's not the
 same as `if`.

 Also, if we want to get this working in 3.0.0 (which would be a good
 idea), you'd need to replace the `imhtml` with a `webview`. The upside is
 that you are free to set up the async logging stuff if you feel like it.

-- 
Ticket URL: <https://developer.pidgin.im/ticket/13134#comment:7>
Pidgin <http://pidgin.im>
Pidgin