bug#63620: 30.0.50; [Feature Request] run hooks on sleep/wake

[Michael Albinus]

Andrew Cohen <acohen@ust.hk> writes:

> Dear Michael

Hi Andrew,

sorry for the delayed answer, I've been busy ...

> I've now put everything together and modified the gnus code to use the
> new sleep.el. But I remain unsure if I should remove gnus-dbus.el
> entirely (everything remains the same for the user: setting
> gnus-dbus-close-on-sleep to a non-nil value will enable the feature
> using the global minor mode rather than gnus-dbus).  And is sleep.el
> something worth adding?

It depends whether sleep.el will be added to vanilla Emacs, or as GNU
ELPA package. It's not my decision, but I lobby for the former.

In this case, it could be used in gnus-dbus.el.

Just some few comments on the code

> ;;; sleep.el --- run hooks on sleep and wake  -*- lexical-binding:t -*-

I would be more precise. You don't mean sleeping Emacs or another
application, but you mean sleeping the machine on OS level. Say it so.

> ;;; This global minor mode enables evaluating code when the device
> ;;; running Emacs enters or leaves the sleep state.  Two hooks are
> ;;; used, sleep-sleep-hook and sleep-wake-hook, run when the system
> ;;; detects that it is going to sleep or waking up.  Currently only a
> ;;; dbus interface to detect sleep state change is implemented.

Please quote Lisp objects like `sleep-sleep-hook'. D-Bus is spelled out
in commentaries as D-Bus.

> (defgroup sleep nil
>   "Run hooks on entering/leaving the sleep state."
>   :group 'hardware)
>
> (defcustom sleep-sleep-hook nil
>   "Hook to run on entering sleep."
>   :group 'sleep
>   :type 'hook)
>
> (defcustom sleep-wake-hook nil
>   "Hook to run on leaving sleep."
>   :group 'sleep
>   :type 'hook)

These are the user visible objects. Please be precise what is going to
sleep. The machine or device.

> Run sleep-sleep-hook and sleep-wake-hook as appropriate."

Please quote sleep-sleep-hook.

>   (unless sleep-registration-object
>     (setq sleep-registration-object
>           (dbus-register-signal :system
>                                 "org.freedesktop.login1"
>                                 "/org/freedesktop/login1"
>                                 "org.freedesktop.login1.Manager"
>                                 "PrepareForSleep"
>                                 #'sleep-handler))))

I would also protect against D-Bus errors. Like

(ignore-error dbus-error
  (unless sleep-registration-object
    (setq sleep-registration-object
          (dbus-register-signal
           :system "org.freedesktop.login1"
           "/org/freedesktop/login1" "org.freedesktop.login1.Manager"
           "PrepareForSleep" #'sleep-handler))))

>   (condition-case nil
>       (progn
>         (dbus-unregister-object
>          sleep-registration-object)
>         (setq sleep-registration-object nil))
>     (wrong-type-argument nil)))

Aka

(ignore-error (dbus-error wrong-type-argument)
  (dbus-unregister-object
   sleep-registration-object)
  (setq sleep-registration-object nil))

> When sleep-wake-mode is enabled, Emacs will execute the hooks

Please quote sleep-wake-mode.

> support dbus detection of sleep state change."

D-Bus is spelled out ...

Furthermore, it would be nice if you add

- documentation in lispref node "(elisp) System Interface". I'm not sure
  whether it is good for a new subnode, or whether it shall be
  documented in "(elisp) System Environment". This documentation should
  also give a practical example for functions in sleep-sleep-hook and
  sleep-wake-hook.

- adding the functionality to etc/NEWS

- if possible, adding tests in test/lisp/sleep-tests.el

Best regards, Michael.