On 26/04/2022 16:07, Ihor Radchenko wrote:
LGTM. I think you can go ahead and continue with the followup patch
making use of the new macro.
The attached patch set is assumed to be complete.
I have no chance to thoroughly test it. Existing unit tests pass for
Emacs-26 and Emacs-27. Nothing has changed for Emacs-25, as for the
"main" branch one test fails. I have not tried Emacs-28 or the current
git version.
In comparison to the previous patch version I have expanded the
docstring and added a bit more tests. I have tried to support recently
committed to Emacs 6-elements list for `encode-time', but I do not like
the following compile-time warning:
In toplevel form:
org-macs.el:1397:23:Warning: encode-time called with 1 argument, but requires
6+
I had to carve the patch originally posted by Paul Eggert for the Emacs
repository to adapt it for the Org repository and to drop
org-encode-time-1 chunks.
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54764#10
The largest patch replaces all calls of `encode-time' to `org-encode-time'
Goals of `org-encode-time':
- it should work at the compile or load time to minimize runtime
performance impact,
- since both ways to call `encode-time' are necessary (in a half of
cases a list returned by `decode-time' is available, in other cases
timestamps are assembled from scratch, none is preferred), it should be
convenient in both cases,
- it should allow Org to work even if support of multiple `encode-time'
arguments will be removed from Emacs,
- it should allow to avoid a pitfall with Emacs-27+ `encode-time': DST
argument is taken into account when the function called with single list
argument but it is ignored when multiple arguments is passed.
`org-encode-time' respects DST value unless it is running in Emacs-26 or
earlier. I have not added special code for old versions considering it
as graceful degradation.
Finally, let me remind that these changes are result of the following
discussions:
Ignacio Casso to emacs-orgmode. [BUG] org-agenda thinks timestamps after
23:00 correspond to the next day. Tue, 29 Mar 2022 15:09:10 +0200.
https://list.orgmode.org/paxpr06mb7760238f410cbe3203f78ee0c6...@paxpr06mb7760.eurprd06.prod.outlook.com
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54731
https://list.orgmode.org/8b85c879-4f9b-eac3-e700-f176cc588...@gmail.com
#54731 - Please, revert part of dd0727e1ec1 related to Org mode
(`encode-time') - GNU bug report logs
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54764
https://list.orgmode.org/5ed963b2-3fa8-48d8-627e-bc0571d15...@gmail.com
#54764 - encode-time: make DST and TIMEZONE fields of the list argument
optional ones - GNU bug report logsFrom fbddfa8cfeecc8465117eacf101a0880cfc13775 Mon Sep 17 00:00:00 2001
From: Paul Eggert
Date: Sat, 9 Apr 2022 00:17:09 -0700
Subject: [PATCH 1/6] Use unknown DST instead of standard time in timestamps
* lisp/ol.el (org-store-link): Prefer plain (encode-time ...)
to (apply 'encode-time ...), for speed.
* lisp/org-macs.el (org-parse-time-string): Return unknown DST,
not standard time.
* lisp/org.el (org-read-date-analyze): Return a timestamp with a DST
flag of -1 (unknown) rather than nil (standard time).
Max Nikulin:
A larger patch "Improve Org usage of timestamps" was suggested in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54764#10
Changes selected for this patch normalizes timestamp format:
if it is a list than it should contain 9 elements to be compatible
with Emacs-27 and Emacs-28 `encode-time' single argument, nil should not
be used for DST field since it means standard time while actual value
is unknown and should be guessed.
Ignacio Casso reported a problem with agenda
https://list.orgmode.org/paxpr06mb7760238f410cbe3203f78ee0c6...@paxpr06mb7760.eurprd06.prod.outlook.com
due to Emacs commit dd0727e1ec1 changing Org code. It was mostly reverted
by 8ef37913d3 (bug#54731). Code in the Org repository did not have
the bug, but it safer to add protection against similar refactoring.
---
lisp/ol.el | 4 +---
lisp/org-macs.el | 2 +-
lisp/org.el | 2 +-
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/lisp/ol.el b/lisp/ol.el
index d4bd0e40c..0de9c921b 100644
--- a/lisp/ol.el
+++ b/lisp/ol.el
@@ -1618,9 +1618,7 @@ non-nil."
(setq link
(format-time-string
(car org-time-stamp-formats)
- (apply 'encode-time
- (list 0 0 0 (nth 1 cd) (nth 0 cd) (nth 2 cd)
- nil nil nil
+ (encode-time 0 0 0 (nth 1 cd) (nth 0 cd) (nth 2 cd
(org-link-store-props :type "calendar" :date cd)))
((eq major-mode 'w3-mode)
diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index 8535bf2cd..241155064 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -1412,7 +1412,7 @@ This should be a lot faster than the `parse-time-string'."
(string-to-number (match-string 4 s))
(string-to-number (match-string 3 s))
(string-to-number (match-string 2 s))
- nil nil nil))
+ nil -1 nil))
(defun org-matcher-time
