emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* org-x11idle-exists-p with emacs --daemon
@ 2022-10-27 22:31 Julien Cubizolles
  2022-10-28  3:08 ` Max Nikulin
  2022-10-28  4:28 ` Ihor Radchenko
  0 siblings, 2 replies; 16+ messages in thread
From: Julien Cubizolles @ 2022-10-27 22:31 UTC (permalink / raw)
  To: emacs-orgmode


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



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-27 22:31 org-x11idle-exists-p with emacs --daemon Julien Cubizolles
@ 2022-10-28  3:08 ` Max Nikulin
  2022-10-28  4:29   ` Ihor Radchenko
  2022-10-28  4:28 ` Ihor Radchenko
  1 sibling, 1 reply; 16+ messages in thread
From: Max Nikulin @ 2022-10-28  3:08 UTC (permalink / raw)
  To: emacs-orgmode

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"))'





^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-27 22:31 org-x11idle-exists-p with emacs --daemon Julien Cubizolles
  2022-10-28  3:08 ` Max Nikulin
@ 2022-10-28  4:28 ` Ihor Radchenko
  2022-10-28 21:48   ` Julien Cubizolles
  2022-10-29 12:33   ` Max Nikulin
  1 sibling, 2 replies; 16+ messages in thread
From: Ihor Radchenko @ 2022-10-28  4:28 UTC (permalink / raw)
  To: Julien Cubizolles; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

Julien Cubizolles <j.cubizolles@free.fr> 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?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-x11idle-exists-p-Do-not-demand-X-window-system-a.patch --]
[-- Type: text/x-patch, Size: 1335 bytes --]

From 95ec1073ab27bbfe522b25c9852f1e5e85e38e32 Mon Sep 17 00:00:00 2001
Message-Id: <95ec1073ab27bbfe522b25c9852f1e5e85e38e32.1666931293.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
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 <j.cubizolles@free.fr>
Link: https://orgmode.org/list/871qqs6gqs.fsf@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


[-- Attachment #3: Type: text/plain, Size: 225 bytes --]



-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-28  3:08 ` Max Nikulin
@ 2022-10-28  4:29   ` Ihor Radchenko
  2022-10-28  4:39     ` Max Nikulin
  0 siblings, 1 reply; 16+ messages in thread
From: Ihor Radchenko @ 2022-10-28  4:29 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> 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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-28  4:29   ` Ihor Radchenko
@ 2022-10-28  4:39     ` Max Nikulin
  2022-10-28  5:19       ` Ihor Radchenko
  0 siblings, 1 reply; 16+ messages in thread
From: Max Nikulin @ 2022-10-28  4:39 UTC (permalink / raw)
  To: emacs-orgmode

On 28/10/2022 11:29, Ihor Radchenko wrote:
> Max Nikulin <manikulin@gmail.com> 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.




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-28  4:39     ` Max Nikulin
@ 2022-10-28  5:19       ` Ihor Radchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Ihor Radchenko @ 2022-10-28  5:19 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> 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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-28  4:28 ` Ihor Radchenko
@ 2022-10-28 21:48   ` Julien Cubizolles
  2022-10-29  2:54     ` Ihor Radchenko
  2022-10-29 12:33   ` Max Nikulin
  1 sibling, 1 reply; 16+ messages in thread
From: Julien Cubizolles @ 2022-10-28 21:48 UTC (permalink / raw)
  To: emacs-orgmode

Ihor Radchenko <yantar92@posteo.net> 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



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-28 21:48   ` Julien Cubizolles
@ 2022-10-29  2:54     ` Ihor Radchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Ihor Radchenko @ 2022-10-29  2:54 UTC (permalink / raw)
  To: Julien Cubizolles; +Cc: emacs-orgmode

Julien Cubizolles <j.cubizolles@free.fr> writes:

> Ihor Radchenko <yantar92@posteo.net> 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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-28  4:28 ` Ihor Radchenko
  2022-10-28 21:48   ` Julien Cubizolles
@ 2022-10-29 12:33   ` Max Nikulin
  2022-10-30  1:33     ` Ihor Radchenko
  1 sibling, 1 reply; 16+ messages in thread
From: Max Nikulin @ 2022-10-29 12:33 UTC (permalink / raw)
  To: emacs-orgmode

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.




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-29 12:33   ` Max Nikulin
@ 2022-10-30  1:33     ` Ihor Radchenko
  2022-10-30  6:20       ` Max Nikulin
  0 siblings, 1 reply; 16+ messages in thread
