Created attachment 370654 [details] [review]
GSimpleAction: Explain nullable @parameter(_type)s

If something is nullable, it's always helpful to confirm what NULL means

Created attachment 370655 [details] [review]
GSimpleAction: Explain "(expected|correct) type"

Let's avoid users having to infer what these mean; it's easy to explain.

Created attachment 370656 [details] [review]
GSimpleAction: Slightly improve docs for new()s

Explain why we say "See new_stateful()" (although it's pretty obvious).
Drop a redundant copy of the argument description in the body text.
Add a # to the GVariant type name so that we can have a nice link.

Review of attachment 370654 [details] [review]:

Yup

Review of attachment 370655 [details] [review]:

::: gio/gsimpleaction.c
@@ +379,3 @@
+   * @parameter will always be of the expected type, i.e. the parameter type
+   * specified when the action was created. If an incorrect type is given when
+   * activating the action, this signal is not be emitted.

s/is not be emitted/is not emitted/.

Review of attachment 370656 [details] [review]:

::: gio/gsimpleaction.c
@@ +602,3 @@
  *
+ * The created action is stateless. See g_simple_action_new_stateful() if you
+ * want to create an action that has state.

s/if you want to/to/.

Thanks for the quick reviews. I've applied those grammar fixes.

I also updated the 1st patch to explain why I change references to "the activate function" to ::activate, namely that we as SimpleAction take that over, so the user never deals with it. I dunno how I forgot that bit of the message before.

Should that be changed in the property description too?

  g_object_class_install_property (object_class, PROP_PARAMETER_TYPE,
                                   g_param_spec_boxed ("parameter-type",
                                                       P_("Parameter Type"),
                                                       P_("The type of GVariant passed to activate()"),

But then: in GSimpleAction we are installing a property with the same name as a read-only one that already exists in GAction, so maybe that's an argument for keeping the name the same there? It hardly seems like the most important place to change this anyway.

Created attachment 370663 [details] [review]
GSimpleAction: Clarify/fix @parameter(_type) docs

later if wanted.

--

GSimpleAction: Clarify/fix @parameter(_type) docs

If something is nullable, it's always helpful to identify what NULL
means. Also, this is not the parameter for the .activate() vfunc, as we
take that over: rather, it is the parameter for the ::activate signal.

Created attachment 370664 [details] [review]
GSimpleAction: Explain "(expected|correct) type"

Let's avoid users having to infer what these mean; it's easy to explain.

Created attachment 370665 [details] [review]
GSimpleAction: Slightly improve docs for new()s

Explain why we say "See new_stateful()" (although it's pretty obvious).
Drop a redundant copy of the argument description in the body text.
Add a # to the GVariant type name so that we can have a nice link.

Review of attachment 370663 [details] [review]:

Yup

Review of attachment 370664 [details] [review]:

Yup

Review of attachment 370665 [details] [review]:

Yup.

Thanks. Should I pick to glib-2-56? (this and bug 795096)

(In reply to Daniel Boles from comment #14)
> Thanks. Should I pick to glib-2-56? (this and bug 795096)

No, generally documentation tweaks are not important enough to backport unless specifically requested by distributions (or unless they correct errors). Backporting too much makes it harder for distributions to see the important patches to cherry-pick.

Thanks, pushed to master