Summary

Modernizing PurpleChatConversation.

Review Request #583 — Created March 26, 2021 and submitted April 6, 2021, 3:28 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

Commit

f06ea8fa57fc

Reviewers

Groups

pidgin

People

Description

I wasn't able to do everything as PurpleConversation still isn't declared with
G_DECLARE_DERIVABLE_TYPE.

Testing Done

Compiled ran locally with both xmpp mucs and irc channels.

Issues

Description	From	Last Updated
PurpleChatUser	QuLogic	March 29, 2021, 8:39 a.m.
@user or @message	QuLogic	March 29, 2021, 8:39 a.m.
Can drop this comment now, I think.	QuLogic	March 29, 2021, 8:39 a.m.
its	QuLogic	March 29, 2021, 8:39 a.m.
Maybe this should be PROP_CHAT_ID?	QuLogic	March 29, 2021, 8:39 a.m.
This seems new; is it really needed right now?	QuLogic	April 6, 2021, 2:37 a.m.
Why not set priv->updated directly like in purple_chat_conversation_ignore?	QuLogic	March 29, 2021, 8:39 a.m.
You deleted the setting of the new values?	QuLogic	March 29, 2021, 8:40 a.m.
I don't understand why users and flags needed an extra iteration variable (ul and fl) while extra_msgs didn't.	QuLogic	March 29, 2021, 8:40 a.m.
Can remove the extra space.	QuLogic	March 29, 2021, 8:41 a.m.
gchar?	QuLogic	March 29, 2021, 8:41 a.m.
Missed that this should be 'referred' earlier.	QuLogic	April 6, 2021, 2:35 a.m.

libpurple/purplechatconversation.h (Diff revision 1)
The issue has been resolved. Show all issues
```
PurpleChatUser
```
libpurple/purplechatconversation.h (Diff revision 1)
The issue has been resolved. Show all issues
```
@user or @message
```
libpurple/purplechatconversation.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Can drop this comment now, I think.
```
libpurple/purplechatconversation.c (Diff revision 1)
The issue has been resolved. Show all issues
```
its
```
libpurple/purplechatconversation.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Maybe this should be PROP_CHAT_ID?
```

libpurple/purplechatconversation.c (Diff revision 1)

The issue has been resolved. Show all issues

This seems new; is it really needed right now?

grim March 29, 2021, 6:14 a.m.

The old code checked if purple_account_get_connection() was NULL to do the same check.

QuLogic March 29, 2021, 8:37 a.m.

But those are not the same thing, as the account would have a connection while connecting. Which I think is subtly different enough that it should not go into a refactoring change like this.

libpurple/purplechatconversation.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Why not set priv->updated directly like in purple_chat_conversation_ignore?
```
libpurple/purplechatconversation.c (Diff revision 1)
The issue has been resolved. Show all issues
```
You deleted the setting of the new values?
```

libpurple/purplechatconversation.c (Diff revision 1)

The issue has been resolved. Show all issues

I don't understand why users and flags needed an extra iteration variable (ul and fl) while extra_msgs didn't.

libpurple/purplechatconversation.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Can remove the extra space.
```
libpurple/purplechatconversation.c (Diff revision 1)
The issue has been resolved. Show all issues
```
gchar?
```

Change Summary:

Address review findings.

Commit:

0b896343642f

1557c651770a

Diff:

Revision 2 (+633 -651)

Show changes

	doc/reference/libpurple/libpurple-docs.xml
	libpurple/conversations.h
	libpurple/meson.build
	libpurple/conversationtypes.h
	libpurple/conversationtypes.c
	libpurple/purplechatuser.h
	libpurple/purpleprotocolim.h
	po/POTFILES.in

libpurple/purplechatconversation.h (Diff revision 2)
The issue has been resolved. Show all issues
```
Missed that this should be 'referred' earlier.
```

Change Summary:

rebased and address issues.