Summary

Replace PidginAccountsDisabledMenu with PidginInactiveAccountsMenu which is a GMenuModel subclass.

Review Request #1366 — Created March 22, 2022 and submitted March 25, 2022, 2:57 a.m.

Information

Owner

grim

Repository

pidgin/pidgin

Branch

default

Bugs

Depends On

Reviewers

Groups

pidgin

People

Description

This gives us a bit more control as we are always in control of the attributes,
so we don't need to modify the menu in place. This also made it trivial to add
the protocol icons which are very helpful when you have the same user name on
multiple protocols.

Testing Done

Enabled and disabled two accounts a lot while under valgrind.

Commits

Summary	ID
Reimplement the PidginAccountsDisabledMenu to be a GMenuModel subclass. This gives us a bit more control as we are always in control of the attributes, so we don't need to modify the menu in place. This also made it trivial to add the protocol icons which are very helpful when you have the same user name on multiple protocols.	f54207e24fef2f6921106ca24ab72f419b02d28b

Issues

Description	From	Last Updated
Should it be disabled? Or maybe inactive?	QuLogic	March 25, 2022, 2:38 a.m.
Not a Menu any more; probably easier to keep the type out of the docs here.	QuLogic	March 25, 2022, 2:39 a.m.
This leaked the existing menu->accounts list.	QuLogic	March 25, 2022, 2:40 a.m.
The key doesn't seem to need to be strdup'd, unless that's required of the API?	QuLogic	March 25, 2022, 2:40 a.m.
Should use the attribute name constants.	QuLogic	March 25, 2022, 2:40 a.m.
I'm no sure if this should be g_variant_ref_sink instead.	QuLogic	March 25, 2022, 2:40 a.m.
Add space.	QuLogic	March 25, 2022, 2:40 a.m.
Update type in comment.	QuLogic	March 25, 2022, 2:52 a.m.

libpurple/purpleaccountmanager.h (Diff revision 1)

The issue has been resolved. Show all issues

Should it be disabled? Or maybe inactive?

grim March 25, 2022, 1:18 a.m.

Well we have "enabled" already, so I went with "disabled" but typoed. "inactive" I dunno?

pidgin/pidginaccountsdisabledmenu.h (Diff revision 1)
The issue has been resolved. Show all issues
```
Not a Menu any more; probably easier to keep the type out of the docs here.
```
pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
The issue has been resolved. Show all issues
```
This leaked the existing menu->accounts list.
```
pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
The issue has been resolved. Show all issues
```
The key doesn't seem to need to be strdup'd, unless that's required of the API?
```
pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Should use the attribute name constants.
```
pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
The issue has been resolved. Show all issues
```
I'm no sure if this should be g_variant_ref_sink instead.
```
pidgin/pidginaccountsdisabledmenu.c (Diff revision 1)
The issue has been resolved. Show all issues
```
Add space.
```

Change Summary:

Address issues and rename to PidginInactiveAccountsMenu

Summary:

Reimplement the PidginAccountsDisabledMenu to be a GMenuModel subclass.

Replace PidginAccountsDisabledMenu with PidginInactiveAccountsMenu which is a GMenuModel subclass.

Commits:

	Summary	ID
	Reimplement the PidginAccountsDisabledMenu to be a GMenuModel subclass. This gives us a bit more control as we are always in control of the attributes, so we don't need to modify the menu in place. This also made it trivial to add the protocol icons which are very helpful when you have the same user name on multiple protocols.	2e74220169504805271bc92eb25f734a432e03d5
	Reimplement the PidginAccountsDisabledMenu to be a GMenuModel subclass. This gives us a bit more control as we are always in control of the attributes, so we don't need to modify the menu in place. This also made it trivial to add the protocol icons which are very helpful when you have the same user name on multiple protocols.	d0c53135a30fe6087a0f99367327a64a50f56349

Diff:

Revision 2 (+438 -184)

Show changes

	libpurple/purpleaccountmanager.h
	libpurple/purpleaccountmanager.c
	pidgin/meson.build
	pidgin/pidginapplication.c
	pidgin/pidginaccountsdisabledmenu.h
	pidgin/pidginaccountsdisabledmenu.c
	pidgin/resources/gtk/menus.ui
	po/POTFILES.in

pidgin/pidginapplication.c (Diff revisions 1 - 2)
The issue has been resolved. Show all issues
```
Update type in comment.
```

Change Summary:

rebased

Commits:

	Summary	ID
	Reimplement the PidginAccountsDisabledMenu to be a GMenuModel subclass. This gives us a bit more control as we are always in control of the attributes, so we don't need to modify the menu in place. This also made it trivial to add the protocol icons which are very helpful when you have the same user name on multiple protocols.	d0c53135a30fe6087a0f99367327a64a50f56349
	Reimplement the PidginAccountsDisabledMenu to be a GMenuModel subclass. This gives us a bit more control as we are always in control of the attributes, so we don't need to modify the menu in place. This also made it trivial to add the protocol icons which are very helpful when you have the same user name on multiple protocols.	f54207e24fef2f6921106ca24ab72f419b02d28b

Diff:

Revision 3 (+440 -186)

Show changes

	libpurple/purpleaccountmanager.h
	libpurple/purpleaccountmanager.c
	pidgin/meson.build
	pidgin/pidginapplication.c
	pidgin/pidginaccountsdisabledmenu.h
	pidgin/pidginaccountsdisabledmenu.c
	pidgin/resources/gtk/menus.ui
	po/POTFILES.in

Ship it!

```
Ship It!
```

Status:: Completed