From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Dokos Subject: Re: A bit of work around org-clock-idle-time Date: Wed, 18 Jul 2012 18:43:23 -0400 Message-ID: <8935.1342651403@alphaville> References: Reply-To: nicholas.dokos@hp.com Return-path: Received: from eggs.gnu.org ([208.118.235.92]:55242) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Srcxs-00051Y-0R for emacs-orgmode@gnu.org; Wed, 18 Jul 2012 18:43:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Srcxq-0001pI-A1 for emacs-orgmode@gnu.org; Wed, 18 Jul 2012 18:43:27 -0400 Received: from g4t0016.houston.hp.com ([15.201.24.19]:19286) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Srcxp-0001ow-Nz for emacs-orgmode@gnu.org; Wed, 18 Jul 2012 18:43:25 -0400 In-Reply-To: Message from Nicolas Calderon of "Wed, 18 Jul 2012 16:38:06 EDT." List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Nicolas Calderon Cc: emacs-orgmode@gnu.org Nicolas Calderon 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.