- Download Diff

Summary:

Convert plugin actions to GMenu and GAction

Review Request #1408 — Created April 30, 2022 and submitted May 27, 2022, 7:53 p.m.

Information
Owner:	grim
Repository:	pidgin/pidgin
Branch:	default
Bugs:
Depends On:	~~1412~~
Reviewers
Groups:	pidgin
People:

Description

Also ported the idle maker plugin to the new api.

Testing Done

Made sure all of the actions for the idle maker plugin worked.

Commits

Summary
Convert plugin actions to GMenu and GAction

Screenshots

Issues

Description	From	Last Updated
I wouldn't intersperse the g_menu_new up here, when the action group is done later.	QuLogic	May 27, 2022, 9:12 a.m.
Would calling the action directly not work?	QuLogic	May 27, 2022, 9:12 a.m.
I would say "action group"	QuLogic	May 27, 2022, 9:12 a.m.
And "menu model"	QuLogic	May 27, 2022, 9:12 a.m.
Flip order, just to match args.	QuLogic	May 27, 2022, 9:01 a.m.
Do we have to worry about removing these on plugin unload?	QuLogic	May 27, 2022, 9:15 a.m.
Given the if above, is this guaranteed non-NULL?	QuLogic	May 27, 2022, 9:13 a.m.
Do you have to check if either of these are NULL?	QuLogic	May 27, 2022, 9:20 a.m.
caller is interested	QuLogic	May 27, 2022, 9:13 a.m.
Isn't this guaranteed by pidgin_plugins_menu_add_item?	QuLogic	May 27, 2022, 9:13 a.m.

I'm not super happy with the plugin interface for this, but it does work.

grim May 1, 2022, 6:39 a.m.

Much happier with the interface now that the ui is controlling the namespace/prefix and the plugin doesn't need to care about it. We should probably do some normalization on this, but it works for now.

Change Summary:

have the plugins menu manage the namespace/prefix for the menu. This elimantes the need for the plugin to care about it and lets the ui just handle it.

Commits:

	Summary
-	Convert plugin actions to GMenu and GAction
+	Convert plugin actions to GMenu and GAction

Diff:

Revision 2 (+746 -558)

Show changes

	libpurple/purpleplugininfo.h
	libpurple/purpleplugininfo.c
	libpurple/plugins/idle.c
	pidgin/pidginapplication.c
	pidgin/pidgincontactlist.c
	pidgin/pidginpluginsmenu.h
	pidgin/pidginpluginsmenu.c
	pidgin/glade/pidgin3.xml.in
	3 more

Depends On:

+	~~1412 - Chain up to the parent class's implementation of the window_added default signal handler~~

Change Summary:

migrate to pidgin_application_add_action_group

Commits:

	Summary
-	Convert plugin actions to GMenu and GAction
+	Convert plugin actions to GMenu and GAction

Diff:

Revision 3 (+742 -558)

Show changes

	libpurple/purpleplugininfo.h
	libpurple/purpleplugininfo.c
	libpurple/plugins/idle.c
	pidgin/pidginapplication.c
	pidgin/pidgincontactlist.c
	pidgin/pidginpluginsmenu.h
	pidgin/pidginpluginsmenu.c
	pidgin/glade/pidgin3.xml.in
	3 more

Change Summary:

rebased

Commits:

	Summary
-	Convert plugin actions to GMenu and GAction
+	Convert plugin actions to GMenu and GAction

Diff:

Revision 4 (+742 -558)

Show changes

	libpurple/purpleplugininfo.h
	libpurple/purpleplugininfo.c
	libpurple/plugins/idle.c
	pidgin/pidginapplication.c
	pidgin/pidgincontactlist.c
	pidgin/pidginpluginsmenu.h
	pidgin/pidginpluginsmenu.c
	pidgin/glade/pidgin3.xml.in
	3 more

Change Summary:

rebased

Commits:

	Summary
-	Convert plugin actions to GMenu and GAction
+	Convert plugin actions to GMenu and GAction

Diff:

Revision 5 (+742 -558)

Show changes

	libpurple/purpleplugininfo.h
	libpurple/purpleplugininfo.c
	libpurple/plugins/idle.c
	pidgin/pidginapplication.c
	pidgin/pidgincontactlist.c
	pidgin/pidginpluginsmenu.h
	pidgin/pidginpluginsmenu.c
	pidgin/glade/pidgin3.xml.in
	3 more

libpurple/plugins/idle.c (Diff revision 5)

Show all issues

I wouldn't intersperse the g_menu_new up here, when the action group is done later.

libpurple/plugins/idle.c (Diff revision 5)
Show all issues
```
Would calling the action directly not work?
```
libpurple/purpleplugininfo.c (Diff revision 5)
Show all issues
```
I would say "action group"
```
libpurple/purpleplugininfo.c (Diff revision 5)
Show all issues
```
And "menu model"
```

pidgin/pidginpluginsmenu.c (Diff revision 5)

Show all issues

Flip order, just to match args.

grim May 27, 2022, 8:56 a.m.

Let's not, because if we do, then it goes "GPlugin, Pidgin, GPlugin" for the type names.

pidgin/pidginpluginsmenu.c (Diff revision 5)

Show all issues

Do we have to worry about removing these on plugin unload?

pidgin/pidginpluginsmenu.c (Diff revision 5)

Show all issues

Given the if above, is this guaranteed non-NULL?

pidgin/pidginpluginsmenu.c (Diff revision 5)
Show all issues
```
caller is interested
```

pidgin/pidginpluginsmenu.c (Diff revision 5)

Show all issues

Isn't this guaranteed by pidgin_plugins_menu_add_item?

grim May 27, 2022, 8:56 a.m.

Yes, but someone else could find the menu model and attempt to add stuff to it.

QuLogic May 27, 2022, 9:02 a.m.

But this is the menu model implementation, and its internals are not exposed, are they?

grim May 27, 2022, 9:08 a.m.

ah ha! I forgot that the model is read-only, so yes this should be fine.

Change Summary:

rebased and addressed issues.

Commits:

	Summary
-	Convert plugin actions to GMenu and GAction
+	Convert plugin actions to GMenu and GAction

Diff:

Revision 6 (+762 -560)

Show changes

	libpurple/purpleplugininfo.h
	libpurple/purpleplugininfo.c
	libpurple/plugins/idle.c
	pidgin/pidginapplication.c
	pidgin/pidgincontactlist.c
	pidgin/pidginpluginsmenu.h
	pidgin/pidginpluginsmenu.c
	pidgin/glade/pidgin3.xml.in
	3 more

pidgin/pidginpluginsmenu.c (Diff revisions 5 - 6)
Show all issues
```
Do you have to check if either of these are NULL?
```

Change Summary:

addressed issue.

Commits:

	Summary
-	Convert plugin actions to GMenu and GAction
+	Convert plugin actions to GMenu and GAction

Diff:

Revision 7 (+770 -560)

Show changes

	libpurple/purpleplugininfo.h
	libpurple/purpleplugininfo.c
	libpurple/plugins/idle.c
	pidgin/pidginapplication.c
	pidgin/pidgincontactlist.c
	pidgin/pidginpluginsmenu.h
	pidgin/pidginpluginsmenu.c
	pidgin/glade/pidgin3.xml.in
	3 more

Ship it!

```
Ship It!
```

You have a pending review.

Review Board 4.0.7

Convert plugin actions to GMenu and GAction

Change Summary:

Commits:

Diff:

Depends On:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Change Summary:

Commits:

Diff:

Status: Closed (submitted)