Summary

Rework the way roomlists work so we can more easily port them to GTK4

Review Request #1293 — Created Feb. 8, 2022 and submitted May 3, 2022, 2:59 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

Reviewers

Groups

pidgin

People

Description

Rework the way roomlists work so we can more easily port them to GTK4

Testing Done

Joined rooms on XMPP via the buttons, double clicking, and the context menu.

Commits

Summary	ID
Rework the way roomlists work so we can more easily port them to GTK4	85cd7833df73e33edfedff861455ea4e4941cce1

Issues

Description	From	Last Updated
To be done now, or ...?	QuLogic	March 13, 2022, 12:03 a.m.
Is this a user count that can be passed to _set_user_count, or a list of usernames?	QuLogic	March 13, 2022, 1:26 a.m.
Can this be passed to _set_user_count?	QuLogic	March 13, 2022, 1:22 a.m.
It doesn't look like you're setting the channel field? That is used by the irc_chat_join vfunc.	QuLogic	March 13, 2022, 1:22 a.m.
Seems like name was previously the description?	QuLogic	March 13, 2022, 1:22 a.m.
Don't you need the room field for the join_chat vfunc to work?	QuLogic	March 13, 2022, 1:22 a.m.
Same here as irc about the channel field.	QuLogic	March 13, 2022, 1:23 a.m.
Should be spaces?	QuLogic	March 13, 2022, 1:23 a.m.
field is going to leak since there is no key destroy function, but it seems like everywhere else uses a …	QuLogic	March 13, 2022, 1:23 a.m.
whose	QuLogic	March 13, 2022, 1:23 a.m.
This is gone, too.	QuLogic	March 13, 2022, 1:24 a.m.
Since purple_roomlist_room_get_components is transfer none, this modifies the hash table on the room object directly. Should there be a copy?	QuLogic	March 13, 2022, 1:25 a.m.
Weird indent.	QuLogic	March 13, 2022, 1:25 a.m.
Doing this after do_join_cb would make more sense, and maybe not need the comment?	QuLogic	March 13, 2022, 1:26 a.m.
These both seem to be unused?	QuLogic	March 13, 2022, 1:25 a.m.
Seems unrelated, but okay.	QuLogic	March 13, 2022, 5:05 a.m.
Do you need this model, since it's set at runtime? (Or it doesn't need to be set at runtime?)	QuLogic	March 13, 2022, 1:26 a.m.

finch/gntroomlist.c (Diff revision 1)
The issue has been resolved. Show all issues
```
To be done now, or ...?
```
libpurple/protocols/facebook/facebook.c (Diff revision 1)
The issue has been dropped. Show all issues
```
Is this a user count that can be passed to _set_user_count, or a list of usernames?
```
1. grim March 11, 2022, 8:15 a.m.
  this is a list of usernames
libpurple/protocols/gg/chat.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Can this be passed to _set_user_count?
```
1. grim March 11, 2022, 8:15 a.m.
  yes it can!

libpurple/protocols/irc/msgs.c (Diff revision 1)

The issue has been resolved. Show all issues

It doesn't look like you're setting the channel field? That is used by the irc_chat_join vfunc.

libpurple/protocols/jabber/chat.c (Diff revision 1)

The issue has been resolved. Show all issues

Seems like name was previously the description?

grim March 11, 2022, 9:22 a.m.

Nice catch, i just checked pidgin2 and for devel@conference.pidgin.im it has a name of devel and a description of Pidgin Development

libpurple/protocols/null/nullprpl.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Don't you need the room field for the join_chat vfunc to work?
```

libpurple/protocols/silc/ops.c (Diff revision 1)

The issue has been resolved. Show all issues

Same here as irc about the channel field.