From: Ihor Radchenko @ 2022-10-30  1:33 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> 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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-30  1:33     ` Ihor Radchenko
@ 2022-10-30  6:20       ` Max Nikulin
  2022-10-31  2:50         ` Ihor Radchenko
  2022-10-31  2:53         ` Change default value of org-clock-x11-idle-program-name (was: org-x11idle-exists-p with emacs --daemon) Ihor Radchenko
  0 siblings, 2 replies; 16+ messages in thread
From: Max Nikulin @ 2022-10-30  6:20 UTC (permalink / raw)
  To: emacs-orgmode

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.




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: org-x11idle-exists-p with emacs --daemon
  2022-10-30  6:20       ` Max Nikulin
@ 2022-10-31  2:50         ` Ihor Radchenko
  2022-10-31  2:53         ` Change default value of org-clock-x11-idle-program-name (was: org-x11idle-exists-p with emacs --daemon) Ihor Radchenko
  1 sibling, 0 replies; 16+ messages in thread
From: Ihor Radchenko @ 2022-10-31  2:50 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> 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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Change default value of org-clock-x11-idle-program-name (was: org-x11idle-exists-p with emacs --daemon)
  2022-10-30  6:20       ` Max Nikulin
  2022-10-31  2:50         ` Ihor Radchenko
@ 2022-10-31  2:53         ` Ihor Radchenko
  2023-01-24 12:41           ` [PATCH] Change default value of org-clock-x11-idle-program-name Ihor Radchenko
  1 sibling, 1 reply; 16+ messages in thread
From: Ihor Radchenko @ 2022-10-31  2:53 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Max Nikulin <manikulin@gmail.com> 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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] Change default value of org-clock-x11-idle-program-name
  2022-10-31  2:53         ` Change default value of org-clock-x11-idle-program-name (was: org-x11idle-exists-p with emacs --daemon) Ihor Radchenko
