emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: bug#54764: encode-time: make DST and TIMEZONE fields of the list arg


From: Paul Eggert
Subject: Re: bug#54764: encode-time: make DST and TIMEZONE fields of the list argument optional ones
Date: Sat, 9 Apr 2022 00:52:57 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0

On 4/7/22 05:37, Max Nikulin wrote:

(encode-time '(0 30 20 07 04 2022 nil nil nil)) ; wrong!

Yes, and I see a couple of places (org-parse-time-string, org-read-date-analyze) where Org mode returns the wrong decoded timestamps, ending in (nil nil nil) even though the DST flag is unknown so they should end in (nil -1 nil). Please see attached proposed patch which fixes this (also see below).


Since Emacs-27 time fields as separated arguments are considered obsolete for calls of `encode-time'.

Obsolescent, not obsolete. The old form still works and it's not going away any time soon. If the efficiency concerns you mention are significant, we should keep the old form indefinitely.


t is inconvenient to add 3 extra mandatory components at the each place.

Then let's keep using the obsolescent calling convention for places where that's convenient. Perhaps we should change the documentation to say "older" instead of "obsolescent".


From my point of view it is better to change implementation of `encode-time' so that it may accept 6-component list SECOND...YEAR. It should not add noticeable performance penalty but makes the function more convenient in use.

Unfortunately it makes the function more convenient to use incorrectly. This was part of the motivation for the API change. The obsolescent calling convention has no way to deal with ambiguous timestamps like 2022-11-06 01:30 when TZ="America/Los_Angeles". Org mode surely has bugs in this area, although I don't have time to scout them out.


Daylight saving time field matters only as a list component and ignored as a separate argument (by the way, it should be stressed in the docstring).

Do you have a wording suggestion? (The doc string already covers the topic concisely; however, conciseness is not always a virtue. :-)

The reason -1 is the default in the obsolete API is backward compatibility. If we could have designed the API from scratch it would have been different.


In the Org code it is unsure which way to call `encode-time' is more convenient. In a half of the cases a list is obtained from another function, but another half is timestamp built from computed components. Unless the inconsistency with DST I would say that both ways to call the function should be supported.

Yes, that's the idea.


So my proposal is to not force Org mode to use new calling convention for `encode-time' till DST and ZONE list components will became optional ones in a released Emacs version.

This would delay things for ten years or so, no? We'd have to wait until Org mode supported only Emacs 29 and later.

Instead, I suggest that we stick with what we have when that's cleaner. That is, Org mode can use the obsolescent encode-time API when it's cleaner to do that.

It would be helpful for Org mode to use the new encode-time form in some cases, when the new form is cleaner. It's easy to support the new form efficiently even in older Emacs, using a compatibility shim. This is also in the attached proposed patch.

This patch has a few other minor cleanups in the area.

I haven't installed the patch, or tested it other than via 'make check'.


PS. Org mode usually uses encode-time for calendrical calculations. This is dicey, as some days don't exist (for example, December 30, 2011 does not exist if TZ="Pacific/Apia", because Samoa moved across the International Date Line that day). And it's also dicey when Org mode uses 00:00:00 (midnight at the start of the day) as a timestamp representing the entire day, as it's all too common for midnight to not exist (or to be duplicated) due to a DST transition. Generally speaking, when Org mode is doing calendrical calculations it should use calendrical functions rather than encode-time+decode-time, which are best used for time calculations not calendar calculations. (I realize that fixing this in Org would be nontrivial; perhaps I should file this "PS" as an Org bug report for whoever has time to fix it....)

Attachment: 0001-Improve-Org-usage-of-timestamps.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]