I don't really get what the actual problem is. Is it that services are not supposed to open up a window when launched? For $EDITOR we used to have --wait (which was nice because it would be bound to specific files, not the whole app), but we had to remove that because supporting it with GtkApplication was difficult.

(In reply to comment #1)
> but we had to remove that because supporting it with GtkApplication was
> difficult.

Actually --wait is working great with gapplication, we had to remove --background.

Oops! Sorry, I should know better than to try to recall things instead of looking them up, memory fail.

The $EDITOR problem is that the first instance of the app to run will stay around as long as any windows at all are open.

So if you 'git commit', causing gedit to start (first instance) and then open a separate tab, and save and close the first tab, gedit will still be running until you close the second tab.

(In reply to comment #4)
> So if you 'git commit', causing gedit to start (first instance) and then open a
> separate tab, and save and close the first tab, gedit will still be running
> until you close the second tab.

No "gedit --wait" (which is what you should put in EDITOR) has the magic to track specific tab opened

If you specify --wait, it will dbus activate the gedit service, and then communicate with it to open the specified files and be signalled when you're done editing. As soon as you're done editing those specific files, the gedit --wait instance will quit properly, but if there are still other tabs open, the activated instance will keep on running (as what I would expect).

So I guess gedit does pretty much the right thing, it's just that it's inverted with respect to what you propose.

I just tried starting 'gedit --wait'.  At least on my system, there is no D-Bus activation here.  The instance started with '--wait' directly becomes the primary instance and if I open another document and close the original file, 'gedit --wait' continues to run.

yes If I recall when we ported gedit to GApplication this was one of the issues of not going with the server/client approach. --wait will work only if you close the window...

Looks like we all don't really remember how things are working :) Could we use G_APPLICATION_IS_LAUNCHER in case of --wait to prevent become the primary instance? Or doesn't that work like that?

This would work if you added --gapplication-service to the service file, I think.  There may be more work needed since I'm not sure exactly how this works internally in gedit.

Created attachment 265957 [details] [review]
dbus service: use --gapplication-service

We have to parse this argument for ourselves because we do our own
commandline argument handling.

There remains one problem here: gedit and its service file don't agree on the appid:

  service file:    org.gnome.gedit
  application id;  org.gnome.Gedit

I'm not sure which of these is more correct, so I left it unchanged for now, but one of them needs to be updated in order to close the loop.

Just for me to understand how this works:

--gapplication-service, when specified, gedit is _forced_ to run as a service. If it's already running as a service, then this will fail. If it succeeds, this will still run appklass->command_line on the service side (I think?) getting passed the args. We call g_application_activate in command_line, so it seems the behavior might still be wrong (will open empty doc, new window, etc).

Just to list the different scenario's, current situation and expected behavior:

1) launched by the desktop
2) run from the terminal
3) activated by dbus
   Currently all handled the same way. If there is no primary instance,
   this launch becomes primary and blocks. Otherwise args/stdin are passed
   to the service and handled there

3) run with --standalone
   Does not register as a service as all, always blocks

4) run with --wait
   Should (but currently doesn't) activate dbus service and open specified docs,
   blocks waiting for a signal from the service that the opened documents are
   closed.

So, your patch force 3) to be a service, but doesn't yet correctly _not_ open a window or new empty doc (or should it). Furthermore, gedit launched from the terminal should maybe always act as a client, dbus activating the service and possibly be replacing --wait? We can keep --standalone as is, I use it mostly for testing/debugging, but maybe we can rename it to --gapplication-local if that's now more conventional.

Anyone up for untangling this?

(In reply to comment #13)
> --gapplication-service, when specified, gedit is _forced_ to run as a service.
> If it's already running as a service, then this will fail.

It would not already be running as a service.  Other than by this option, there would be nothing to tell it to become a service.

> If it succeeds, this
> will still run appklass->command_line on the service side (I think?) getting
> passed the args. We call g_application_activate in command_line, so it seems
> the behavior might still be wrong (will open empty doc, new window, etc).

This is not true and is perhaps the source of most of the confusion.  In service mode, the commandline is not parsed.  In fact, it is rejected if it contains anything at all.  See below under '3' for what typically happens in service mode.

> Just to list the different scenario's, current situation and expected behavior:
> 
> 1) launched by the desktop

Right now this happens via the Exec= line in the desktop file, running 'gedit' with no args.  In that case, this patch does nothing and gedit works like usual (ie: becomes the primary instance directly, unless one is already running).

In the future, the 'launched by the desktop' case will work via D-Bus activation (and in fact, you should make that change to gedit after this patch).

> 2) run from the terminal

Same as above.  User runs 'gedit' and it becomes the primary instance directly, unless one is already running.

> 3) activated by dbus
>    Currently all handled the same way. If there is no primary instance,
>    this launch becomes primary and blocks. Otherwise args/stdin are passed
>    to the service and handled there

The D-Bus service file contains 'gedit --gapplication-service' so in this case we go into service mode.  Nothing gets activated automatically.  However: D-Bus has autostarted us for a reason -- there is probably an incoming Activate, Open or ActivateAction method that will cause us to do something useful very soon.  If not, we exit after 10 seconds (as is the behaviour for service mode).

> 3) run with --standalone
>    Does not register as a service as all, always blocks

Correct.  Could also be thought of as always becoming the primary instance but without taking the D-Bus name.  Is capable of receiving action invocations if directed at the unique bus name (as would be by gnome-shell's appmenu).

> 4) run with --wait
>    Should (but currently doesn't) activate dbus service and open specified
> docs,
>    blocks waiting for a signal from the service that the opened documents are
>    closed.

Yes.  --wait is a bit broken right now, but we could fix it easily by using launcher mode (assuming we fix up the service vs. appid mismatch mentioned above).

> So, your patch force 3) to be a service, but doesn't yet correctly _not_ open a
> window or new empty doc (or should it). Furthermore, gedit launched from the
> terminal should maybe always act as a client, dbus activating the service and
> possibly be replacing --wait? We can keep --standalone as is, I use it mostly
> for testing/debugging, but maybe we can rename it to --gapplication-local if
> that's now more conventional.

My patch does nothing to --standalone.

--gapplication-local is just a name I was tossing around.  This doesn't exist at all yet.

> Anyone up for untangling this?

I hope that helped.

(In reply to comment #13)
> Just for me to understand how this works:
> 
> --gapplication-service, when specified, gedit is _forced_ to run as a service.
> If it's already running as a service, then this will fail.

It just occurred to me that this comment could be referring to the fact that after we parse --gapplication-service in gedit's local_command_line, we pass it again to the chainup, which fails because (as mentioned), services reject all commandline arguments.

That's a bug in how gedit handles its commandline arguments in general and is the subject of bug 721947 (which has a gedit patch to improve the situation).

One more thing: GLib had a small bug with HANDLES_COMMAND_LINE services.  That was fixed *very* recently (minutes ago):  

  https://git.gnome.org/browse/glib/commit/?id=d3017967d8123e800fd593e22fda1c0d7f40071f

Make sure you have this patch if you're poking around right now or you're likely to be even more confused :)

Review of attachment 265957 [details] [review]:

ok, you are the master on this.

Comment on attachment 265957 [details] [review]
dbus service: use --gapplication-service

Attachment 265957 [details] pushed as 016a3fc - dbus service: use --gapplication-service

Created attachment 265989 [details] [review]
Change application ID to org.gnome.gedit

Attachment 265989 [details] pushed as 0aa89c2 - Change application ID to org.gnome.gedit