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.