[O] 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]

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