XdpAppInfo cleanups #1619
XdpAppInfo cleanups #1619
Conversation
Purely cosmetics.
They're not public.
* Add space before '('
* gchar → char
* Remove stray newline
It's unused.
Makes it slightly easier to read.
We'll start shuffling things around, so that XdpAppInfo & subclasses operate with the mechanics proposed by GInitable.
| @@ -67,7 +67,6 @@ typedef struct _XdpAppInfoPrivate | |||
| GAppInfo *gappinfo; | |||
| gboolean supports_opath; | |||
| gboolean has_network; | |||
| gboolean requires_pid_mapping; | |||
There was a problem hiding this comment.
Maybe I screwed something up when refactoring XdpAppInfo? Might be worth double checking what this was used for...
And pass it during object construction (i.e. g_initable_new())
And pass it during object construction (i.e. g_initable_new())
And pass it during object construction (i.e. g_initable_new()).
And pass it during object construction (i.e. g_initable_new())
Generate flag type from the private enum, and make it a construct-only property.
And pass it during object construction (i.e. g_initable_new()). This was the last value passed to xdp_app_info_initialize() so remove this function as well.
Now that the constructor is a simple g_initable_new(), the xdp_app_info_host_new_full() constructor seems unnecessary. Next commits will introduce different flags for each path so just separate them now.
Right now it's somewhat useless, and just serves as a safeguard, but it will be useful for sharing the GAppInfo creation code a little more.
Instead of making each constructor create a GAppInfo in almost exactly equal ways, add an overridable vfunc, and implement the common path in XdpAppInfo. XdpAppInfoSnap needs a little bit of handholding, since the desktop file does not match the app id, so add a construct-only property to XdpAppInfoSnap and use it for a Snap-specific GAppInfo construction.
No functional changes (hopefully).
GeorgesStavracas
force-pushed
the
gbsneto/xdp-app-info-followups
branch
from
35f2e5a to
c8b31e4
Compare
| @@ -175,6 +180,11 @@ xdp_app_info_set_property (GObject *object, | |||
| priv->instance = g_value_dup_string (value); | |||
| break; | |||
|
|
|||
| case PROP_PIDFD: | |||
| g_assert (priv->pidfd == -1); | |||
| priv->pidfd = dup (g_value_get_int (value)); | |||
There was a problem hiding this comment.
typedef enum _XdpAppInfoFlags
tbh, I'm unsure why this even is a pattern in e.g. mutter but at least it makes it easy to grep for the declaration...
| @@ -23,6 +23,12 @@ | |||
|
|
|||
| #include "xdp-app-info.h" | |||
|
|
|||
| typedef enum | |||
There was a problem hiding this comment.
typedef enum _XdpAppInfoFlags
tbh, I'm unsure why this even is a pattern in e.g. mutter but at least it makes it easy to grep for the declaration...
|
I'm a bit on the fence about the approach. Wouldn't it be better to move the code we currently have in |
That was my original approach, but it got pretty spectacularly messy and I just settled for these construct-only properties. |
Mh, I couldn't resist and tried it as well. The result is pretty nice, but I need to untangle the changes into smaller commits: https://github.com/swick/xdg-desktop-portal/commits/wip/app-info-initable/ |
See commits. This is the first part of #1614