Re: [PATCH] Avoid losing persisted clock data when exiting emac without loading org-mode

From: Carsten Dominik <carsten.dominik@gmail.com>
To: Kai Tetzlaff <kai.tetzlaff@gmx.de>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] Avoid losing persisted clock data when exiting emac without loading org-mode
Date: Sun, 25 Oct 2009 09:10:58 +0100	[thread overview]
Message-ID: <815420E4-6D45-4301-86B5-BD16BCD602F9@gmail.com> (raw)
In-Reply-To: <4AE34A24.2030608@gmx.de>

Hi Kai,

after giving it more thought, what I checked in is actually
a combination of the two patches you sent.

Thanks you for finding and fixing this issue.

- Carsten

On Oct 24, 2009, at 8:40 PM, Kai Tetzlaff wrote:

> Carsten Dominik wrote:
>> Hi Kai,
>>
>>
>> On Oct 23, 2009, at 9:01 AM, Kai Tetzlaff wrote:
>>
>>> Hi,
>>>
>>> i noticed that when using the org-mode clock persistence, the stored
>>> clock data gets deleted when i start emacs and exit again without
>>> turning on org-mode in between.
>>>
>>> When looking at org-clock-persistence-insinuate it looks like org- 
>>> clock
>>> load will only run after org-mode gets started whereas org-clock- 
>>> save
>>> will always be called when exiting emacs:
>>>
>>> (defun org-clock-persistence-insinuate ()
>>> "Set up hooks for clock persistence"
>>> (add-hook 'org-mode-hook 'org-clock-load)
>>> (add-hook 'kill-emacs-hook 'org-clock-save))
>>>
>>> Not running org-mode-hook (i.e. not starting org-mode) thus does not
>>> load clock data but org-clock-save overwrites any prviously saved  
>>> data
>>> when exiting emacs.
>>>
>>> An easy fix for that would be to just add org-clock-load to e.g.
>>> emacs-startup-hook. But this will only work if the code in
>>> org-clock-load does not depend on any org-mode initialization code  
>>> (or
>>> would require loading org-mode).
>>>
>>> So org-clock-save should probably check if org-clock-load has been
>>> running during the current emacs session (or if clock persistence  
>>> was
>>> just enabled) and only then save clock data when exiting emacs. I  
>>> tried
>>> to add this to the code in org-clock-save:
>>>
>>> diff --git a/lisp/org-clock.el b/lisp/org-clock.el
>>> index c7ebbf8..c0fe4e6 100644
>>> --- a/lisp/org-clock.el
>>> +++ b/lisp/org-clock.el
>>> @@ -1803,7 +1803,8 @@ This function is made for clock tables."
>>>  "Persist various clock-related data to disk.
>>> The details of what will be saved are regulated by the variable
>>> `org-clock-persist'."
>>> -  (when org-clock-persist
>>> +  (when (and org-clock-persist
>>> +             (or org-clock-loaded (not (file-exists-p
>>> org-clock-persist-file))))
>>>    (let (b)
>>>      (with-current-buffer (find-file (expand-file-name
>>> org-clock-persist-file))
>>>    (progn
>>>
>>>
>>> /Kai
>>>
>>
>> I see you point, but I am not convinced that you fix is the right  
>> one.
>> What one really wants to do is save the clock and history if and  
>> only if
>> the clock has been used in the current session.  So I guess it is  
>> better
>> to create a special variable org-clock-used-in-this-session which  
>> then
>> will be set to t each time clock-in starts a clock somewhere.  I  
>> believe
>> this would be a better insurance policy.
>>
>> Would you like to try your hand at a patch in this spirit?
>
> Ok, i've attached a new path which uses the suggested approach. I  
> wasn't
> exactly sure about where to set org-clock-used-in-this-session to t.
> It's now done right at the end of org-clock-in, just before running
> hooks in org-clock-in-hook.
>
> I've done a couple of (simple) tests and it works fine for me.
>
> /Kai
>
>> - Carsten
>>
>>> _______________________________________________
>>> Emacs-orgmode mailing list
>>> Remember: use `Reply All' to send replies to the list.
>>> Emacs-orgmode@gnu.org
>>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>>
>> - Carsten
>>
>>
>>
>>
>
>
> diff --git a/lisp/org-clock.el b/lisp/org-clock.el
> index b1041e2..467ffc4 100644
> --- a/lisp/org-clock.el
> +++ b/lisp/org-clock.el
> @@ -244,6 +244,7 @@ to add an effort property.")
> (defvar org-clock-heading "")
> (defvar org-clock-heading-for-remember "")
> (defvar org-clock-start-time "")
> +(defvar org-clock-used-in-this-session nil)
>
> (defvar org-clock-left-over-time nil
>   "If non-nil, user cancelled a clock; this is when leftover time  
> started.")
> @@ -953,6 +954,7 @@ the clocking selection, associated with the  
> letter `d'."
> 	    (setq org-clock-idle-timer
> 		  (run-with-timer 60 60 'org-resolve-clocks-if-idle))
> 	    (message "Clock starts at %s - %s" ts msg-extra)
> +            (setq org-clock-used-in-this-session t)
> 	    (run-hooks 'org-clock-in-hook)))))))
>
> (defun org-clock-mark-default-task ()
> @@ -1824,7 +1826,7 @@ This function is made for clock tables."
>   "Persist various clock-related data to disk.
> The details of what will be saved are regulated by the variable
> `org-clock-persist'."
> -  (when org-clock-persist
> +  (when (and org-clock-persist org-clock-used-in-this-session)
>     (let (b)
>       (with-current-buffer (find-file (expand-file-name org-clock- 
> persist-file))
> 	(progn

- Carsten