This is the reason that we introudced the "you must register local commandline arguments to change the behaviour" restriction.  Unfortunately, I forgot about the automatic handling of --help.

That's this check:

  /* If the application has not registered local options and it has
   * G_APPLICATION_HANDLES_COMMAND_LINE then we have to assume that
   * their primary instance commandline handler may want to deal with
   * the arguments.  We must therefore ignore them.
   */
  if (application->priv->main_options == NULL && (application->priv->flags & G_APPLICATION_HANDLES_COMMAND_LINE))
    g_option_context_set_ignore_unknown_options (context, TRUE);


There is a tug here between not stealing --help and the handling of options from other option groups (for example, from Gtk).  Those are the options that would be recognised during the parse, even with no arguments explicitly added by the app itself.

Given that the Gtk options were never handled previously, and that the current situation is breaking existing applications (namely meld) then maybe we should skip parsing entirely if no local options have been registered.

I only worry that this may regress someone else now that the stable release has been made (ie: perhaps someone is now using the Gtk options, from a desktop file or script).

Can you describe in a bit more detail how broken --help handling is hurting git mergetool?  Maybe we can find a nice way to fix this very specific case.

Created attachment 287717 [details] [review]
GApplication: ignore --help if not handling args

If the user didn't register any arguments for parsing, also ignore
--help.  This fixes a regression in meld.


I'd feel fairly comfortable including this change, even on the stable branch.
Please let me know if it works for you.

btw: please consider moving to the new API :)

(of course, we need to fix this bug anyway so that the existing versions continue to work)

(In reply to comment #1)
> This is the reason that we introudced the "you must register local commandline
> arguments to change the behaviour" restriction.  Unfortunately, I forgot about
> the automatic handling of --help.
> 
> That's this check:
> 
>   /* If the application has not registered local options and it has
>    * G_APPLICATION_HANDLES_COMMAND_LINE then we have to assume that
>    * their primary instance commandline handler may want to deal with
>    * the arguments.  We must therefore ignore them.
>    */
>   if (application->priv->main_options == NULL && (application->priv->flags &
> G_APPLICATION_HANDLES_COMMAND_LINE))
>     g_option_context_set_ignore_unknown_options (context, TRUE);

Yep, that's exactly the case I'm seeing. FWIW, I would have overridden local-command-line and handled local options there, which I assume would have avoided this case, but... you can't from Python.

> There is a tug here between not stealing --help and the handling of options
> from other option groups (for example, from Gtk).  Those are the options that
> would be recognised during the parse, even with no arguments explicitly added
> by the app itself.
> 
> Given that the Gtk options were never handled previously, and that the current
> situation is breaking existing applications (namely meld) then maybe we should
> skip parsing entirely if no local options have been registered.
> 
> I only worry that this may regress someone else now that the stable release has
> been made (ie: perhaps someone is now using the Gtk options, from a desktop
> file or script).

I'm not bothered by the GApplication-enforced handling for pretty much every option *other* than --help. Technically, I guess you could be breaking someone who happened to support a custom --gapplication-service argument, but... well... that's probably okay.

> Can you describe in a bit more detail how broken --help handling is hurting git
> mergetool?  Maybe we can find a nice way to fix this very specific case.

The git mergetool meld helper runs `meld --help` and greps it the result looking for --output to decide whether to do a three-way-with-output merge. Don't blame me... I didn't write it!

This is pretty trivially fixable on Git's side (e.g., check --version instead), but that mergetool script is installed with Git by default, so that would be a lot of breakage until people update.

(In reply to comment #2)
> Created an attachment (id=287717) [details] [review]
> GApplication: ignore --help if not handling args
> 
> If the user didn't register any arguments for parsing, also ignore
> --help.  This fixes a regression in meld.
> 
> 
> I'd feel fairly comfortable including this change, even on the stable branch.
> Please let me know if it works for you.

Yes, that patch works great for me. Thanks.

(In reply to comment #3)
> btw: please consider moving to the new API :)
> 
> (of course, we need to fix this bug anyway so that the existing versions
> continue to work)

I'm definitely going to look at moving to the new API when I get some time. It looks nice! It just wasn't available when I started the GTK3 port.

Attachment 287717 [details] pushed as f4af8d1 - GApplication: ignore --help if not handling args