Ihor, what I do not like in your patch is that an external process is
unconditionally executed during load time. Earlier there was a (failed)
attempt to limit it to X11.
- Unsure if Windows builds of Emacs may connect to X servers.
- MacOS does not use x11idle, it calls ioreg and perl instead.
In above cases, there might be a point to execute x11idle if a user is
running remote Emacs session on a Windows or a MacOS machine (e.g.
through ssh) from local X server. Unsure if somebody has ever tried it.
The reason to try x11idle should be a test if current frame is
associated with X.
A side note. I am aware that the following comment existed before your
commit.
(and (eq 0 (call-process-shell-command
(format "command -v %s" org-clock-x11idle-program-name)))
;; Check that x11idle can retrieve the idle time
;; FIXME: Why "..-shell-command" rather than just `call-process'?
(eq 0 (call-process-shell-command org-clock-x11idle-program-name))))
`call-process' can not be used here because "command" is a shell
built-in, not a real executable. I have another question. Why
`locate-file' is not used instead?
(locate-file command exec-path exec-suffixes 1))
https://emacs.stackexchange.com/questions/332/how-can-i-find-the-path-to-an-executable-with-emacs-lisp
On 30/10/2022 08:33, Ihor Radchenko wrote:
Max Nikulin writes:
In server.el I found
(frame-parameter frame 'display)
..
I do not understand.
echo "$DISPLAY"
:0
emacs -Q --daemon
emacsclient -nw
window-system
nil
(frame-parameter nil 'display)
":0"
(call-process "xterm")
So formally a tty frame has access to X11 server.
Multiple X connection means non-deterministic idle time. Each individual
X server will have its own idle time. The current approach with running
`org-x11-idle-seconds' in current frame is the most reasonable in such
scenario, IMHO.
I would say that minimal value should be used. However such case is
rarely used and will be almost untested.
From my point of view `org-clock-x11idle-program-name' approach is a
bit fragile. I do not like "%s" substitutions while formatting shell
commands. On the other hand it allows a complex command instead of
executable, e.g.
dbus-send --print-reply --dest=org.gnome.Mutter.IdleMonitor
/org/gnome/Mutter/IdleMonitor/Core org.gnome.Mutter.IdleMonitor.GetIdletime
for Wayland, see
https://unix.stackexchange.com/questions/396911/how-can-i-tell-if-a-user-is-idle-in-wayland/492328
Perhaps, using Emacs D-BUS API would be even better.
x11idle as `org-clock-x11idle-program-name' default is questionable as
well. Debian, Arch, Gentoo have xprintidle packages and this tool have
some additional code.
My summary:
- To be really flexible (e.g. to support Wayland)
`org-user-idle-seconds' should have an extension point allowing to
specify elisp function.
- x11idle availability should be checked when X connection is detected,
not at startup time and perhaps `locate-file' is better than "command"
shell built-in.
- (frame-parameter nil 'display) might be more accurate in addition to
`window-system' check.
- xprintidle should be either the default or an alternative during the test.
- I can not figure out how to detect --display argument of "emacs
--daemon", but perhaps there is no point of time logging in the case of
a headless process.
- There are tricky cases like remote Emacs or multihead Emacs. Custom
idle function will allow to postpone their discussion till actual request.