unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Maxim Nikulin <m.a.nikulin@gmail.com>
To: Paul Eggert <eggert@cs.ucla.edu>, Lars Ingebrigtsen <larsi@gnus.org>
Cc: Eli Zaretskii <eliz@gnu.org>, 55635-done@debbugs.gnu.org
Subject: bug#55635: `make-decoded-time' incorrectly sets DST to nil, it should be -1 (guess)
Date: Tue, 31 May 2022 19:25:25 +0700	[thread overview]
Message-ID: <c7e47697-36ba-2be3-e436-dd8b4b79a447@gmail.com> (raw)
In-Reply-To: <6ce0d14a-017c-2b28-d924-fb461396c547@cs.ucla.edu>

On 30/05/2022 05:04, Paul Eggert wrote:
> diff --git a/lisp/simple.el b/lisp/simple.el
> index a254ee2251..d6b7045432 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -10519,6 +10519,14 @@ capitalize-dwim
>  the number of seconds east of Greenwich.")
>    )
>  
> +;; Document that decoded-time-dst is problematic on 6-element lists.
> +;; It should return -1 indicating unknown DST, but currently returns
> +;; nil indicating standard time.
> +(put 'decoded-time-dst 'function-documentation
> +     (append (get 'decoded-time-dst 'function-documentation)
> +             "As a special case, `decoded-time-dst' returns an unspecified
> +value when given a list too short to have a dst element."))
> +

Paul, thank you for your efforts to fix the issues.

I have never used `cl-defstruct' before (and frankly speaking I am 
rather confused that the `decoded-time' struct and its constructor 
`make-decoded-time' are defined in different files and even directories 
are not the same), so my question may be naïve. Why did not you just add 
this new sentence to the :documentation property of the DST slot a bit 
above?

By the way, after updating of `make-decoded-time', default value for DST 
should be updated in `cl-defstruct' as well, otherwise
     (describe-symbol 'decoded-time)
reports that the default is nil.

It may be reasonable to cross-link `decoded-time' and 
`make-decoded-time' in docstrings.

Concerning timestamp vs. interval, first of all, I do not request 
immediate changes. I raised the question to make developers aware that 
the problem exist and it should be taken into account during further 
modifications or implementation of new features.

Lars Ingebrigtsen, Sun, 29 May 2022 15:10:19 +0200.
> If somebody wants to do
> interval calculations and passes in a DST to make-decoded-time, that's a
> classic "well, don't do that" situation.

DST slot with -1 (default) value is rather confusing for intervals but 
it safer for timestamps. Certainly it possible to do anything with bytes 
in memory, but direction of programming languages and libraries 
development is to allow users to clearly express intentions code and to 
add some measures that prevents bugs. That is why I mentioned tagged 
structure for interval type to distinguish it from timestamps.

Eli Zaretskii, Sat, 28 May 2022 19:53:47 +0300. >> From: Maxim Nikulin 
Date: Sat, 28 May 2022 23:31:43 +0700
>>
>> I think, it is confusing that `make-decoded-time' is used to create 
>> timestamps *and* time intervals. They are different types, for example 
>> sum of intervals is meaningful (despite may be ambiguous) while there is 
>> no point to add timestamps.
> 
> But this situation already exists with time units anyway.  You can add
> an hour to some other time, but there's also a valid time stamp that
> expresses 1 hour past the epoch UTC, and their values are exactly
> identical.

Certainly timestamps are actually intervals with various reference 
points. Encoded time is counted from 1970, decoded time from 0 a.d., so 
trying to add the same timestamps you will get result depending on their 
representation. Decoded time in some cases is more convenient since 1 
day may be not the same as 24 hours, not to mention varying duration of 
1 month.

The problem is that `decoded-time' time have field that are alien for 
intervals (timezone). Using the same constructor for both types makes 
code more obscure, it is impossible to enforce particular type of 
function argument to catch a programming error.

It is possible to use the same type for timestamps and intervals 
further, I am trying to dispute that it is the best design choice.





  reply	other threads:[~2022-05-31 12:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-25 14:46 bug#55635: `make-decoded-time' incorrectly sets DST to nil, it should be -1 (guess) Maxim Nikulin
2022-05-26 12:13 ` Lars Ingebrigtsen
2022-05-27  2:11   ` Paul Eggert
2022-05-27 10:40     ` Lars Ingebrigtsen
2022-05-27 19:26       ` Paul Eggert
2022-05-28 10:41         ` Lars Ingebrigtsen
2022-05-28 16:31           ` Maxim Nikulin
2022-05-28 16:53             ` Eli Zaretskii
2022-05-28 17:25               ` Paul Eggert
2022-05-29 13:10                 ` Lars Ingebrigtsen
2022-05-29 22:04                   ` Paul Eggert
2022-05-31 12:25                     ` Maxim Nikulin [this message]
2022-06-13 21:30                       ` Paul Eggert
2022-06-14 15:57                         ` Maxim Nikulin

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.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c7e47697-36ba-2be3-e436-dd8b4b79a447@gmail.com \
    --to=m.a.nikulin@gmail.com \
    --cc=55635-done@debbugs.gnu.org \
    --cc=eggert@cs.ucla.edu \
    --cc=eliz@gnu.org \
    --cc=larsi@gnus.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.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).