Created attachment 164929 [details] [review]
Allow a return value in peas_method_apply_valist

I know this works for returning a gboolean and an object.

Created attachment 164931 [details] [review]
Allow a return value in peas_method_apply_valist - splinter please load this one

Attaching another to hopefully get splinter to load if for review.

Also without the quoted that was meant for another patch.

Comment on attachment 164931 [details] [review]
Allow a return value in peas_method_apply_valist - splinter please load this one

>+  /* Notes: According to GCC 4.4,
>+   *  - int8, uint8, int16, uint16, short and ushort are promoted to int when passed through '...'
>+   *  - float is promoted to double when passed through '...'
>+   */
>+   /* The above shouldnt matter so I think we can unpromote the below */

I think you can remove these comments as what gets passed through '...' is just a pointer, not the actual type, unlike for (in) arguments.
Hence you should also use the "real" types in the switch, not the promoted ones.

>+      *(gboolean *) in_retval = out_retval->v_boolean;

Could you use *((gboolean *) int_retval) = out_retval->v_boolean instead? (notice the parens) I think it's more readable at the end, operators priority is not always obvious.

>+
>+static gboolean
> read_next_argument (GArgument  *cur_arg,

What's the reason for making this become a gboolean?

> 
>+  retval_info = g_callable_info_get_return_type (func_info);
>+  retval_is_void = g_type_info_get_tag (retval_info) == GI_TYPE_TAG_VOID;
>+
>+  if (!retval_is_void)
>+    {
>+      in_retval = va_arg (args, gpointer);
>+    }

I think the return value argument should be put at the end of the peas_method_apply() arg list, as it is usually done for (out) args.

> I think the return value argument should be put at the end of the
> peas_method_apply() arg list, as it is usually done for (out) args.

The above statement means that for functions with a single (out) param, the apply() call should be the same if the param is the return value or if the param is an (out) value of the function.

Created attachment 164936 [details] [review]
Allow a return value in peas_method_apply_valist v3

(In reply to comment #3)
> (From update of attachment 164931 [details] [review])
> >+  /* Notes: According to GCC 4.4,
> >+   *  - int8, uint8, int16, uint16, short and ushort are promoted to int when passed through '...'
> >+   *  - float is promoted to double when passed through '...'
> >+   */
> >+   /* The above shouldnt matter so I think we can unpromote the below */
> 
> I think you can remove these comments as what gets passed through '...' is just
> a pointer, not the actual type, unlike for (in) arguments.
> Hence you should also use the "real" types in the switch, not the promoted
> ones.

Done

> >+      *(gboolean *) in_retval = out_retval->v_boolean;
> 
> Could you use *((gboolean *) int_retval) = out_retval->v_boolean instead?
> (notice the parens) I think it's more readable at the end, operators priority
> is not always obvious.

Done

> >+
> >+static gboolean
> > read_next_argument (GArgument  *cur_arg,
> 
> What's the reason for making this become a gboolean?

Extra bit of safety in-case a new tag is added.

> > 
> >+  retval_info = g_callable_info_get_return_type (func_info);
> >+  retval_is_void = g_type_info_get_tag (retval_info) == GI_TYPE_TAG_VOID;
> >+
> >+  if (!retval_is_void)
> >+    {
> >+      in_retval = va_arg (args, gpointer);
> >+    }
> 
> I think the return value argument should be put at the end of the
> peas_method_apply() arg list, as it is usually done for (out) args.

Done

Created attachment 164986 [details] [review]
Handle return values in peas_method_apply_valist()

Created attachment 164987 [details] [review]
[PeasUIConfigurable] Make the dialog the return value.

It was previously an out parameter, but we recently gained the ability
to use return values of extension interface methods.

Attachment 164986 [details] pushed as 72b5eab - Handle return values in peas_method_apply_valist()
Attachment 164987 [details] pushed as 3892bd5 - [PeasUIConfigurable] Make the dialog the return value.