Summary

Convert all of the buddylist context menus to GMenu

Review Request #1481 — Created May 27, 2022 and submitted May 31, 2022, 6:26 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

Reviewers

Groups

pidgin

People

Description

Convert all of the buddylist context menus to GMenu

Testing Done

Every item every way I think...

Commits

Summary	ID
Convert all of the buddylist context menus to GMenu	0dc88168188365f6a14301da98c4719548419c62

Issues

Description	From	Last Updated
Delete?	QuLogic	May 31, 2022, 4:47 a.m.
Can this be changed for make data the buddy list instead of looking at a global?	QuLogic	May 31, 2022, 4:48 a.m.
Ditto.	QuLogic	May 31, 2022, 4:49 a.m.
Can this use the blist from data instead of the gktblist global?	QuLogic	May 31, 2022, 4:50 a.m.
Isn't that blist, and node?	QuLogic	May 31, 2022, 4:51 a.m.
This probably shouldn't be in between the two custom icon actions.	QuLogic	May 31, 2022, 4:52 a.m.
Why so far away from the audio one?	QuLogic	May 31, 2022, 4:52 a.m.
Maybe add vars for account and name, since they're used 3 times here.	QuLogic	May 31, 2022, 4:53 a.m.
Maybe cast to GtkApplication and GActionMap? I think they're never used as these types.	QuLogic	May 31, 2022, 4:57 a.m.
Should you not check for PURPLE_PROTOCOL_IMPLEMENTS(protocol, MEDIA, get_caps)?	QuLogic	May 31, 2022, 5:01 a.m.
Can use enabled variable here, if it fits better?	QuLogic	May 31, 2022, 4:57 a.m.
Again, maybe pre-cast?	QuLogic	May 31, 2022, 5:03 a.m.
Check protocol != NULL, or else drop the check for xfer above? Also, remove outdated comment.	QuLogic	May 31, 2022, 6:18 a.m.
Didn't this always return the same menu? Do you need to search for custom-icon every time?	QuLogic	May 31, 2022, 6:18 a.m.

Change Summary:

rebased

Commits:

	Summary	ID
	Convert all of the buddylist context menus to GMenu	5e5e0c1d808c8a6583b5418da08023f597b6883c
	Convert all of the buddylist context menus to GMenu	a59ee7f6a427bb21032c6e5840d77f08ed209c90

Diff:

Revision 2 (+1548 -1214)

Show changes

	ChangeLog.API
	pidgin/gtkblist.h
	pidgin/gtkblist.c
	pidgin/resources/gtk/menus.ui

Change Summary:

rebased

Commits:

	Summary	ID
	Convert all of the buddylist context menus to GMenu	a59ee7f6a427bb21032c6e5840d77f08ed209c90
	Convert all of the buddylist context menus to GMenu	4385b5d8f54fb5d9aaadfe9983b5d8f9f00b1dbb

Diff:

Revision 3 (+1548 -1214)

Show changes

	ChangeLog.API
	pidgin/gtkblist.h
	pidgin/gtkblist.c
	pidgin/resources/gtk/menus.ui

pidgin/gtkblist.c (Diff revision 3)
Show all issues
```
Delete?
```

pidgin/gtkblist.c (Diff revision 3)

Show all issues

Can this be changed for make data the buddy list instead of looking at a global?

pidgin/gtkblist.c (Diff revision 3)
Show all issues
```
Ditto.
```

pidgin/gtkblist.c (Diff revision 3)

Show all issues

Can this use the blist from data instead of the gktblist global?

pidgin/gtkblist.c (Diff revision 3)

Show all issues

Isn't that blist, and node?

grim May 31, 2022, 4:42 a.m.
```
not sure where you're seeing node..?
```

QuLogic May 31, 2022, 4:51 a.m.

Ah, I must have expanded too far, or maybe it was changed between rev 1 and 3 when I originally commented.

pidgin/gtkblist.c (Diff revision 3)

Show all issues

This probably shouldn't be in between the two custom icon actions.

grim May 31, 2022, 4:42 a.m.

I was trying to alphabetize them but they don't all have the same namespace...

pidgin/gtkblist.c (Diff revision 3)
Show all issues
```
Why so far away from the audio one?
```
1. grim May 31, 2022, 4:42 a.m.
  alphabetical

pidgin/gtkblist.c (Diff revision 3)

Show all issues

Maybe add vars for account and name, since they're used 3 times here.

pidgin/gtkblist.c (Diff revision 3)

Show all issues

Maybe cast to GtkApplication and GActionMap? I think they're never used as these types.

pidgin/gtkblist.c (Diff revision 3)

Show all issues

Should you not check for PURPLE_PROTOCOL_IMPLEMENTS(protocol, MEDIA, get_caps)?

pidgin/gtkblist.c (Diff revision 3)

Show all issues

Can use enabled variable here, if it fits better?

pidgin/gtkblist.c (Diff revision 3)
Show all issues
```
Again, maybe pre-cast?
```

Change Summary:

address issues

Commits:

	Summary	ID
	Convert all of the buddylist context menus to GMenu	4385b5d8f54fb5d9aaadfe9983b5d8f9f00b1dbb
	Convert all of the buddylist context menus to GMenu	52940bc1f5db6b96aa0da433c0022a7468aecb0a

Diff:

Revision 4 (+1522 -1214)

Show changes

	ChangeLog.API
	pidgin/gtkblist.h
	pidgin/gtkblist.c
	pidgin/resources/gtk/menus.ui

pidgin/gtkblist.c (Diff revision 4)

Show all issues

Check protocol != NULL, or else drop the check for xfer above?

Also, remove outdated comment.

pidgin/gtkblist.c (Diff revision 4)

Show all issues

Didn't this always return the same menu? Do you need to search for custom-icon every time?

Change Summary:

address issues.

Commits:

	Summary	ID
	Convert all of the buddylist context menus to GMenu	52940bc1f5db6b96aa0da433c0022a7468aecb0a
	Convert all of the buddylist context menus to GMenu	0dc88168188365f6a14301da98c4719548419c62

Diff:

Revision 5 (+1524 -1214)

Show changes

	ChangeLog.API
	pidgin/gtkblist.h
	pidgin/gtkblist.c
	pidgin/resources/gtk/menus.ui

Ship it!

```
Ship It!
```

Review Board 6.0.2

Convert all of the buddylist context menus to GMenu

Information

Reviewers

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Status: Closed (submitted)