* A bit of work around org-clock-idle-time
@ 2012-07-18 20:38 Nicolas Calderon
2012-07-18 22:43 ` Nick Dokos
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Calderon @ 2012-07-18 20:38 UTC (permalink / raw)
To: emacs-orgmode
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.
Thanks,
--
Nicolas Calderon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: A bit of work around org-clock-idle-time
2012-07-18 20:38 A bit of work around org-clock-idle-time Nicolas Calderon
@ 2012-07-18 22:43 ` Nick Dokos
2012-07-27 23:43 ` Bastien
0 siblings, 1 reply; 6+ messages in thread
From: Nick Dokos @ 2012-07-18 22:43 UTC (permalink / raw)
To: Nicolas Calderon; +Cc: emacs-orgmode
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: A bit of work around org-clock-idle-time
2012-07-18 22:43 ` Nick Dokos
@ 2012-07-27 23:43 ` Bastien
2012-07-28 3:19 ` Nick Dokos
0 siblings, 1 reply; 6+ messages in thread
From: Bastien @ 2012-07-27 23:43 UTC (permalink / raw)
To: nicholas.dokos; +Cc: Nicolas Calderon, emacs-orgmode
Nick Dokos <nicholas.dokos@hp.com> writes:
> 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))
> ...
Yes, this is cleaner, thanks!
--
Bastien
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: A bit of work around org-clock-idle-time
2012-07-27 23:43 ` Bastien
@ 2012-07-28 3:19 ` Nick Dokos
2012-07-28 9:30 ` Nicolas Calderon
0 siblings, 1 reply; 6+ messages in thread
From: Nick Dokos @ 2012-07-28 3:19 UTC (permalink / raw)
To: Bastien; +Cc: Nicolas Calderon, emacs-orgmode
Bastien <bzg@gnu.org> wrote:
> Nick Dokos <nicholas.dokos@hp.com> writes:
>
> > 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))
> > ...
>
> Yes, this is cleaner, thanks!
>
Cleaner maybe, but it's not correct, as Nicolas C. pointed out. Nicolas
C. and I had an email exchange and I hope he will follow up with revised
patches.
Nick
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: A bit of work around org-clock-idle-time
2012-07-28 3:19 ` Nick Dokos
@ 2012-07-28 9:30 ` Nicolas Calderon
2012-07-28 9:54 ` Bastien
0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Calderon @ 2012-07-28 9:30 UTC (permalink / raw)
To: nicholas.dokos; +Cc: Bastien, emacs-orgmode
Sorry, I forgot to reply to all when Nicolas D. answered the first
email and took it off the list. Here is what you missed regarding
org-clock.el:
On Thu, Jul 19, 2012 at 2:33 PM, Nick Dokos <nicholas.dokos@hp.com> wrote:
> Nicolas Calderon <nicolas.calderon.asselin@gmail.com> wrote:
>
>>
>> As for the max solution for idle time, I don't think it's a good
>> solution. It solves the case where x11idle always return 0, but if
>> you're active in X (x11idle always low) but not in emacs (high idle
>> time), you'll end up in clock resolving mode.
>>
>
> Fair enough, but it can still be done so as to simulate today's behavior
> while avoiding the 0 case. Something like
>
> ((eq window-system 'x)
> (let ((idle (org-x11-idle-seconds)))
> (if (> idle 0)
> idle
> (org-emacs-idle-seconds))))
The problem I see here is that 0 is a valid output for x11idle, so if
the check is done while X11 is not idle, it will fall back to
org-emacs-idle-seconds. From what I could see in the
shell-command-to-string, there is no way to tell between an output
from the program and the 0 the function returns if there is no output
(can't find the command). That's why I added the additional check for
the command existence.
Optimally, that check would be done once at startup or the first time
idle time is checked but I lack the elisp skills to perform such a
"feat".
That being said, the patch was merged in, so I don't know if it's an
indication that I should not worry.
Thanks,
--
Nicolas Calderon
On Fri, Jul 27, 2012 at 11:19 PM, Nick Dokos <nicholas.dokos@hp.com> wrote:
> Bastien <bzg@gnu.org> wrote:
>
>> Nick Dokos <nicholas.dokos@hp.com> writes:
>>
>> > 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))
>> > ...
>>
>> Yes, this is cleaner, thanks!
>>
>
> Cleaner maybe, but it's not correct, as Nicolas C. pointed out. Nicolas
> C. and I had an email exchange and I hope he will follow up with revised
> patches.
>
> Nick
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: A bit of work around org-clock-idle-time
2012-07-28 9:30 ` Nicolas Calderon
@ 2012-07-28 9:54 ` Bastien
0 siblings, 0 replies; 6+ messages in thread
From: Bastien @ 2012-07-28 9:54 UTC (permalink / raw)
To: Nicolas Calderon; +Cc: nicholas.dokos, emacs-orgmode
Nicolas Calderon <nicolas.calderon.asselin@gmail.com> writes:
> Sorry, I forgot to reply to all when Nicolas D. answered the first
> email and took it off the list. Here is what you missed regarding
> org-clock.el:
Thanks for resending.
> Optimally, that check would be done once at startup or the first time
> idle time is checked but I lack the elisp skills to perform such a
> "feat".
This is now the case.
> That being said, the patch was merged in, so I don't know if it's an
> indication that I should not worry.
You should _always_ worry. :)
--
Bastien
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-28 9:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-18 20:38 A bit of work around org-clock-idle-time Nicolas Calderon
2012-07-18 22:43 ` Nick Dokos
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
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).