emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Nick Dokos <nicholas.dokos@hp.com>
To: Nicolas Calderon <nicolas.calderon.asselin@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: A bit of work around org-clock-idle-time
Date: Wed, 18 Jul 2012 18:43:23 -0400	[thread overview]
Message-ID: <8935.1342651403@alphaville> (raw)
In-Reply-To: Message from Nicolas Calderon <nicolas.calderon.asselin@gmail.com> of "Wed, 18 Jul 2012 16:38:06 EDT." <CAHbMD+EqDFc3qRn=VYqL4+_TYTsNp+Ttx=McV-HXtu2__qD8VQ@mail.gmail.com>

Nicolas Calderon <nicolas.calderon.asselin@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.

  reply	other threads:[~2012-07-18 22:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-18 20:38 A bit of work around org-clock-idle-time Nicolas Calderon
2012-07-18 22:43 ` Nick Dokos [this message]
2012-07-27 23:43   ` Bastien
2012-07-28  3:19     ` Nick Dokos
2012-07-28  9:30       ` Nicolas Calderon
2012-07-28  9:54         ` Bastien

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8935.1342651403@alphaville \
    --to=nicholas.dokos@hp.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=nicolas.calderon.asselin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).