Bug: clocktable thinks Oct 29 is Oct 28 [9.1.14 (release_9.1.14-1012-g643481 @ /home/unhammer/src/org-mode/lisp/)]

Bug: clocktable thinks Oct 29 is Oct 28 [9.1.14 (release_9.1.14-1012-g643481 @ /home/unhammer/src/org-mode/lisp/)]

[Kevin Brubeck Unhammer]

Hi,

I did

$ git clone https://code.orgmode.org/bzg/org-mode.git

(which got me 643481fade386aa51d88ccc22b9669b0b77daaa1 )

$ cd org-mode
$ make autoloads
$ emacs -Q -l /tmp/init.el /tmp/foo.org

where /tmp/init.el has:

(add-to-list 'load-path "~/src/org-mode/lisp")
(require 'org)


and /tmp/foo.org has:


#+STARTUP: showall

* Top

#+BEGIN: clocktable :maxlevel 3  :tstart "2018-10-01" :tend "2018-11-01" :step day :stepskip0 t

#+END:

** TODO Undeobfuscate widget
    :LOGBOOK:
    CLOCK: [2018-10-29 må. 14:17]--[2018-10-29 må. 14:35] =>  0:18
    :END:
** DONE Deobfuscate widget
    :LOGBOOK:
    CLOCK: [2018-10-28 su. 14:17]--[2018-10-28 su. 14:35] =>  0:18
    :END:


and then, on the BEGIN line, I did C-c C-c. This gave me:
#+STARTUP: showall

* Top

#+BEGIN: clocktable :maxlevel 3  :tstart "2018-10-01" :tend "2018-11-01" :step day :stepskip0 t

Daily report: [2018-10-28 su.]
| Headline               | Time   |      |
|------------------------+--------+------|
| *Total time*           | *0:18* |      |
|------------------------+--------+------|
| Top                    | 0:18   |      |
| \_  Deobfuscate widget |        | 0:18 |

Daily report: [2018-10-28 su.]
| Headline                 | Time   |      |
|--------------------------+--------+------|
| *Total time*             | *0:18* |      |
|--------------------------+--------+------|
| Top                      | 0:18   |      |
| \_  Undeobfuscate widget |        | 0:18 |

#+END:

** TODO Undeobfuscate widget
    :LOGBOOK:
    CLOCK: [2018-10-29 må. 14:17]--[2018-10-29 må. 14:35] =>  0:18
    :END:
** DONE Deobfuscate widget
    :LOGBOOK:
    CLOCK: [2018-10-28 su. 14:17]--[2018-10-28 su. 14:35] =>  0:18
    :END:

The second date is 28, where it should be 29. I see the same on org-mode
9.1.9 (bundled with Emacs 26.1) or 9.1.12 and 9.1.14 from melpa-stable,
gnu elpa.

I do *not* see this in 8.2.10 bundled with Emacs 25.3.2.

The bug is also there without ":stepskip0 t" 


Emacs  : GNU Emacs 25.3.2 (x86_64-pc-linux-gnu, GTK+ Version 3.22.29)
 of 2018-04-19
Package: Org mode version 9.1.14 (release_9.1.14-1012-g643481 @ /home/unhammer/src/org-mode/lisp/)

current state:
==============
(setq
 org-tab-first-hook '(org-babel-hide-result-toggle-maybe
		      org-babel-header-arg-expand)
 org-speed-command-hook '(org-speed-command-activate
			  org-babel-speed-command-activate)
 org-occur-hook '(org-first-headline-recenter)
 org-metaup-hook '(org-babel-load-in-session-maybe)
 org-confirm-shell-link-function 'yes-or-no-p
 org-after-todo-state-change-hook '(org-clock-out-if-current)
 org-src-mode-hook '(org-src-babel-configure-edit-buffer
		     org-src-mode-configure-edit-buffer)
 org-agenda-before-write-hook '(org-agenda-add-entry-text)
 org-babel-pre-tangle-hook '(save-buffer)
 org-mode-hook '((closure
		  (org--rds reftex-docstruct-symbol orgtbl-line-start-regexp
		   org-element-greater-elements org-table-coordinate-overlays
		   org-table-auto-blank-field org-clock-history
		   org-agenda-current-date org-with-time org-defdecode org-def
		   org-read-date-inactive org-ans2 org-ans1
		   org-columns-current-fmt-compiled org-clock-current-task
		   org-clock-effort org-agenda-skip-function
		   org-agenda-skip-comment-trees org-agenda-archives-mode
		   org-end-time-was-given org-time-was-given
		   org-log-note-extra org-log-note-purpose
		   org-log-post-message org-last-inserted-timestamp
		   org-last-changed-timestamp
		   org-entry-property-inherited-from org-blocked-by-checkboxes
		   org-state org-agenda-headline-snapshot-before-repeat
		   org-capture-last-stored-marker org-agenda-start-on-weekday
		   org-agenda-buffer-tmp-name
		   clean-buffer-list-kill-buffer-names
		   org-id-link-to-org-use-id org-agenda-buffer-name
		   org-priority-regexp buffer-face-mode-face org-mode-map
		   org-tbl-menu org-org-menu org-struct-menu org-entities
		   org-last-state org-id-track-globally org-clock-start-time
		   texmathp-why remember-data-file
		   org-agenda-tags-todo-honor-ignore-options
		   iswitchb-temp-buflist calc-embedded-open-mode
		   calc-embedded-open-formula calc-embedded-close-formula
		   align-mode-rules-list org-emphasis-alist
		   org-emphasis-regexp-components
		   org-export-registered-backends org-modules
		   org-babel-load-languages org-indent-indentation-per-level
		   org-element-paragraph-separate ffap-url-regexp
		   org-inlinetask-min-level t)
		  nil
		  (add-hook (quote change-major-mode-hook)
		   (quote org-show-all) (quote append) (quote local))
		  )
		 (closure
		  (org-bracket-link-regexp org-src-window-setup *this*
		   org-babel-confirm-evaluate-answer-no
		   org-src-preserve-indentation org-src-lang-modes
		   org-link-file-path-type org-edit-src-content-indentation
		   org-babel-library-of-babel t)
		  nil
		  (add-hook (quote change-major-mode-hook)
		   (quote org-babel-show-result-all) (quote append)
		   (quote local))
		  )
		 org-babel-result-hide-spec org-babel-hide-all-hashes)
 org-bibtex-headline-format-function '(closure
				       (org-id-locations
					org-agenda-search-view-always-boolean
					org-agenda-overriding-header t)
				       (entry) (cdr (assq :title entry)))
 org-archive-hook '(org-attach-archive-delete-maybe)
 org-cycle-hook '(org-cycle-hide-archived-subtrees org-cycle-show-empty-lines
		  org-optimize-window-after-visibility-change)
 org-confirm-elisp-link-function 'yes-or-no-p
 org-metadown-hook '(org-babel-pop-to-session-maybe)
 org-link-parameters '(("id" :follow org-id-open)
		       ("eww" :follow eww :store org-eww-store-link)
		       ("rmail" :follow org-rmail-open :store
			org-rmail-store-link)
		       ("mhe" :follow org-mhe-open :store org-mhe-store-link)
		       ("irc" :follow org-irc-visit :store org-irc-store-link
			:export org-irc-export)
		       ("info" :follow org-info-open :export org-info-export
			:store org-info-store-link)
		       ("gnus" :follow org-gnus-open :store
			org-gnus-store-link)
		       ("docview" :follow org-docview-open :export
			org-docview-export :store org-docview-store-link)
		       ("bibtex" :follow org-bibtex-open :store
			org-bibtex-store-link)
		       ("bbdb" :follow org-bbdb-open :export org-bbdb-export
			:complete org-bbdb-complete-link :store
			org-bbdb-store-link)
		       ("w3m" :store org-w3m-store-link) ("file+sys")
		       ("file+emacs") ("doi" :follow org--open-doi-link)
		       ("elisp" :follow org--open-elisp-link)
		       ("file" :complete org-file-complete-link)
		       ("ftp" :follow
			(lambda (path) (browse-url (concat "ftp:" path))))
		       ("help" :follow org--open-help-link)
		       ("http" :follow
			(lambda (path) (browse-url (concat "http:" path))))
		       ("https" :follow
			(lambda (path) (browse-url (concat "https:" path))))
		       ("mailto" :follow
			(lambda (path) (browse-url (concat "mailto:" path))))
		       ("news" :follow
			(lambda (path) (browse-url (concat "news:" path))))
		       ("shell" :follow org--open-shell-link))
 org-clock-out-hook '(org-clock-remove-empty-clock-drawer)
 )

