From: Max Nikulin <firstname.lastname@example.org> To: email@example.com Subject: Re: [DRAFT][PATCH v2] org-encode-time compatibility and convenience helper Date: Sun, 24 Apr 2022 18:34:40 +0700 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <87mtgcmckj.fsf@localhost> [-- Attachment #1: Type: text/plain, Size: 2764 bytes --] 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. [-- Attachment #2: 0001-org-macs.el-Introduce-a-helper-for-encode-time.patch --] [-- Type: text/x-patch, Size: 4647 bytes --] From 8fdcca3f3b0cf3e890149eb79ea6c7970b7d2cfa Mon Sep 17 00:00:00 2001 From: Max Nikulin <email@example.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)) +\f +;;; 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))))) + \f ;;; Comments @@ -179,6 +193,48 @@ \f ;;; 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
next prev parent reply other threads:[~2022-04-24 11:35 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-11 15:22 [DRAFT][PATCH] " Max Nikulin 2022-04-11 17:43 ` Paul Eggert 2022-04-23 8:25 ` Ihor Radchenko 2022-04-23 19:37 ` Paul Eggert 2022-04-24 3:35 ` Ihor Radchenko 2022-04-24 11:34 ` Max Nikulin [this message] 2022-04-26 9:07 ` [DRAFT][PATCH v2] " Ihor Radchenko 2022-05-03 12:14 ` [PATCH v3] " Max Nikulin 2022-05-04 9:56 ` Ihor Radchenko 2022-05-04 16:49 ` Max Nikulin 2022-05-05 15:22 ` [PATCH v4] " Max Nikulin 2022-05-07 4:46 ` Ihor Radchenko 2022-07-17 8:50 ` Ihor Radchenko 2022-05-10 14:31 ` Max Nikulin 2022-05-11 13:20 ` Ihor Radchenko 2022-05-13 15:14 ` Max Nikulin 2022-05-14 6:06 ` Ihor Radchenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: https://www.orgmode.org/ * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org' \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [DRAFT][PATCH v2] org-encode-time compatibility and convenience helper' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this 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).