Summary

Create a manager for conversations.

Review Request #677 — Created May 27, 2021 and submitted June 14, 2021, 4:08 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

Commit

391d7cd30c38

Reviewers

Groups

pidgin

People

Description

I Skipped unit tests because need a connection and full protocol implementation to do it properly.

Testing Done

Compiled and ran with an IRC account.

Issues

Description	From	Last Updated
Do you really need two vars? Seems like g_list_delete_link would work.	QuLogic	June 14, 2021, 2:57 a.m.
Should be l->data, I believe.	QuLogic	June 14, 2021, 2:57 a.m.
Actually, now it seems like purple_conversations_get_all is called multiple times; can you not free here and start from list again …	QuLogic	June 14, 2021, 2:57 a.m.
Extra blank line.	QuLogic	June 14, 2021, 2:58 a.m.
Same comments as the previous function.	QuLogic	June 14, 2021, 2:58 a.m.
gint?	QuLogic	June 14, 2021, 2:56 a.m.
#PurpleConversation\s or something like that?	QuLogic	June 14, 2021, 2:57 a.m.
an	QuLogic	June 14, 2021, 2:57 a.m.
@name	QuLogic	June 14, 2021, 2:57 a.m.
@name	QuLogic	June 14, 2021, 2:57 a.m.
All callers have already checked these things, so they're redundant, except for purple_conversation_manager_find_chat_by_id, which passes name=NULL and is broken by …	QuLogic	June 14, 2021, 3:10 a.m.
!func(conversation, userdata)	QuLogic	June 14, 2021, 2:56 a.m.
The hash table should be freed here?	QuLogic	June 14, 2021, 2:56 a.m.
Don't need the if.	QuLogic	June 14, 2021, 2:58 a.m.
Funny wrap indent.	QuLogic	June 14, 2021, 2:56 a.m.
Funny wrap indent.	QuLogic	June 14, 2021, 2:56 a.m.
Why is this guint instead of gint?	QuLogic	June 14, 2021, 2:56 a.m.
Is gtkblist->menutrayicon not just created in the below deleted code, and thus unused now?	QuLogic	June 14, 2021, 3 a.m.
Same questions as in finch.	QuLogic	June 14, 2021, 2:55 a.m.

Decided to do some more work here, so don't check this out yet...

Description:

~		This will eventually replace the purple_conversations API. I Skipped unit
	~	I Skipped unit tests because need a connection and full protocol implementation to do it properly.
-		tests because need a connection and full protocol implementation to do it
-		properly.

Commit:

3b164a92379c

2df720f9bd1e

Diff:

Revision 2 (+560 -404)

Show changes

	doc/reference/libpurple/libpurple-docs.xml
	finch/gntconv.c
	libpurple/conversations.h
	libpurple/conversations.c
	libpurple/core.c
	libpurple/meson.build
	libpurple/purpleconversation.c
	libpurple/purpleconversationmanager.h
	7 more

finch/gntconv.c (Diff revision 2)

The issue has been resolved. Show all issues

Do you really need two vars? Seems like g_list_delete_link would work.

finch/gntconv.c (Diff revision 2)
The issue has been resolved. Show all issues
```
Should be l->data, I believe.
```

finch/gntconv.c (Diff revision 2)

The issue has been resolved. Show all issues

Actually, now it seems like purple_conversations_get_all is called multiple times; can you not free here and start from list again later? I assume the conversation count doesn't change.

finch/gntconv.c (Diff revision 2)
The issue has been resolved. Show all issues
```
Extra blank line.
```
finch/gntconv.c (Diff revision 2)
The issue has been resolved. Show all issues
```
Same comments as the previous function.
```
libpurple/protocols/null/nullprpl.c (Diff revision 2)
The issue has been resolved. Show all issues
```
gint?
```
libpurple/purpleconversationmanager.h (Diff revision 2)
The issue has been resolved. Show all issues
```
#PurpleConversation\s or something like that?
```
libpurple/purpleconversationmanager.h (Diff revision 2)
The issue has been resolved. Show all issues
```
an
```
libpurple/purpleconversationmanager.h (Diff revision 2)
The issue has been resolved. Show all issues
```
@name
```
libpurple/purpleconversationmanager.h (Diff revision 2)
The issue has been resolved. Show all issues
```
@name
```

libpurple/purpleconversationmanager.c (Diff revision 2)

The issue has been resolved. Show all issues

All callers have already checked these things, so they're redundant, except for purple_conversation_manager_find_chat_by_id, which passes name=NULL and is broken by this name != NULL check.

QuLogic June 14, 2021, 3 a.m.

You can drop the others too? It's not public.

grim June 14, 2021, 3:03 a.m.

I always get hesitent with this as it could eventually get made public, but then again since it is named _internal that's probably fine.

libpurple/purpleconversationmanager.c (Diff revision 2)
The issue has been resolved. Show all issues
```
!func(conversation, userdata)
```
libpurple/purpleconversationmanager.c (Diff revision 2)
The issue has been resolved. Show all issues
```
The hash table should be freed here?
```
libpurple/purpleconversationmanager.c (Diff revision 2)
The issue has been resolved. Show all issues
```
Don't need the if.
```
libpurple/purpleconversationmanager.c (Diff revision 2)
The issue has been resolved. Show all issues
```
Funny wrap indent.
```
libpurple/purpleconversationmanager.c (Diff revision 2)
The issue has been resolved. Show all issues
```
Funny wrap indent.
```

libpurple/purpleconversationmanager.c (Diff revision 2)

The issue has been resolved. Show all issues

Why is this guint instead of gint?

grim June 13, 2021, 8:41 p.m.

i was going with guint because id's aren't typically negative, but I'll change it back.

pidgin/gtkblist.c (Diff revision 2)

The issue has been resolved. Show all issues

Is gtkblist->menutrayicon not just created in the below deleted code, and thus unused now?

pidgin/gtkconv.c (Diff revision 2)
The issue has been resolved. Show all issues
```
Same questions as in finch.
```

Change Summary:

this should be everything found

Commit:

2df720f9bd1e

9a4ec97578d8

Diff:

Revision 3 (+583 -437)

Show changes

	doc/reference/libpurple/libpurple-docs.xml
	finch/gntconv.c
	libpurple/conversations.h
	libpurple/conversations.c
	libpurple/core.c
	libpurple/meson.build
	libpurple/purpleconversation.c
	libpurple/purpleconversationmanager.h
	8 more

Commit:

9a4ec97578d8

391d7cd30c38

Diff:

Revision 4 (+580 -437)

Show changes

	doc/reference/libpurple/libpurple-docs.xml
	finch/gntconv.c
	libpurple/conversations.h
	libpurple/conversations.c
	libpurple/core.c
	libpurple/meson.build
	libpurple/purpleconversation.c
	libpurple/purpleconversationmanager.h
	8 more

Ship it!

```
Ship It!
```

Status:: Completed