On 23/04/2022 15:25, Ihor Radchenko wrote:
Max Nikulin writes:
My patch requires more changes since the macro is just defined but not
actually used. It does not fix the problem with "no DST" flag returned
by some function in Org. I can prepare next patches, but I think it
should be decided at first which approach should be accepted by Org Mode:
- org-encode-time accepting both list or separate arguments
- mix of `encode-time' with multiple arguments and org-encode-time-1 for
lists.
This whole timezone staff is complex. I got lost in the emacs devel
discussion half-way through. From point of view of API, I would prefer a
single function with docstring explaining the necessary caveats.
To have namely a single function that accepts both a list or multiple
arguments, it is necessary to introduce a new name to Emacs.
`encode-time' has a subtle difference in behavior depending on the
calling style and fixing this issue may break some code. That is why I
decided to offer a macro.
I have not figured out which kind of high-level API I would like to have
instead of `encode-time', and I believe, it is acceptable to rely on
default normalization and ambiguity resolution as the status quo.
Problems may happen during 2 days of 365 and people usually expect some
glitches for these days. There are too many caveats to explain them in
docstring.
+ (if (cdr time)
+ `(encode-time ,@time)
+ `(apply #'encode-time ,(car time))))
Do I miss something or can you instead just do `(encode-time ,@time)
without if?
I changed the code to use ,@time in both cases. Previous time likely I
forgot to re-evaluate some definition, got some unexpected error, and
decided to use `car' for a while.
`if' can not be dropped however. In the case of
(org-encode-time '(0 0 23 30 04 2022 nil nil nil))
`(encode-time ,@time) will be expanded to
(encode-time (list 0 0 23 30 04 2022 nil nil nil))
due to (&rest time) argument. Single list argument is unsupported in
Emacs-26. So `apply' cat not be avoided.
+ (should (string-equal
+ "2022-03-24 23:30:01"
+ (format-time-string
+ "%F %T"
+ (org-encode-time '(01 30 23 24 03 2022 nil -1 nil)))))
...
These tests will be executed using system value of TZ. I am not sure if
tests are not going to break, say, in southern hemisphere or at some
other bizzare values of TZ.
You are right, it is safer to run this test with explicitly chosen TZ
value. I do not think there is a point in speculation whether the test
fails in some currently existing time zone.
I am attaching an updated version of the draft. I have added a macro for
testing of particular time zones.From 8fdcca3f3b0cf3e890149eb79ea6c7970b7d2cfa Mon Sep 17 00:00:00 2001
From: Max Nikulin <maniku...@gmail.com>
Date: Fri, 8 Apr 2022 23:10:50 +0700
Subject: [PATCH] org-macs.el: Introduce a helper for `encode-time'
* lisp/org-macs.el (org-encode-time): New compatibility and convenience
helper macro to allow a list for time components or separate arguments
independently of Emacs version.
* testing/lisp/test-org.el (org-test-with-timezone): New macro to ensure
that some code is executed with or without daylight saving time or other
time shifts by setting certain TZ environment value.
* testing/lisp/test-org.el (test-org/org-encode-time): Tests for various
ways to call `org-encode-time'.
Ensure recommended way to call `encode-time' for Emacs-27 and newer with
hope to avoid bugs due to attempts to modernize the code similar to
bug#54731.
---
lisp/org-macs.el | 20 ++++++++++++++
testing/lisp/test-org.el | 56 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+)
diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index a09115e7c..85dd20028 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -1225,6 +1225,26 @@ nil, just return 0."
(b (org-2ft b)))
(and (> a 0) (> b 0) (\= a b))))
+(if (version< emacs-version "27.1")
+ (defmacro org-encode-time (&rest time)
+ (if (cdr time)
+ `(encode-time ,@time)
+ `(apply #'encode-time ,@time)))
+ (defmacro org-encode-time (&rest time)
+ (pcase (length time)
+ (1 `(encode-time ,(car time)))
+ (6 `(encode-time (list ,@time nil -1 nil)))
+ (9 `(encode-time (list ,@time)))
+ (_ (error "`org-encode-time' may be called with 1, 6, or 9 arguments but %d given"
+ (length time))))))
+(put 'org-encode-time 'function-documentation
+ "Compatibility and convenience helper for `encode-time'.
+May be called with 9 components list (SECONDS ... YEAR IGNORED DST ZONE)
+as the recommended way since Emacs-27 or with 6 or 9 separate arguments
+similar to the only possible variant for Emacs-26 and earlier.
+Warning: use -1 for DST that means guess actual value, nil means no
+daylight saving time and may be wrong at particular time.")
+
(defun org-parse-time-string (s &optional nodefault)
"Parse Org time string S.
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 6aecc3af8..aa3f9a3f6 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -24,6 +24,20 @@
(eval-and-compile (require 'cl-lib))
+
+;;; Helpers
+
+(defmacro org-test-with-timezone (tz &rest body)
+ "Evaluate BODY with TZ environment temporary set to the passed value."
+ (declare (indent 1))
+ (org-with-gensyms (tz-saved)
+ `(let ((,tz-saved (getenv "TZ")))
+ (unwind-protect
+ (progn
+ (setenv "TZ" ,tz)
+ ,@body)
+ (setenv "TZ" ,tz-saved)))))
+
;;; Comments
@@ -179,6 +193,48 @@
;;; Date and time analysis
+(ert-deftest test-org/org-encode-time ()
+ "Test various ways to call `org-encode-time'"
+ (org-test-with-timezone "UTC"
+ ;; list as the sole argument
+ (should (string-equal
+ "2022-03-24 23:30:01"
+ (format-time-string
+ "%F %T"
+ (org-encode-time '(1 30 23 24 3 2022 nil -1 nil)))))
+ ;; SECOND...YEAR
+ (should (string-equal
+ "2022-03-24 23:30:02"
+ (format-time-string
+ "%F %T"
+ (org-encode-time 2 30 23 24 3 2022))))
+ ;; SECOND...YEAR IGNORED DST ZONE
+ (should (string-equal
+ "2022-03-24 23:30:03"
+ (format-time-string
+ "%F %T"
+ (org-encode-time 3 30 23 24 3 2022 nil -1 nil))))
+ ;; function call
+ (should (string-equal
+ "2022-03-24 23:30:04"
+ (format-time-string
+ "%F %T"
+ (org-encode-time (apply #'list 4 30 23 '(24 3 2022 nil -1 nil))))))
+ ;; wrong number of arguments
+ (if (not (version< emacs-version "27.1"))
+ (should-error (string-equal
+ "2022-03-24 23:30:05"
+ (format-time-string
+ "%F %T"
+ (org-encode-time 5 30 23 24 3 2022 nil))))))
+ ;; daylight saving time
+ (org-test-with-timezone "Europe/Madrid"
+ (should (string-equal
+ "2022-03-31 23:30:06"
+ (format-time-string
+ "%F %T"
+ (org-encode-time 6 30 23 31 3 2022))))))
+
(ert-deftest test-org/org-read-date ()
"Test `org-read-date' specifications."
;; Parse ISO date with abbreviated year and month.
--
2.25.1
