Summary

Modernize PurpleProtocol

Review Request #552 — Created March 3, 2021 and submitted March 8, 2021, 11:23 p.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

PIDGIN-17492

Depends On

Commit

12b391973a1d

Reviewers

Groups

pidgin

People

Description


Migrate PurpleProtocol to the G_DECLARE_DERIVABLE_TYPE and G_DEFINE_TYPE macros.
Rename purple_protocol_class_* to purple_protocol_*.
Move the class properties to instance properties and add accessors as necessary.

Testing Done

Ran locally and connected with bonjour, facebook, irc, and xmpp. Verified that the other prpls are loading and visible in the account manager.

Issues

Description	From	Last Updated
The initial g_return_if_fail checks in the old protocol_class methods might be causing issues. I had to remove the PURPLE_IS_ACCOUNT check …	grim	March 5, 2021, 10:31 p.m.
Frees	QuLogic	March 8, 2021, 7:30 p.m.
Should use this in the boxed type registration for consistency.	QuLogic	March 8, 2021, 7:30 p.m.
These docs need to move into PurpleProtocolClass.	QuLogic	March 8, 2021, 7:31 p.m.
Should this be purple_protocol_get_status_types?	QuLogic	March 8, 2021, 7:35 p.m.
And purple_protocol_get_list_icon?	QuLogic	March 8, 2021, 7:35 p.m.
Doesn't seem to be read from anything?	QuLogic	March 8, 2021, 7:31 p.m.
Seems this need finishing.	QuLogic	March 8, 2021, 7:31 p.m.
Tabs	QuLogic	March 8, 2021, 9:15 p.m.
Tabs	QuLogic	March 8, 2021, 9:15 p.m.
Tabs	QuLogic	March 8, 2021, 9:15 p.m.
Extra parens around the g_object_new call.	QuLogic	March 8, 2021, 7:34 p.m.
Extra parens around g_object_new.	QuLogic	March 8, 2021, 7:34 p.m.
This might leak as the condition is composite.	QuLogic	March 8, 2021, 7:34 p.m.
This might leak as the condition is composite.	QuLogic	March 8, 2021, 7:34 p.m.
As this is a bit flag check in a composite, I would add an extra set of parentheses here.	QuLogic	March 8, 2021, 7:34 p.m.

Show all issues

The initial g_return_if_fail checks in the old protocol_class methods might be causing issues. I had to remove the PURPLE_IS_ACCOUNT check from list_icon because the Add/Edit Account dialog was calling it with account set to NULL. However, trying to test all these methods is very tedious but I'll look more into it shortly.

grim March 5, 2021, 10:31 p.m.

Followed all the paths and get_list_icon was the only one that needed it as all the other functions only have a single caller.

libpurple/buddyicon.h (Diff revision 1)
Show all issues
```
Frees
```

libpurple/buddyicon.c (Diff revision 1)

Show all issues

Should use this in the boxed type registration for consistency.

libpurple/protocol.h (Diff revision 1)

Show all issues

These docs need to move into PurpleProtocolClass.

libpurple/protocol.h (Diff revision 1)

Show all issues

Should this be purple_protocol_get_status_types?

grim March 4, 2021, 11:57 p.m.

probably, but I didn't want to add more noise right now.

grim March 5, 2021, 10:32 p.m.

Turns out the noise was nothing as we already renamed the function.. sorry for being dense ;)

libpurple/protocol.h (Diff revision 1)
Show all issues
```
And purple_protocol_get_list_icon?
```
libpurple/protocol.c (Diff revision 1)
Show all issues
```
Doesn't seem to be read from anything?
```

libpurple/protocols/gg/gg.c (Diff revision 1)

Show all issues

Seems this need finishing.

grim March 4, 2021, 11:57 p.m.

kind of.. it's weird, but I put a NULL check into ggp_servconn_setup to make it work.

libpurple/protocols/null/nullprpl.c (Diff revision 1)

Show all issues

Tabs

grim March 4, 2021, 11:57 p.m.

The entire file is 2 space tab indent and my editor just continued it. I can reformat it in pieces now or we can do it as a separate pr.

QuLogic March 8, 2021, 7:34 p.m.

These are complete functions, so I don't think it's a problem to add them with the right style. That also means the next PR fixing things will be smaller. Plus the code that was moved here was already using tabs.

libpurple/protocols/null/nullprpl.c (Diff revision 1)
Show all issues
```
Tabs
```
1. QuLogic March 8, 2021, 7:34 p.m.
  This function is already using tabs.
libpurple/protocols/null/nullprpl.c (Diff revision 1)
Show all issues
```
Tabs
```
libpurple/tests/test_protocol_attention.c (Diff revision 1)
Show all issues
```
Extra parens around the g_object_new call.
```
libpurple/tests/test_protocol_xfer.c (Diff revision 1)
Show all issues
```
Extra parens around g_object_new.
```

pidgin/gtkaccount.c (Diff revision 1)

Show all issues

This might leak as the condition is composite.

pidgin/gtkaccount.c (Diff revision 1)

Show all issues

This might leak as the condition is composite.

pidgin/gtkblist.c (Diff revision 1)

Show all issues

As this is a bit flag check in a composite, I would add an extra set of parentheses here.

Commit:

-	7928147e9aeb
+	87751a4c824c

Diff:

Revision 2 (+1035 -613)

Show changes

	finch/gntaccount.c
	libpurple/account.c
	libpurple/buddyicon.h
	libpurple/buddyicon.c
	libpurple/connection.c
	libpurple/log.c
	libpurple/protocol.h
	libpurple/protocol.c
	18 more

Change Summary:

rebased
renamed status_types to get_status_types
renamed list_icon to get_list_icon

Commit:

-	87751a4c824c
+	6e42c876b5b9

Diff:

Revision 3 (+1038 -613)

Show changes

	finch/gntaccount.c
	libpurple/account.c
	libpurple/buddyicon.h
	libpurple/buddyicon.c
	libpurple/connection.c
	libpurple/log.c
	libpurple/protocol.h
	libpurple/protocol.c
	18 more

Commit:

-	6e42c876b5b9
+	12b391973a1d

Diff:

Revision 4 (+1038 -613)

Show changes

	finch/gntaccount.c
	libpurple/account.c
	libpurple/buddyicon.h
	libpurple/buddyicon.c
	libpurple/connection.c
	libpurple/log.c
	libpurple/protocol.h
	libpurple/protocol.c
	18 more

Ship it!

```
Ship It!
```

Review Board 6.0.2

Modernize PurpleProtocol

Information

Reviewers

Commit:

Diff:

Change Summary:

Commit:

Diff:

Commit:

Diff:

Status: Closed (submitted)