search

elementary / calendar

Desktop calendar app designed for elementary OS
https://elementary.io
GNU General Public License v3.0
129 stars 39 forks source link

Embedded daemon in one unified binary #702

Closed marbetschar closed 3 years ago

marbetschar commented 3 years ago

This PR fixes the notification icon issue from #677 by embedding the daemonized background process into the same application binary (in the screenshot: "Other" is before this change, the correct notification is after this change):

Screenshot from 2021-09-08 08-52-28

This way, we gain the following benefits:

1. The notification icon is correct

The notification icon is read from the Icon key stored in /usr/share/applications/{application_id}.desktop. Since the background process shares the same application_id as the main application, the notification server uses the correct icon to display the alert.

2. The background process starts automatically

Autostart files are read from /etc/xdg/autostart/{application_id}.desktop. We can easily place a second desktop file with the EXEC line io.elementary.calendar --background in this directory.

3. When clicking on the notification, the corresponding app opens

When clicking on the notification, the notification server executes the Exec key of the /usr/share/applications/{application_id}.desktop file. Here it is crucial we embedded the background process within the same binary - because this way, the regular binary starts and the user is presented with the app itself. Just like we want it.

4. System Settings > Notifications work as expected

As it turned out when working on this in Mail, if we don't embed the background process within the same binary we either end up with two application entries there - or even worse, with one which does not have any effect at all.

mcclurgm commented 3 years ago

Never mind my comment about on_events_updated, that's out of scope for this.

Seems to work as expected. As I said in my last comment there are a few minor style things, but I think this pretty much works! I'll approve it once we get those sorted.

marbetschar commented 3 years ago

@mcclurgm thanks for your review! I addressed the proposed changes and I think we should be good to go here now.

mcclurgm commented 3 years ago

Alright, seems good to me.

quienn commented 2 years ago

Latest Calendar version comes with the autostart entry for the daemon (at /etc/xdg/autostart/io.elementary.calendar-daemon.desktop) though it never really starts the daemon. 😕 Was this intended?

marbetschar commented 2 years ago

@ghsttwn no, this was not the intention :)

The daemon should be autostarted as soon as you end your session and start a new one (read "logout and login again"). Doesn't this work on your end? If not, can you please open a new issue with a link to this PR, so we can investigate there?