Nicolas Calderon <nicolas.calderon.asse...@gmail.com> wrote:
> Hello,
>
> I was trying to get org-clock-idle-time to work on my machine, but it
> would never kick in. Looking at the doc
> (http://orgmode.org/manual/Resolving-idle-time.html), I was left under
> the impression that x11idle was an option for a better experience, but
> emacs idle time would be used otherwise. After digging around a bit, I
> found out it was not the case. If you are using X, emacs WILL use
> x11idle, wether you have it or not, and in the latter case always get
> an idle time of 0.
>
> From that, I have two patches to submit (next 2 emails):
>
> I made a few modifications to x11idle itself. It seemed it could crash
> in many ways, one that was noted in comments but somehow not averted
> by the addition of a if. I added a few more checks, and made it return
> more meaningful error codes (more on that later).
>
>
> Since org-mode doesn't depend on x11idle being installed on the
> machine (at least not on debian), I thought it could be interesting to
> add a few checks. First of all, I make sure that the command exists (I
> used this post to do it the most generic way,
> http://stackoverflow.com/questions/592620/check-if-a-program-exists-from-a-bash-script),
> and then, that the command can execute properly (can connect to the
> display, there is enough memory for the info struct and the reporting
> of idle time is supported). I'm not sure this is the best
> implementation (how often does this get called? If it's often, it
> might be worth caching the results rather than invoking two shell
> commands every time), but that's as good as I could do with my
> knowledge of lisp (none, as of before looking into this).
>
> Hopefully, all this will respect what I read here:
> http://orgmode.org/worg/org-contribute.html.
>
Fixing these problems is a good idea, but I have some comments on your
fixes:
o The x11idle.c fixes have whitespace problems, there is an
unnecessary (badly named *and* misspelled) variable introduced[fn:1]
and, at least on my (64-bit) platform the %u format causes the
compiler to complain: it needs to be %lu in my case. This last is of
course an original problem, not a problem that you introduced, but if
you are going to fix things, maybe it should be fixed as well, but I'm
not sure whether %lu is OK on a 32-bit system - can somebody check?
o in org-clock.el, instead of checking whether x11idle exists or not, how
about something like this:
...
(eq window-system 'x)
(max (org-x11-idle-seconds) (org-emacs-idle-seconds))
...
This is no worse than it is today as far as efficiency goes, but in any
case, it's not so much the inefficiency of calling out to the shell that I
object to, it's more the unnecessary complications added to the code.
Also, it is easy to memoize org-x11-idle-seconds so that it doesn't call
out to the shell every time, if that seems desirable, but that can be deferred
for later.
Thanks for finding and fixing the problems: I wouldn't have looked if
you hadn't pointed the way! Hope the comments are useful.
Nick
Footnotes:
[fn:1] I'm talking about "querry" - you can leave it out altogether:
...
//X11 is running, try to retrieve info
if (XScreenSaverQueryInfo(display, DefaultRootWindow(display), info) == 0) {
return -1;
}
//info was retrieved successfully, print idle time
printf("%lu\n", info->idle);
...
will do just as well.
