Summary

Move conversation management from gtkconv.c to PidginConversationWindow.

Review Request #1108 — Created Oct. 28, 2021 and submitted Dec. 7, 2021, 5:15 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

Reviewers

Groups

pidgin

People

Description

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.

Testing Done

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

Commits

Summary	ID
Move conversation management from gtkconv.c to PidginConversationWindow. 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.	bea831d8f9f7a4200fc96af25b7b91d6f34317f4

Issues

Description	From	Last Updated
instead?	QuLogic	Dec. 6, 2021, 9:16 a.m.
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 …	QuLogic	Dec. 7, 2021, 2:51 a.m.
Comparing to e.g., GtkStack, there is a visible-child property, and you can use a notify signal for this. Do we …	QuLogic	Dec. 7, 2021, 5:06 a.m.
Double semicolon.	QuLogic	Dec. 6, 2021, 9:16 a.m.
Surely we don't need to name it purplewin (considering it's a PidginConversationWindow) now that all the lines with it have …	QuLogic	Dec. 6, 2021, 9:17 a.m.

Change Summary:

rebased

Commits:

	Summary	ID
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	830c64f6659ea19ddc57a44658492f074c591ffa
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	ce022af5655717f2da906b317c9cd5447de766bd

Diff:

Revision 2 (+1540 -3532)

Show changes

	ChangeLog.API
	pidgin/gtkconv.h
	pidgin/gtkconv.c
	pidgin/gtkconvwin.h
	pidgin/meson.build
	pidgin/pidginconversationwindow.h
	pidgin/pidginconversationwindow.c
	pidgin/pidgininfopane.c
	7 more

Change Summary:

rebased

Commits:

	Summary	ID
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	ce022af5655717f2da906b317c9cd5447de766bd
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	2835561522dc315dc01ee250580038c8d031c52c

Diff:

Revision 3 (+1540 -3532)

Show changes

	ChangeLog.API
	pidgin/gtkconv.h
	pidgin/gtkconv.c
	pidgin/gtkconvwin.h
	pidgin/meson.build
	pidgin/pidginconversationwindow.h
	pidgin/pidginconversationwindow.c
	pidgin/pidgininfopane.c
	7 more

Change Summary:

rebased

Commits:

	Summary	ID
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	2835561522dc315dc01ee250580038c8d031c52c
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	37776707eb10ee3aab6065e58e7c6fb20f0bcfcc

Diff:

Revision 4 (+1540 -3532)

Show changes

	ChangeLog.API
	pidgin/gtkconv.h
	pidgin/gtkconv.c
	pidgin/gtkconvwin.h
	pidgin/meson.build
	pidgin/pidginconversationwindow.h
	pidgin/pidginconversationwindow.c
	pidgin/pidgininfopane.c
	7 more

Change Summary:

rebased

Commits:

	Summary	ID
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	37776707eb10ee3aab6065e58e7c6fb20f0bcfcc
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	0b483f227502533e5f17a7bae113d6da90d475f2

Diff:

Revision 5 (+1536 -3522)

Show changes

	ChangeLog.API
	pidgin/gtkconv.h
	pidgin/gtkconv.c
	pidgin/gtkconvwin.h
	pidgin/meson.build
	pidgin/pidginconversationwindow.h
	pidgin/pidginconversationwindow.c
	pidgin/pidgininfopane.c
	7 more

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

pidgin/pidginconversationwindow.h (Diff revision 4)
Show all issues
```
instead?
```

pidgin/pidginconversationwindow.c (Diff revision 4)

Show all issues

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.

grim Dec. 5, 2021, 11 a.m.

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.

QuLogic Dec. 7, 2021, 2:51 a.m.

I mean, that has to do with GObject cleanup, so I don't know if we can trigger it twice.

pidgin/pidginconversationwindow.c (Diff revision 4)

Show all issues

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?

grim Dec. 5, 2021, 11 a.m.

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?

QuLogic Dec. 7, 2021, 2:51 a.m.

Do they not already need to be changed anyway? I guess if not broken, then we could keep this as is.

grim Dec. 7, 2021, 4:18 a.m.

well they're using the conversation-switched signal, which could be changed, but they should be working as is right now.

QuLogic Dec. 7, 2021, 5:06 a.m.
```
OK, let's leave it for now then.
```

pidgin/pidginconversationwindow.c (Diff revision 4)
Show all issues
```
Double semicolon.
```

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

Show all issues

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

Change Summary:

Addressed issues found in review

Commits:

	Summary	ID
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	0b483f227502533e5f17a7bae113d6da90d475f2
	Move conversation management from gtkconv.c to PidginConversationWindow. 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.	bea831d8f9f7a4200fc96af25b7b91d6f34317f4

Diff:

Revision 6 (+1540 -3522)

Show changes

	ChangeLog.API
	pidgin/gtkconv.h
	pidgin/gtkconv.c
	pidgin/gtkconvwin.h
	pidgin/meson.build
	pidgin/pidginconversationwindow.h
	pidgin/pidginconversationwindow.c
	pidgin/pidgininfopane.c
	7 more

Ship it!

```
Ship It!
```

Review Board 6.0.2

Move conversation management from gtkconv.c to PidginConversationWindow.

Information

Reviewers

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Status: Closed (submitted)