Re: Bug: clocktable thinks Oct 29 is Oct 28 [9.1.14 (release_9.1.14-1012-g643481 @ /home/unhammer/src/org-mode/lisp/)]

[Kevin Brubeck Unhammer]

[-- Attachment #1: Type: text/plain, Size: 330 bytes --]

Kevin Brubeck Unhammer <unhammer@fsfe.org> čálii:

[...]

> I see the same on org-mode
> 9.1.9 (bundled with Emacs 26.1) or 9.1.12 and 9.1.14 from melpa-stable,
> gnu elpa.
>
> I do *not* see this in 8.2.10 bundled with Emacs 25.3.2.

git bisect tells me
8c7a14a850e7b988fe4c9d0eea9207793e2af368
introduced this.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[PATCH] clocktable thinks Oct 29 is Oct 28 [9.1.14 (release_9.1.14-1012-g643481 @ /home/unhammer/src/org-mode/lisp/)]

[Kevin Brubeck Unhammer]

[-- Attachment #1.1: Type: text/plain, Size: 135 bytes --]

The attached patch solves this, as far as I can tell.

I have signed the FSF copyright papers.

best regards,
Kevin Brubeck Unhammer



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-org-clock-Handle-DST-in-clocktable.patch --]
[-- Type: text/x-diff, Size: 4128 bytes --]

From e818cd5d1c651a516a0e791a10d91fe1e310cde9 Mon Sep 17 00:00:00 2001
From: Kevin Brubeck Unhammer <unhammer@fsfe.org>
Date: Thu, 1 Nov 2018 13:59:09 +0100
Subject: [PATCH] org-clock: Handle DST in clocktable

* lisp/org-clock.el (org-clocktable-steps): new fn
  org-clocktable-increment-day to add/subtract days that are not
  exactly 24 hours
* testing/lisp/test-org-clock.el (test-org-clock/clocktable/step): Add
  test for DST.
---
 lisp/org-clock.el              | 27 +++++++++++++++++++--------
 testing/lisp/test-org-clock.el | 21 ++++++++++++++++++++-
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 420fdadfe..6292a259f 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -2681,6 +2681,15 @@ LEVEL is an integer.  Indent by two spaces per level above 1."
   (if (= level 1) ""
     (concat "\\_" (make-string (* 2 (1- level)) ?\s))))
 
