bug-gnu-emacs
[Top][All Lists]
Advanced

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

bug#52209: 28.0.60; [PATCH] date-to-time fails on pure dates


From: Bob Rogers
Subject: bug#52209: 28.0.60; [PATCH] date-to-time fails on pure dates
Date: Wed, 29 Dec 2021 17:01:01 -0500

   From: Paul Eggert <eggert@cs.ucla.edu>
   Date: Wed, 29 Dec 2021 11:29:44 -0800

   On 12/29/21 07:19, Lars Ingebrigtsen wrote:
   > Bob Rogers <rogers-emacs@rgrjr.homedns.org> writes:
   > 
   >>     I am currently working on broadening what the parser will accept,
   >> though I think it is close to a usable state.
   > 
   > Makes sense to me.  Perhaps Paul has some comments; added to the CCs.

   My first comment is "be careful what you're getting into" :-).  I'm
   trying to retire from date-parsing as its users are never happy and
   rightly so.

No worries; I have spent more of my career than I like to think about
dealing with date/time issues, so I know what a can of worms I am in the
process of opening.

   But here goes.  I took a quick look at
   <https://bugs.gnu.org/52209#58> and have a few comments.

They are greatly appreciated; thank you.

   * Calling it parse-date is a bit confusing, as it parses both dates and 
   times. I suggest calling it parse-timestamp or parse-date-time instead. 
   (I know the existing package is called parse-time but we can't fix that.)

Lars originally suggested parse-time, but there's already a
parse-time-tests.el, so I switched to parse-date so I could use
parse-date-tests.el to correspond.  So the namespace is already crowded.
But I would be OK with either of those alternatives.  Since it will
actually give you either date or time, or both, parse-date-time might
make more sense.

   * If the package is called X, the error should be called X-error. 
   Currently the package is called parse-date and the error is called 
   date-parse-error, which is confusing.

My thought was that for the "parse-date" function, the verb should come
before the noun, and in "date-parse-error", the "date" is an adjective
further modifying "parse error."  But I think I'm way fussier about
these things than anybody I know, so your point is well taken.

   * The patch should also modify the comment at the start of
   parse-time.el to indicate parse-date-time as another possibility.

I took that as a late-stage task, something to do alongside updating
Elisp documentation.  (Which I haven't even begun to look at.)

   * I suggest preferring the symbol 'rfc-email' for parsing
   email-related dates, for consistency with the --rfc-email option of
   GNU 'date'. This should use the current RFC (5322 now, perhaps
   updated later). 

I started with RFC822 and RFC2822 because I had copies of these lying
around; you're right that I should have looked for more recent
standards.  And using rfc-email as a synonym for the latest version is a
good idea.

   I suppose you could also advertise 'rfc-822' for strict RFC 822
   conformance, and similarly 'rfc2822' for strict 2822 conformance, but
   I expect these alternatives would be less useful in practice.

Anyone parsing email headers would need their date parser to support
RFC822 in case they encountered very old emails, but (since later
standards are backward-compatible) it's not clear what supporting
intermediate standards would buy.

   > +   nil => like us-date with two-digit years disallowed.

   This doesn't sound like a good default. For example, it completely 
   mishandles dates in Brazil, which use DD/MM/YYYY format.

I subsequently added a euro-date format for DD/MM (with various lengths
of years).

   > +Anything else is treated as iso-8601 if it looks similar, else
   > +us-date with two-digit years disallowed.

   This might be a better default (for nil), but it should have an explicit 
   name other than nil.

Suggestions?

   > +   * For all formats except iso-8601, parsing is case-insensitive.

   It's pretty common for ISO 8601 parsers to be case-insensitive. For 
   example, Java's OffsetDateTime.parse(CharSequence) allow both lower and 
   upper case T and Z. Perhaps some people need strict ISO 8601 parsers, 
   but I imagine a more-generous parser would be more useful. So you could 
   have iso-8601 and iso-8601-strict; or you could have a strictness arg; 
   or something like that.

Actually, I am handing those off to the existing iso8601-parse code,
which doesn't like lowercase T (at least).

   > +   * Commas and whitespace are ignored.

   This is quite wrong for some formats, if you want to be strict.  And
   even if not, commas are part of ISO 8601 format and can't be ignored
   if I understand what you mean by "ignored".

I see I need to clarify the docstring to state that these other bulleted
comments also do not apply to ISO-8601 dates.

   > +   * Two digit years, when allowed, are in the 1900's when
   > +between 50 and 99 inclusive and in the 2000's when between 0 and
   > +49 inclusive.

   This disagrees with the POSIX standard for 'date' (supported by GNU 
   'date'), which says 69-99 are treated as 1969-1999 and 00-68 are treated 
   as 2000-2068. I suggest going with the POSIX heuristic if you're going 
   to use a fixed heuristic for dates at all.

I was just following the existing parse-time-string heuristic.  So which
do you think should rule:  POSIX or parse-time-string compatibility?

   Better might be to have an optional argument of context specifying the 
   default time for incomplete timestamps. You can use that the context to 
   fill in more-significant parts that are missing. E.g., if the year is 
   missing, you take it from the context; if the century is missing, you 
   take that from the context. The default context would be empty, i.e., 
   missing years or centuries would be an error.

Again, I'm just doing what parse-time-string is doing, namely leaving
everything that is not specified nil, and letting the caller decide how
to apply defaults.  The only exception is when time is specified without
seconds; in that case, the seconds are set to zero (which is also
compatible with parse-time-string).

   And even defaulting from context is not straightforward:  If given a
date without a year that is not today, should that be in the future or
in the past?  There's a can of worms I don't need to touch.  ;-}

   For more formats that need parsing, see:

   https://en.wikipedia.org/wiki/Date_format_by_country
   https://metacpan.org/search?q=datetime%3A%3Aformat

   You don't need to support them all now, but you should take a look at 
   what's out there and make sure the API can be extended to handle them.

Excellent; thank you!  I have been looking at date parsing module
documentation but so far the ones I've seen have not been very clear
about what they actually accept.

                                        -- Bob Rogers
                                           http://www.rgrjr.com/





reply via email to

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