Summary

Create PurpleIRCv3Capabilities for managing capabilities for each connection

Review Request #2100 — Created Nov. 26, 2022 and submitted Nov. 29, 2022, 6:09 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

Reviewers

Groups

pidgin

People

Description

This implements everything in capabilities version 302 except for handling the
NEW and DEL commands from the server. I'm not sure how we can test that yet,
so we're ignoring it for now.

After the server has told us about all of the capabilities it supports, we
explicitly request cap-notify which we implictly request when we send
CAP LS 302. This is mostly to test our code and might be removed in the future.

Testing Done

Connected to my local ergo and used ngrep to verify that everything was being sent correctly.

Commits

Summary	ID
Create PurpleIRCv3Capabilities for managing capabilities for each connection This implements everything in capabilities version 302 except for handling the NEW and DEL commands from the server. I'm not sure how we can test that yet, so we're ignoring it for now. After the server has told us about all of the capabilities it supports, we explicitly request cap-notify which we implictly request when we send CAP LS 302. This is mostly to test our code and might be removed in the future.	e6a4dbd3828d4cd7adb223c1fe5a90dde83684d5

Summary

Create PurpleIRCv3Capabilities for managing capabilities for each connection

This implements everything in capabilities version 302 except for handling the NEW and DEL commands from the server. I'm not sure how we can test that yet, so we're ignoring it for now. After the server has told us about all of the capabilities it supports, we explicitly request cap-notify which we implictly request when we send CAP LS 302. This is mostly to test our code and might be removed in the future.

e6a4dbd3828d4cd7adb223c1fe5a90dde83684d5

Issues

Description	From	Last Updated
Is there a reason for connecting to the signal from the same object? Should that be the class handler?	QuLogic	Nov. 29, 2022, 5:49 a.m.
nak	QuLogic	Nov. 29, 2022, 5:49 a.m.
nacked	QuLogic	Nov. 29, 2022, 5:49 a.m.
This is never freed?	QuLogic	Nov. 29, 2022, 5:50 a.m.
splitting	QuLogic	Nov. 29, 2022, 5:50 a.m.
You can avoid the g_strdup by setting parts[i] = NULL so it won't be freed.	QuLogic	Nov. 29, 2022, 5:50 a.m.
Not sure why this can't be inlined?	QuLogic	Nov. 29, 2022, 5:50 a.m.
Not unused.	QuLogic	Nov. 29, 2022, 5:51 a.m.
Ditto	QuLogic	Nov. 29, 2022, 5:51 a.m.
This is just real_found = g_hash_table_lookup_extended(...	QuLogic	Nov. 29, 2022, 5:51 a.m.

libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)

Show all issues

Is there a reason for connecting to the signal from the same object? Should that be the class handler?

grim Nov. 29, 2022, 5:31 a.m.

hadn't considered that one, that's probably a good move.

libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)
Show all issues
```
nak
```
libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)
Show all issues
```
nacked
```
libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)
Show all issues
```
This is never freed?
```
libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)
Show all issues
```
splitting
```

libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)

Show all issues

You can avoid the g_strdup by setting parts[i] = NULL so it won't be freed.

grim Nov. 29, 2022, 5:35 a.m.

We can't do this, because we have to call g_strfreev which stops at the first NULL.

libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)

Show all issues

Not sure why this can't be inlined?

grim Nov. 29, 2022, 5:31 a.m.

Do you mean mark the function as inline ?

QuLogic Nov. 29, 2022, 5:35 a.m.

No, I mean this is a single-line function that's called from only one place, plus it has extra parameters when called that end up unused.

grim Nov. 29, 2022, 5:45 a.m.

Gotta it, sorry that wasn't clicking for some reason.

libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)
Show all issues
```
Not unused.
```
libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)
Show all issues
```
Ditto
```
libpurple/protocols/ircv3/purpleircv3capabilities.c (Diff revision 1)
Show all issues
```
This is just real_found = g_hash_table_lookup_extended(...
```

Change Summary:

rebased and addressed issues

Commits:

	Summary	ID
	Create PurpleIRCv3Capabilities for managing capabilities for each connection This implements everything in capabilities version 302 except for handling the NEW and DEL commands from the server. I'm not sure how we can test that yet, so we're ignoring it for now. After the server has told us about all of the capabilities it supports, we explicitly request cap-notify which we implictly request when we send CAP LS 302. This is mostly to test our code and might be removed in the future.	6a974b0d5dd88149dd2299d6e7dd3d4debce83c4
	Create PurpleIRCv3Capabilities for managing capabilities for each connection This implements everything in capabilities version 302 except for handling the NEW and DEL commands from the server. I'm not sure how we can test that yet, so we're ignoring it for now. After the server has told us about all of the capabilities it supports, we explicitly request cap-notify which we implictly request when we send CAP LS 302. This is mostly to test our code and might be removed in the future.	e6a4dbd3828d4cd7adb223c1fe5a90dde83684d5

Diff:

Revision 2 (+1130 -244)

Show changes

	libpurple/protocols/ircv3/meson.build
	libpurple/protocols/ircv3/purpleircv3capabilities.h
	libpurple/protocols/ircv3/purpleircv3capabilities.c
	libpurple/protocols/ircv3/purpleircv3connection.h
	libpurple/protocols/ircv3/purpleircv3connection.c
	libpurple/protocols/ircv3/purpleircv3core.c
	libpurple/protocols/ircv3/purpleircv3messagehandlers.h
	libpurple/protocols/ircv3/purpleircv3messagehandlers.c
	1 more

Ship it!

```
Ship It!
```

Review Board 6.0.2

Create PurpleIRCv3Capabilities for managing capabilities for each connection

Information

Reviewers

Change Summary:

Commits:

Diff:

Status: Closed (submitted)