libpurple/purpleroomlistroom.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Should be spaces?
```

libpurple/purpleroomlistroom.c (Diff revision 1)

The issue has been resolved. Show all issues

field is going to leak since there is no key destroy function, but it seems like everywhere else uses a static key, which makes this annoying.
Maybe add a comment for later if this is going to be rewritten differently?

grim March 11, 2022, 9:22 a.m.

we're only called with static strings, so this g_strdup isn't necessary.

libpurple/roomlist.h (Diff revision 1)
The issue has been resolved. Show all issues
```
whose
```
libpurple/roomlist.c (Diff revision 1)
The issue has been resolved. Show all issues
```
This is gone, too.
```

libpurple/roomlist.c (Diff revision 1)

The issue has been resolved. Show all issues

Since purple_roomlist_room_get_components is transfer none, this modifies the hash table on the room object directly. Should there be a copy?

pidgin/gtkroomlist.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Weird indent.
```

pidgin/gtkroomlist.c (Diff revision 1)

The issue has been resolved. Show all issues

Doing this after do_join_cb would make more sense, and maybe not need the comment?

pidgin/gtkroomlist.c (Diff revision 1)
The issue has been resolved. Show all issues
```
These both seem to be unused?
```

pidgin/pidgincolor.c (Diff revision 1)

The issue has been resolved. Show all issues

Seems unrelated, but okay.

grim March 13, 2022, 3:27 a.m.

do you want me ot pull this out to another rr?

QuLogic March 13, 2022, 4:44 a.m.
```
I dunno; is it actually unrelated?
```

grim March 13, 2022, 4:47 a.m.

I found it while testing, so technically no.. :-D Anyways I'll split it out quick..

pidgin/resources/Roomlist/roomlist.ui (Diff revision 1)

The issue has been resolved. Show all issues

Do you need this model, since it's set at runtime? (Or it doesn't need to be set at runtime?)

Change Summary:

fix issues, also finch room list is broken on default, so couldn't really test.

Commits:

	Summary	ID
	Rework the way roomlists work so we can more easily port them to GTK4	52df6fbbcdc98884db75a8d56e4a8a9cc1df5fca
	Rework the way roomlists work so we can more easily port them to GTK4	4af19bc115a1ff0f3e6b3d5f07ed300a25dfa3df

Diff:

Revision 2 (+1426 -2082)

Show changes

	ChangeLog.API
	finch/gntroomlist.c
	libpurple/meson.build
	libpurple/purpleroomlistroom.h
	libpurple/purpleroomlistroom.c
	libpurple/roomlist.h
	libpurple/roomlist.c
	libpurple/protocols/facebook/facebook.c
	10 more

Change Summary:

undo the pidgincolor drive by

Commits:

	Summary	ID
	Rework the way roomlists work so we can more easily port them to GTK4	4af19bc115a1ff0f3e6b3d5f07ed300a25dfa3df
	Rework the way roomlists work so we can more easily port them to GTK4	b0d164707959884400bddfc50396b7079f1ad35b

Diff:

Revision 3 (+1420 -2080)

Show changes

	ChangeLog.API
	finch/gntroomlist.c
	libpurple/meson.build
	libpurple/purpleroomlistroom.h
	libpurple/purpleroomlistroom.c
	libpurple/roomlist.h
	libpurple/roomlist.c
	libpurple/protocols/facebook/facebook.c
	9 more

Change Summary:

rebased

Commits:

	Summary	ID
	Rework the way roomlists work so we can more easily port them to GTK4	b0d164707959884400bddfc50396b7079f1ad35b
	Rework the way roomlists work so we can more easily port them to GTK4	85cd7833df73e33edfedff861455ea4e4941cce1

Diff:

Revision 4 (+1420 -2080)

Show changes

	ChangeLog.API
	finch/gntroomlist.c
	libpurple/meson.build
	libpurple/purpleroomlistroom.h
	libpurple/purpleroomlistroom.c
	libpurple/roomlist.h
	libpurple/roomlist.c
	libpurple/protocols/facebook/facebook.c
	9 more

Ship it!

```
Ship It!
```

Status:: Completed