Change default value of org-clock-x11-idle-program-name (was: org-x11idle-exists-p with emacs --daemon)

[Ihor Radchenko]

Max Nikulin  writes:

> 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.

AFAIU, x11idle was a very old hard-coded default. Later, this variable
had been introduced without changing the default.

What we can do is check if xprintidle executable is available and use
it. Otherwise, fall back to x11idle to retain backwards compatibility on
systems that do no have xprintidle installed.

WDYT?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: org-x11idle-exists-p with emacs --daemon

[Ihor Radchenko]

Max Nikulin  writes:

> 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.

We can test `sytem-type' to be in '(gnu gnu/linux gnu/kfreebsd).

> 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.

This is a trade-off. If we want to consider individual frames running on
remote X server, `org-user-idle-seconds' will need to check the command
availability on every call. That is - we will try to run shell command
every 60 seconds (`org-resolve-clocks-if-idle''s default timer). I'd say
that it is not a good idea considering how rare such situation is.

> 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

Not idea. Maybe even `executable-find'?

> 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.

Can we reliably distinguish between X and Wayland this way?

> 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.

Could you prepare a patch?

> - 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.

Again, patches welcome.
It will be a marginal improvement.

> - (frame-parameter nil 'display) might be more accurate in addition to 
> `window-system' check.

Do you mean (or ..)?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: org-x11idle-exists-p with emacs --daemon

[Max Nikulin]

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.

Re: org-x11idle-exists-p with emacs --daemon

[Ihor Radchenko]

Max Nikulin  writes:

> On 28/10/2022 11:28, Ihor Radchenko wrote:
>> 
>> * lisp/org-clock.el (org-x11idle-exists-p): Do not check if load-time
>> `window-system' is `x'.  Instead, rely on the check in
>> `org-user-idle-seconds'.
>
> I would say that even there it is not strictly correct to test 
> `window-system', perhaps `x-display-list' is a bit better since 
> particular frame may be a terminal one running in xterm, but user has x 
> frames.

I am not sure here.

Let us first establish the purpose of all this fiddling with idle
program.

`org-user-idle-seconds' is aiming to find the actual idle time of an Org
user. Not just in Emacs, but also in other places. That's why the
fallback `org-emacs-idle-seconds' is not the most accurate, and we are
trying to use other, more accurate, means to determine idle time.

`org-user-idle-seconds' uses system command to determine idle time, when
possible. On Mac, such command is always available. On X11, Org defaults
to https://git.sr.ht/~bzg/worg/tree/master/item/code/scripts/x11idle.c
script that will fail to work from non-X frame, AFAIU.

So, I do think the `window-system' is the right way to invoke x11idle at
least. Terminal frames may not have access to X11 scope and thus will
not be able to determine the real idle time. Not to mention that
terminal Emacs may not be running on X and even successfully getting
idle time will yield erroneous results.

> In server.el I found
>
>  (frame-parameter frame 'display)
>
> that might be even better and should work with daemon having a single 
> frame in xterm. I can not figure out if --display command line option 
> affects some variable of Emacs daemon.

I do not understand. I just tried to check the value of `window-system'
in terminal Emacs frame and in X11 Emacs frame running using the same
Emacs daemon. The value in X11 frame is 'x and nil in terminal. So,
`window-system' already accounts for current frame.

> Another tricky case is multiple x connection to different displays. It 
> seems GTK3 have some limitations (pgtk build) and such rarely used 
> configuration will become completely obsolete.

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.

>> diff --git a/lisp/org-clock.el b/lisp/org-clock.el
>> index e98a34f0d..ca026c44f 100644
>> --- a/lisp/org-clock.el
>> +++ b/lisp/org-clock.el
>> @@ -1201,8 +1201,7 @@ (defun org-mac-idle-seconds ()
>>  
>>  (defvar org-x11idle-exists-p
>>;; Check that x11idle exists
>> -  (and (eq window-system 'x)
>> -   (eq 0 (call-process-shell-command
>> +  (and (eq 0 (call-process-shell-command
>>(format "command -v %s" org-clock-x11idle-program-name)))
>
> I joined to this discussion presuming (by some confusion) that such 
> variable should be lazily initialized on first call of 
> `org-user-idle-seconds'. I considered `window-system' test as a way to 
> distinguish GUI and text Emacs sessions. Actually such parameter is 
> dynamic. Remotely running Emacs may be disconnected from one X server 
> and connected to another session.
>
> Actually `window-system' check has another role: skip command 
> availability test on systems other than Linux.
>
> What is the result of "command -v" on Windows? Should it be executed 
> there at all (unconditionally at load time)?

It will fail and the value of `org-x11idle-exists-p' will be nil. I do
not see any problem here.

> P.S. There are a number of X11 servers for Windows. I am unsure if Emacs 
> builds for Windows and X11 frames are supported. I am lost how to 
> properly test such cases in the code.

We do not care. We check if idle program works by calling it. If it
fails (due to command being not available or not supporting the X11
server), we fall back to safe approach using Emacs idle time.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: org-x11idle-exists-p with emacs --daemon

[Max Nikulin]

On 28/10/2022 11:28, Ihor Radchenko wrote:


* lisp/org-clock.el (org-x11idle-exists-p): Do not check if load-time
`window-system' is `x'.  Instead, rely on the check in
`org-user-idle-seconds'.


I would say that even there it is not strictly correct to test 
`window-system', perhaps `x-display-list' is a bit better since 
particular frame may be a terminal one running in xterm, but user has x 
frames. In server.el I found


(frame-parameter frame 'display)

that might be even better and should work with daemon having a single 
frame in xterm. I can not figure out if --display command line option 
affects some variable of Emacs daemon.


Another tricky case is multiple x connection to different displays. It 
seems GTK3 have some limitations (pgtk build) and such rarely used 
configuration will become completely obsolete.



diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index e98a34f0d..ca026c44f 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -1201,8 +1201,7 @@ (defun org-mac-idle-seconds ()
 
 (defvar org-x11idle-exists-p

   ;; Check that x11idle exists
-  (and (eq window-system 'x)
-   (eq 0 (call-process-shell-command
+  (and (eq 0 (call-process-shell-command
   (format "command -v %s" org-clock-x11idle-program-name)))


I joined to this discussion presuming (by some confusion) that such 
variable should be lazily initialized on first call of 
`org-user-idle-seconds'. I considered `window-system' test as a way to 
distinguish GUI and text Emacs sessions. Actually such parameter is 
dynamic. Remotely running Emacs may be disconnected from one X server 
and connected to another session.


Actually `window-system' check has another role: skip command 
availability test on systems other than Linux.


What is the result of "command -v" on Windows? Should it be executed 
there at all (unconditionally at load time)?


P.S. There are a number of X11 servers for Windows. I am unsure if Emacs 
builds for Windows and X11 frames are supported. I am lost how to 
properly test such cases in the code.

Re: org-x11idle-exists-p with emacs --daemon

[Ihor Radchenko]

Julien Cubizolles  writes:

> Ihor Radchenko  writes:
>
>> Does the attached patch work for you?
>
> Yes it does, thanks.

Thanks for testing!
Applied onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=a51bb2c448bab7665667471aa227e3e25dbbdced

> Also, org-user-idle-seconds uses (eq window-system 'x) which according
> to its docstring should be replaced by (display-graphic-p).

I do not think so.
org-user-idle-seconds is not about graphics, but about the window
system being used. Now, we have special handling for Mac and X11 and
fall back to universally working, though not accurate,
`org-emacs-idle-seconds'.

> I've tested
> the change and it's working, with the added advantage that it works
> both in a X11 and in a wayland session (provided your
> org-clock-x11idle-program-name works in both, mine does).

Is there a way to detect wayland build? Does window-system have any
special value on the gtk build? If yes, we can introduce
wayland-specific variable.

> There is another use of window-sytem in the org-mode source, namely in
> org-get-x-clipboard in org-compat.el, but I don't know if it's safe to
> make the switch there.

AFAIR, clipboard in Wayland is not the same as in X and might have bugs
https://orgmode.org/list/1902025.jDVfpnRRgo@pluto

I am not sure if `gui-get-selection' is going to work on Wayland. The
docstring says:

 Return the value of an X Windows selection.

Wayland is not X Windows...

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: org-x11idle-exists-p with emacs --daemon

[Julien Cubizolles]

Ihor Radchenko  writes:

> Does the attached patch work for you?

Yes it does, thanks.

Also, org-user-idle-seconds uses (eq window-system 'x) which according
to its docstring should be replaced by (display-graphic-p). I've tested
the change and it's working, with the added advantage that it works
both in a X11 and in a wayland session (provided your
org-clock-x11idle-program-name works in both, mine does).

There is another use of window-sytem in the org-mode source, namely in
org-get-x-clipboard in org-compat.el, but I don't know if it's safe to
make the switch there.
-- 
Julien Cubizolles

Re: org-x11idle-exists-p with emacs --daemon

[Ihor Radchenko]

Max Nikulin  writes:

>>> emacsclient --eval '(server-select-display (getenv "DISPLAY"))'
>> 
>> You are talking about a different problem.
>> Please start a new thread.
>
> Nope
>
> emacs -Q -L ~/src/org-mode/lisp/ --daemon
> emacsclient --eval window-system
> nil
>
> emacsclient --eval '(server-select-display (getenv "DISPLAY"))'
> emacsclient --eval window-system
> x
>
> I suspect that the check of `window-system' was added to detect Emacs 
> instances running as pure terminal applications having no access to X11 
> and the case of daemon was simply overlooked.

The reported problem is related to the fact that org-x11idle-exists-p
value is calculated at load time.

emacsclient --eval '(server-select-display (getenv "DISPLAY"))' may or
may not help depending on when (require 'org) is being executed.

The problem you are talking about is when no Emacs frames are present.
It is a problem that require different solution.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: org-x11idle-exists-p with emacs --daemon

[Max Nikulin]

On 28/10/2022 11:29, Ihor Radchenko wrote:

Max Nikulin  writes:


I am not familiar with `org-x11idle-exists-p', but a problem with access
of X clipboard from :immediate-finish capture templates may be quite
similar. The following command creates a hidden X11 frame if no visible
one exists:

emacsclient --eval '(server-select-display (getenv "DISPLAY"))'


You are talking about a different problem.
Please start a new thread.


Nope

emacs -Q -L ~/src/org-mode/lisp/ --daemon
emacsclient --eval window-system
nil

emacsclient --eval '(server-select-display (getenv "DISPLAY"))'
emacsclient --eval window-system
x

I suspect that the check of `window-system' was added to detect Emacs 
instances running as pure terminal applications having no access to X11 
and the case of daemon was simply overlooked.

Re: org-x11idle-exists-p with emacs --daemon

[Ihor Radchenko]

Julien Cubizolles  writes:

> All the org-*-idle-seconds func test at some point for
> org-x11idle-exists-p which is defvar-ed at load time. 
>
> For me, org-x11idle-exists-p is always nil at startup, but is set to
> true if I eval it later on. I guess it's because I'm starting Emacs in
> daemon mode, (as a systemd user service actually), and
> org-x11idle-exists-p relies on (eq window-system 'x) which is somehow
> not set in the terminal when the daemon.
>
> For the time being, it manually set org-x11idle-exists-p to true but
> shouldn't all this tests on (eq-window-system) be run each time a new
> frame is created ?

Does the attached patch work for you?

>From 95ec1073ab27bbfe522b25c9852f1e5e85e38e32 Mon Sep 17 00:00:00 2001
Message-Id: <95ec1073ab27bbfe522b25c9852f1e5e85e38e32.1666931293.git.yanta...@posteo.net>
From: Ihor Radchenko 
Date: Fri, 28 Oct 2022 12:26:09 +0800
Subject: [PATCH] org-x11idle-exists-p: Do not demand X window system at load
 time

* lisp/org-clock.el (org-x11idle-exists-p): Do not check if load-time
`window-system' is `x'.  Instead, rely on the check in
`org-user-idle-seconds'.

Emacs may start as a daemon and hence `window-system' may not yet be
`x' during startup.

Reported-by: Julien Cubizolles 
Link: https://orgmode.org/list/871qqs6gqs@free.fr
---
 lisp/org-clock.el | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index e98a34f0d..ca026c44f 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -1201,8 +1201,7 @@ (defun org-mac-idle-seconds ()
 
 (defvar org-x11idle-exists-p
   ;; Check that x11idle exists
-  (and (eq window-system 'x)
-   (eq 0 (call-process-shell-command
+  (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'?
-- 
2.35.1



-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: org-x11idle-exists-p with emacs --daemon

[Ihor Radchenko]

Max Nikulin  writes:

> I am not familiar with `org-x11idle-exists-p', but a problem with access 
> of X clipboard from :immediate-finish capture templates may be quite 
> similar. The following command creates a hidden X11 frame if no visible 
> one exists:
>
> emacsclient --eval '(server-select-display (getenv "DISPLAY"))'

You are talking about a different problem.
Please start a new thread.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: org-x11idle-exists-p with emacs --daemon

[Max Nikulin]

On 28/10/2022 05:31, Julien Cubizolles wrote:


For me, org-x11idle-exists-p is always nil at startup, but is set to
true if I eval it later on. I guess it's because I'm starting Emacs in
daemon mode, (as a systemd user service actually), and
org-x11idle-exists-p relies on (eq window-system 'x) which is somehow
not set in the terminal when the daemon.


I am not familiar with `org-x11idle-exists-p', but a problem with access 
of X clipboard from :immediate-finish capture templates may be quite 
similar. The following command creates a hidden X11 frame if no visible 
one exists:


emacsclient --eval '(server-select-display (getenv "DISPLAY"))'

org-x11idle-exists-p with emacs --daemon

[Julien Cubizolles]

All the org-*-idle-seconds func test at some point for
org-x11idle-exists-p which is defvar-ed at load time. 

For me, org-x11idle-exists-p is always nil at startup, but is set to
true if I eval it later on. I guess it's because I'm starting Emacs in
daemon mode, (as a systemd user service actually), and
org-x11idle-exists-p relies on (eq window-system 'x) which is somehow
not set in the terminal when the daemon.

For the time being, it manually set org-x11idle-exists-p to true but
shouldn't all this tests on (eq-window-system) be run each time a new
frame is created ?
-- 
Julien Cubizolles