emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Bug: ‘(org-resolve-clocks)’ picks the wrong target for placing a new clock-drawer when ‘org-clock-out-remove-zero-time-clocks’ is set to t [9.1.14 (9.1.14-9-g131531-elpa @ ~/.emacs.d/elpa/org-20181126/)]
@ 2018-12-02 18:22 Leo Vivier
  2018-12-05 23:26 ` Nicolas Goaziou
  0 siblings, 1 reply; 3+ messages in thread
From: Leo Vivier @ 2018-12-02 18:22 UTC (permalink / raw)
  To: emacs-orgmode

Hello,

There seems to be a bad interaction between ‘(org-resolve-clocks)’ 
and ‘org-clock-out-remove-zero-time-clocks’ set to t. Whilst the 
right tree is targeted by ‘(org-resolve-clocks)’ to delete the 
clock-line and clock-drawer, it adds a new clock-drawer in the 
next tree rather than on the one being acted on.

I was able to replicate this problem with ‘emacs -Q’.


DESCRIPTION:


I use org-clock regularly, and recently re-discovered 
‘org-clock-out-remove-zero-time-clocks’. When I forget to clock an 
item, I run the following commands in quick succession:
# ------------------
(org-clock-in)
(org-resolve-clocks)
<INPUT>: g 10 <ENTER> (For indicating that I ‘got back’ 10 min 
ago)
# ------------------


Because those commands are run in quick succession, the time 
between ‘(org-clock-in)’ and ‘(org-resolve-clocks)’ is usually 
equal to 0 min. Therefore, when ‘(org-resolve-clocks)’ calls 
‘(org-clock-out)’ after pressing <ENTER>, the clock-line is 
deleted, and if the clock-drawer was created by ‘(org-clock-in)’, 
it also gets deleted.


The problem occurs in this context (‘|’ represents ‘(point)’):
# ------------------
* Subtree 1
** Item|
* Subtree 2
# ------------------
‘Item’ is the subtree we want to clock in the past. ‘Subtrees 1 & 
2’ are regular subtrees without any newlines separating them (the 
white-space is important).
Please note that I was only able to get ‘(org-resolve-clocks)’ to 
trigger in an agenda-file with already had clocking info. I 
recommend that you try the snippet in one of your own agenda-files 
rather than trying it in a blank buffer.


Going through the steps mentioned above, here’s the result after 
‘(org-clock-in)’
# ------------------
* Subtree 1
** Item|
:LOGBOOK-CLOCK:
CLOCK: [2018-12-02 Sun 12:00]
:END:
* Subtree 2
# ------------------


And here’s the result after ‘(org-resolve-clocks)’:
# ------------------
* Subtree 1
** Item|
* Subtree 2
:LOGBOOK-CLOCK:
CLOCK: [2018-12-02 Sun 11:50]
:END:
# ------------------


As you can see, the clock-line and clock-drawer are properly 
deleted under ‘Item’, but the new clock-drawer is spawned in the 
next subtree (‘Subtree 2’).
However, if we add a newline after ‘Item’, after running 
‘(org-resolve-clocks)’, the drawer is properly generated under 
‘Item’.


INVESTIGATION:


I’ve investigated the problem with edebug, and it seems to be a 
problem with the way the point is restored after a 
‘(save-restriction …)’ in ‘(org-clock-out)’ (the line is prefixed 
with an arrow ‘->’ in the following snippet).
# ------------------
(defun org-clock-out (&optional switch-to-state fail-quietly 
at-time)
  "Stop the currently running clock.
Throw an error if there is no running clock and FAIL-QUIETLY is 
nil.
With a universal prefix, prompt for a state to switch the clocked 
out task
to, overriding the existing value of 
`org-clock-out-switch-to-state'."
  (interactive "P")
  (catch 'exit
    (when (not (org-clocking-p))
      (setq global-mode-string
	    (delq 'org-mode-line-string global-mode-string))
      (setq frame-title-format org-frame-title-format-backup)
      (force-mode-line-update)
      (if fail-quietly (throw 'exit t) (user-error "No active 
      clock")))
    (let ((org-clock-out-switch-to-state
	   (if switch-to-state
	       (completing-read "Switch to state: "
				(with-current-buffer
				    (marker-buffer 
				    org-clock-marker)
				  org-todo-keywords-1)
				nil t "DONE")
	     org-clock-out-switch-to-state))
	  (now (org-current-time org-clock-rounding-minutes))
	  ts te s h m remove)
      (setq org-clock-out-time now)
->    (save-excursion ; Do not replace this with 
      `with-current-buffer'.
	(with-no-warnings (set-buffer (org-clocking-buffer)))
	(save-restriction
	  (widen)
	  (goto-char org-clock-marker)
	  (beginning-of-line 1)
	  (if (and (looking-at (concat "[ \t]*" 
	  org-keyword-time-regexp))
		   (equal (match-string 1) org-clock-string))
	      (setq ts (match-string 2))
	    (if fail-quietly (throw 'exit nil) (error "Clock start 
	    time is gone")))
	  (goto-char (match-end 0))
	  (delete-region (point) (point-at-eol))
	  (insert "--")
	  (setq te (org-insert-time-stamp (or at-time now) 
	  'with-hm 'inactive))
	  (setq s (- (float-time
		      (apply #'encode-time (org-parse-time-string 
		      te)))
		     (float-time
		      (apply #'encode-time (org-parse-time-string 
		      ts))))
		h (floor (/ s 3600))
		s (- s (* 3600 h))
		m (floor (/ s 60))
		s (- s (* 60 s)))
	  (insert " => " (format "%2d:%02d" h m))
	  (move-marker org-clock-marker nil)
	  (move-marker org-clock-hd-marker nil)
	  ;; Possibly remove zero time clocks.  However, do not 
             add
	  ;; a note associated to the CLOCK line in this case.
	  (cond ((and org-clock-out-remove-zero-time-clocks
		      (= (+ h m) 0))
		 (setq remove t)
		 (delete-region (line-beginning-position)
				(line-beginning-position 2)))
		(org-log-note-clock-out
		 (org-add-log-setup
		  'clock-out nil nil nil
		  (concat "# Task: " (org-get-heading t) 
		  "\n\n"))))
	  (when org-clock-mode-line-timer
	    (cancel-timer org-clock-mode-line-timer)
	    (setq org-clock-mode-line-timer nil))
	  (when org-clock-idle-timer
	    (cancel-timer org-clock-idle-timer)
	    (setq org-clock-idle-timer nil))
	  (setq global-mode-string
		(delq 'org-mode-line-string global-mode-string))
	  (setq frame-title-format org-frame-title-format-backup)
	  (when org-clock-out-switch-to-state
	    (save-excursion
	      (org-back-to-heading t)
	      (let ((org-clock-out-when-done nil))
		(cond
		 ((functionp org-clock-out-switch-to-state)
		  (let ((case-fold-search nil))
		    (looking-at org-complex-heading-regexp))
		  (let ((newstate (funcall 
		  org-clock-out-switch-to-state
					   (match-string 2))))
		    (when newstate (org-todo newstate))))
		 ((and org-clock-out-switch-to-state
		       (not (looking-at (concat org-outline-regexp 
		       "[ \t]*"
						org-clock-out-switch-to-state
						"\\>"))))
		  (org-todo org-clock-out-switch-to-state))))))
	  (force-mode-line-update)
	  (message (concat "Clock stopped at %s after "
			   (org-duration-from-minutes (+ (* 60 h) 
			   m)) "%s")
		   te (if remove " => LINE REMOVED" ""))
	  (run-hooks 'org-clock-out-hook)
	  (unless (org-clocking-p)
	    (setq org-clock-current-task nil)))))))
# ------------------


There’s another ‘(save restriction)’ in 
‘(org-clock-remove-empty-clock-drawer)’, but I figured that since 
it is a ‘(save-restriction)’ within another one, it shouldn’t 
matter.
# ------------------
(defun org-clock-remove-empty-clock-drawer ()
  "Remove empty clock drawers in current subtree."
->(save-excursion
    (org-back-to-heading t)
    (org-map-tree
     (lambda ()
       (let ((drawer (org-clock-drawer-name))
	     (case-fold-search t))
	 (when drawer
	   (let ((re (format "^[ \t]*:%s:[ \t]*$" (regexp-quote 
	   drawer)))
		 (end (save-excursion (org-end-of-subtree))))
	     (while (re-search-forward re end t)
	       (org-remove-empty-drawer-at (point))))))))))
# ------------------


I’m going to investigate further to see if I can fix it reliably 
on my own, but in the meantime, would you have any idea on how to 
fix it?

Thanks for taking the time to look at my report.

HTH,
-- 
Leo Vivier
English Studies & General Linguistics
Master Student, English Department
Université Rennes 2

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Bug: ‘(org-resolve-clocks)’  picks the wrong target for placing a new clock-drawer when ‘org-clock-out-remove-zero-time-clocks’ is set to t [9.1.14 (9.1.14-9-g131531-elpa @ ~/.emacs.d/elpa/org-20181126/)]
  2018-12-02 18:22 Bug: ‘(org-resolve-clocks)’ picks the wrong target for placing a new clock-drawer when ‘org-clock-out-remove-zero-time-clocks’ is set to t [9.1.14 (9.1.14-9-g131531-elpa @ ~/.emacs.d/elpa/org-20181126/)] Leo Vivier
@ 2018-12-05 23:26 ` Nicolas Goaziou
  2018-12-06 10:17   ` Leo Vivier
  0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Goaziou @ 2018-12-05 23:26 UTC (permalink / raw)
  To: Leo Vivier; +Cc: emacs-orgmode

Hello,

Leo Vivier <leo.vivier@gmail.com> writes:

> Hello,
>
> There seems to be a bad interaction between ‘(org-resolve-clocks)’ and
> ‘org-clock-out-remove-zero-time-clocks’ set to t. Whilst the right
> tree is targeted by ‘(org-resolve-clocks)’ to delete the clock-line
> and clock-drawer, it adds a new clock-drawer in the next tree rather
> than on the one being acted on.
>
> I was able to replicate this problem with ‘emacs -Q’.
>
>
> DESCRIPTION:
>
>
> I use org-clock regularly, and recently re-discovered
> ‘org-clock-out-remove-zero-time-clocks’. When I forget to clock an
> item, I run the following commands in quick succession:
> # ------------------
> (org-clock-in)
> (org-resolve-clocks)
> <INPUT>: g 10 <ENTER> (For indicating that I ‘got back’ 10 min ago)
> # ------------------
>
>
> Because those commands are run in quick succession, the time between
> ‘(org-clock-in)’ and ‘(org-resolve-clocks)’ is usually equal to 0 min.
> Therefore, when ‘(org-resolve-clocks)’ calls ‘(org-clock-out)’ after
> pressing <ENTER>, the clock-line is deleted, and if the clock-drawer
> was created by ‘(org-clock-in)’, it also gets deleted.
>
>
> The problem occurs in this context (‘|’ represents ‘(point)’):
> # ------------------
> * Subtree 1
> ** Item|
> * Subtree 2
> # ------------------
> ‘Item’ is the subtree we want to clock in the past. ‘Subtrees 1 & 2’
> are regular subtrees without any newlines separating them (the
> white-space is important).
> Please note that I was only able to get ‘(org-resolve-clocks)’ to
> trigger in an agenda-file with already had clocking info. I recommend
> that you try the snippet in one of your own agenda-files rather than
> trying it in a blank buffer.

Fixed. Thank you.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Bug: ‘(org-resolve-clocks)’  picks the wrong target for placing a new clock-drawer when ‘org-clock-out-remove-zero-time-clocks’ is set to t [9.1.14 (9.1.14-9-g131531-elpa @ ~/.emacs.d/elpa/org-20181126/)]
  2018-12-05 23:26 ` Nicolas Goaziou
@ 2018-12-06 10:17   ` Leo Vivier
  0 siblings, 0 replies; 3+ messages in thread
From: Leo Vivier @ 2018-12-06 10:17 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-orgmode

Hello,

Thank you for your quick patch.
Since I wasn’t able to solve it on my own, I’ll take a look at 
your solution to understand what you did.

Have a great day.

Best,

Nicolas Goaziou writes:

> Hello,
>
> Leo Vivier <leo.vivier@gmail.com> writes:
>
>> Hello,
>>
>> There seems to be a bad interaction between 
>> ‘(org-resolve-clocks)’ and
>> ‘org-clock-out-remove-zero-time-clocks’ set to t. Whilst the 
>> right
>> tree is targeted by ‘(org-resolve-clocks)’ to delete the 
>> clock-line
>> and clock-drawer, it adds a new clock-drawer in the next tree 
>> rather
>> than on the one being acted on.
>>
>> I was able to replicate this problem with ‘emacs -Q’.
>>
>>
>> DESCRIPTION:
>>
>>
>> I use org-clock regularly, and recently re-discovered
>> ‘org-clock-out-remove-zero-time-clocks’. When I forget to clock 
>> an
>> item, I run the following commands in quick succession:
>> # ------------------
>> (org-clock-in)
>> (org-resolve-clocks)
>> <INPUT>: g 10 <ENTER> (For indicating that I ‘got back’ 10 min 
>> ago)
>> # ------------------
>>
>>
>> Because those commands are run in quick succession, the time 
>> between
>> ‘(org-clock-in)’ and ‘(org-resolve-clocks)’ is usually equal to 
>> 0 min.
>> Therefore, when ‘(org-resolve-clocks)’ calls ‘(org-clock-out)’ 
>> after
>> pressing <ENTER>, the clock-line is deleted, and if the 
>> clock-drawer
>> was created by ‘(org-clock-in)’, it also gets deleted.
>>
>>
>> The problem occurs in this context (‘|’ represents ‘(point)’):
>> # ------------------
>> * Subtree 1
>> ** Item|
>> * Subtree 2
>> # ------------------
>> ‘Item’ is the subtree we want to clock in the past. ‘Subtrees 1 
>> & 2’
>> are regular subtrees without any newlines separating them (the
>> white-space is important).
>> Please note that I was only able to get ‘(org-resolve-clocks)’ 
>> to
>> trigger in an agenda-file with already had clocking info. I 
>> recommend
>> that you try the snippet in one of your own agenda-files rather 
>> than
>> trying it in a blank buffer.
>
> Fixed. Thank you.
>
> Regards,


-- 
Leo Vivier
English Studies & General Linguistics
Master Student, English Department
Université Rennes 2

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-12-06 10:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-02 18:22 Bug: ‘(org-resolve-clocks)’ picks the wrong target for placing a new clock-drawer when ‘org-clock-out-remove-zero-time-clocks’ is set to t [9.1.14 (9.1.14-9-g131531-elpa @ ~/.emacs.d/elpa/org-20181126/)] Leo Vivier
2018-12-05 23:26 ` Nicolas Goaziou
2018-12-06 10:17   ` Leo Vivier

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).