@ 2023-01-24 12:41           ` Ihor Radchenko
  2023-01-26 15:43             ` Max Nikulin
  2023-01-27 10:20             ` Ihor Radchenko
  0 siblings, 2 replies; 16+ messages in thread
From: Ihor Radchenko @ 2023-01-24 12:41 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 267 bytes --]

Ihor Radchenko <yantar92@posteo.net> writes:

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

See the attached patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-clock-x11idle-program-name-Prefer-xprintidle-whe.patch --]
[-- Type: text/x-patch, Size: 2634 bytes --]

From 6fe58852f30b7bd8d217bf228252dc97abd05153 Mon Sep 17 00:00:00 2001
Message-Id: <6fe58852f30b7bd8d217bf228252dc97abd05153.1674564051.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Tue, 24 Jan 2023 15:38:26 +0300
Subject: [PATCH] org-clock-x11idle-program-name: Prefer "xprintidle", when
 available

* lisp/org-clock.el (org-clock-x11idle-program-name): Change the
default value to "xprintidle" when its executable is available.
Fallback to previous default otherwise.  Update :package-version and
remove :version tags.
* etc/ORG-NEWS (New and changed options):
(~org-clock-x11idle-program-name~ now defaults to =xprintidle=, when available):
Document the change.

Link: https://orgmode.org/list/874jvkn1po.fsf@localhost
---
 etc/ORG-NEWS      | 11 ++++++++++-
 lisp/org-clock.el |  7 ++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 3ef76ec1a..d65592a2b 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -12,7 +12,16 @@ See the end of the file for license conditions.
 Please send Org bug reports to mailto:emacs-orgmode@gnu.org.
 
 * Version 9.7 (not released yet)
-** New options
+** New and changed options
+*** ~org-clock-x11idle-program-name~ now defaults to =xprintidle=, when available
+
+When =xprintidle= executable is available at =org-clock= load time, it
+is used as the default value for ~org-clock-x11idle-program-name~.
+The old =x11idle= default is used as the fallback.
+
+=xprintidle= is available as system package in most Linux
+distributions, unlike ancient =x11idle= that is distributed via WORG.
+
 *** New options for the "csl" citation export processor's LaTeX output
 
 The ~org-cite-csl-latex-label-separator~ and
diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 0cd473209..ceb1fc833 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -439,7 +439,9 @@ (defcustom org-clock-frame-title-format '(t org-mode-line-string)
   :group 'org-clock
   :type 'sexp)
 
-(defcustom org-clock-x11idle-program-name "x11idle"
+(defcustom org-clock-x11idle-program-name
+  (if (executable-find "xprintidle")
+      "xprintidle" "x11idle")
   "Name of the program which prints X11 idle time in milliseconds.
 
 you can do \"~$ sudo apt-get install xprintidle\" if you are using
@@ -448,8 +450,7 @@ (defcustom org-clock-x11idle-program-name "x11idle"
 Alternatively, can find x11idle.c in
 https://orgmode.org/worg/code/scripts/x11idle.c"
   :group 'org-clock
-  :version "24.4"
-  :package-version '(Org . "8.0")
+  :package-version '(Org . "9.7")
   :type 'string)
 
 (defcustom org-clock-goto-before-context 2
-- 
2.39.1


[-- Attachment #3: Type: text/plain, Size: 225 bytes --]



-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change default value of org-clock-x11-idle-program-name
  2023-01-24 12:41           ` [PATCH] Change default value of org-clock-x11-idle-program-name Ihor Radchenko
@ 2023-01-26 15:43             ` Max Nikulin
  2023-01-27 10:20             ` Ihor Radchenko
  1 sibling, 0 replies; 16+ messages in thread
From: Max Nikulin @ 2023-01-26 15:43 UTC (permalink / raw)
  To: emacs-orgmode

On 24/01/2023 19:41, Ihor Radchenko wrote:
> 
>> 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.
> 
> See the attached patch.

I have not tested your patch, however I consider it as an improvement. I 
hope, effect of `executable-find' is negligible in respect to load time.




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] Change default value of org-clock-x11-idle-program-name
  2023-01-24 12:41           ` [PATCH] Change default value of org-clock-x11-idle-program-name Ihor Radchenko
  2023-01-26 15:43             ` Max Nikulin
@ 2023-01-27 10:20             ` Ihor Radchenko
  1 sibling, 0 replies; 16+ messages in thread
From: Ihor Radchenko @ 2023-01-27 10:20 UTC (permalink / raw)
  To: Max Nikulin; +Cc: emacs-orgmode

Ihor Radchenko <yantar92@posteo.net> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
>> 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.
>
> See the attached patch.


Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=1810c625d

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-01-27 10:20 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-27 22:31 org-x11idle-exists-p with emacs --daemon Julien Cubizolles
2022-10-28  3:08 ` Max Nikulin
2022-10-28  4:29   ` Ihor Radchenko
2022-10-28  4:39     ` Max Nikulin
2022-10-28  5:19       ` Ihor Radchenko
2022-10-28  4:28 ` Ihor Radchenko
2022-10-28 21:48   ` Julien Cubizolles
2022-10-29  2:54     ` Ihor Radchenko
2022-10-29 12:33   ` Max Nikulin
2022-10-30  1:33     ` Ihor Radchenko
2022-10-30  6:20       ` Max Nikulin
2022-10-31  2:50         ` Ihor Radchenko
2022-10-31  2:53         ` Change default value of org-clock-x11-idle-program-name (was: org-x11idle-exists-p with emacs --daemon) Ihor Radchenko
2023-01-24 12:41           ` [PATCH] Change default value of org-clock-x11-idle-program-name Ihor Radchenko
2023-01-26 15:43             ` Max Nikulin
2023-01-27 10:20             ` Ihor Radchenko

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).