+(defun org-clocktable-increment-day (ts &optional n)
+  "Increment day in TS by N (defaulting to 1).
+The TS argument has the same type as the return values of
+`float-time' or `current-time'."
+  (let ((tsd (decode-time ts)))
+    (cl-incf (nth 3 tsd) (or n 1))
+    (setf (nth 8 tsd) nil) ; no time zone: increasing day skips one whole day
+    (apply 'encode-time tsd)))
+
 (defun org-clocktable-steps (params)
   "Step through the range to make a number of clock tables."
   (let* ((ts (plist-get params :tstart))
@@ -2688,7 +2697,6 @@ LEVEL is an integer.  Indent by two spaces per level above 1."
 	 (ws (plist-get params :wstart))
 	 (ms (plist-get params :mstart))
 	 (step0 (plist-get params :step))
-	 (step (cdr (assq step0 '((day . 86400) (week . 604800)))))
 	 (stepskip0 (plist-get params :stepskip0))
 	 (block (plist-get params :block))
 	 cc tsb)
@@ -2715,16 +2723,19 @@ LEVEL is an integer.  Indent by two spaces per level above 1."
 	  (if (eq step0 'week)
 	      (let ((dow (nth 6 (decode-time (seconds-to-time ts)))))
 		(if (<= dow ws) ts
-		  (- ts (* 86400 (- dow ws)))))
+		  (org-clocktable-increment-day ts ; decrement
+						(- ws dow))))
 	    ts))
     (while (< tsb te)
       (unless (bolp) (insert "\n"))
-      (let ((start-time (seconds-to-time (max tsb ts))))
-	(cl-incf tsb (let ((dow (nth 6 (decode-time (seconds-to-time tsb)))))
-		       (if (or (eq step0 'day)
-			       (= dow ws))
-			   step
-			 (* 86400 (- ws dow)))))
+      (let* ((start-time (seconds-to-time (max tsb ts)))
+	     (dow (nth 6 (decode-time (seconds-to-time tsb))))
+	     (days-to-skip (cond ((eq step0 'day) 1)
+				 ;; else 'week:
+				 ((= dow ws) 7)
+				 (t (- ws dow)))))
+	(setq tsb (time-to-seconds (org-clocktable-increment-day tsb
+								 days-to-skip)))
 	(insert "\n"
 		(if (eq step0 'day) "Daily report: "
 		  "Weekly report starting on: ")
diff --git a/testing/lisp/test-org-clock.el b/testing/lisp/test-org-clock.el
index abb3637bd..ddc135669 100644
--- a/testing/lisp/test-org-clock.el
+++ b/testing/lisp/test-org-clock.el
@@ -977,7 +977,26 @@ CLOCK: [2017-12-27 Wed 08:00]--[2017-12-27 Wed 16:00] =>  8:00"
 	    (let ((system-time-locale "en_US"))
 	      (test-org-clock-clocktable-contents
 		  (concat ":step day :tstart \"<2017-12-25 Mon>\" "
-			  ":tend \"<2017-12-27 Wed 23:59>\" :stepskip0 t")))))))
+			  ":tend \"<2017-12-27 Wed 23:59>\" :stepskip0 t"))))))
+  ;; Regression test: Respect DST
+  (should
+   (equal "
+Daily report: [2018-10-29 Mon]
+| Headline     | Time   |
+|--------------+--------|
+| *Total time* | *8:00* |
+|--------------+--------|
+| Foo          | 8:00   |
+"
+	  (org-test-with-temp-text
+	      "* Foo
+CLOCK: [2018-10-29 Mon 08:00]--[2018-10-29 Mon 16:00] =>  8:00"
+	    (let ((system-time-locale "en_US"))
+	      (test-org-clock-clocktable-contents
+		  (concat ":step day "
+			  ":stepskip0 t "
+			  ":tstart \"2018-10-01\" "
+			  ":tend \"2018-11-01\"")))))))
 
 (ert-deftest test-org-clock/clocktable/extend-today-until ()
   "Test assignment of clock time to days in presence of \"org-extend-today-until\"."
-- 
2.17.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] clocktable thinks Oct 29 is Oct 28 [9.1.14 (release_9.1.14-1012-g643481 @ /home/unhammer/src/org-mode/lisp/)]

[Nicolas Goaziou]

Hello,

Kevin Brubeck Unhammer <unhammer@fsfe.org> writes:

> The attached patch solves this, as far as I can tell.

I rewrote `org-clocktable-steps' in "next" branch, and the bug doesn't
seem to appear there. Therefore, I applied your patch in master, but
dropped it in next, except for the test.

Thank you.

Regards,

-- 
Nicolas Goaziou