Move conversation management from gtkconv.c to PidginConversationWindow.

Review Request #1108 — Created Oct. 28, 2021 and updated

grim
pidgin/pidgin
default
pidgin
This replaces the conversation window's notebook with a treeview and is the
first steps in keeping conversations alive even when they're closed in the
Pidgin ui.

Opened an im, closed the window, opened the same im again. Opened multiple ims and a chat, verified all functioned as expected.

Summary
Move conversation management from gtkconv.c to PidginConversationWindow.
Description From Last Updated

instead?

QuLogicQuLogic

Does pidgin_conversation_window_foreach_destroy effectively empty the tree model? Dispose may be called multiple times, so we should be certain we're safe ...

QuLogicQuLogic

Comparing to e.g., GtkStack, there is a visible-child property, and you can use a notify signal for this. Do we ...

QuLogicQuLogic

Double semicolon.

QuLogicQuLogic

Surely we don't need to name it purplewin (considering it's a PidginConversationWindow) now that all the lines with it have ...

QuLogicQuLogic
grim
grim
grim
grim
QuLogic
  1. Admittedly, I didn't check much of the .ui file, but that's not so important anyway if it'll be redesigned later.

  2. pidgin/pidginconversationwindow.h (Diff revision 4)
     
     

    instead?

  3. pidgin/pidginconversationwindow.c (Diff revision 4)
     
     

    Does pidgin_conversation_window_foreach_destroy effectively empty the tree model? Dispose may be called multiple times, so we should be certain we're safe to calling this again.

    1. Looks like we weren't emptying the list, but I'm updating it to remove the items now. That said, do you have any thoughts on how to test it being called multiple times? If you do it manually gtk complains and I'm not sure it actually calls it.

  4. pidgin/pidginconversationwindow.c (Diff revision 4)
     
     

    Comparing to e.g., GtkStack, there is a visible-child property, and you can use a notify signal for this. Do we want to do something like that?

    1. I could really go whichever way on this, but changing this right now means we need to update the notification plugin and a quick search of the plugin pack shows 3 plugins that want this signal too. I'm all for making it better/easier, just not sure right now is the right time for that?

  5. pidgin/pidginconversationwindow.c (Diff revision 4)
     
     

    Double semicolon.

  6. pidgin/plugins/unity.c (Diff revision 4)
     
     

    Surely we don't need to name it purplewin (considering it's a PidginConversationWindow) now that all the lines with it have changed?

  7. 
      
grim
Review request changed

Change Summary:

Addressed issues found in review

Commits:

Summary
-
Move conversation management from gtkconv.c to PidginConversationWindow.
+
Move conversation management from gtkconv.c to PidginConversationWindow.

Diff:

Revision 6 (+1540 -3522)

Show changes

Loading...