Re: [O] A bit of work around org-clock-idle-time

[Nick Dokos]

Nicolas Calderon <address@hidden> 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.