tomaszkam / date

A date and time library based on the C++11/14/17 <chrono> header
Other
0 stars 0 forks source link

[LWG3245] Unnecessary restriction on '%p' parse command #28

Closed tomaszkam closed 5 years ago

tomaszkam commented 5 years ago

Original comment:

When parsing %p no longer come after %I.

tomaszkam commented 5 years ago

Need help with understanding format related issues.

tomaszkam commented 5 years ago

Ok, this boils down to the entry for '%p' from http://eel.is/c++draft/time.parse#tab:time.parse.spec:

The locale's equivalent of the AM/PM designations associated with a 12-hour clock. The command %I must precede %p in the format string.

Per my reading of implementation of from_stream this requirement is no longer present, and '%p' can occur in any place in the format string.

@HowardHinnant This seems to be an extension, that can be done post C++20, as will only make implementation less strict. Is that correct?

HowardHinnant commented 5 years ago

Actually this is an obsolete requirement and it should be struck. The first time I implemented this I didn't know how to do it without this requirement. I've since reimplemented it without needing this.

I.e. this works:

#include "date/date.h"
#include <iostream>
#include <sstream>

int
main()
{
    using namespace date;
    using namespace std;
    using namespace std::chrono;
    istringstream in{"2019-06-16 pm2:34"};
    in.exceptions(ios::failbit);
    sys_seconds tp;
    in >> parse("%F %p%I:%M", tp);
    cout << tp << '\n';
}
tomaszkam commented 5 years ago

I know, but at this point that I think it would be considered design change (new feature). I cannot think of way of justifying this as the wording issue. Any suggestion?

HowardHinnant commented 5 years ago

I would just say that it is needed to support migration from time_get which does not have this restriction. Now is the time to get it right, before it is standardized. Making minor improvements to the design is in scope.

tomaszkam commented 5 years ago

Discussion: The current specification for the '%p' in the "[tab:time.parse.spec] Meaning of parse flags" places a restriction of it's placement with regards to the '%I' command:

The locale's equivalent of the AM/PM designations associated with a 12-hour clock. The command %I must precede %p in the format string.

This restriction makes the migration to new API more difficult, as it is not present for the POSIX strptime (https://pubs.opengroup.org/onlinepubs/009695399/functions/strptime.html) nor in the example implementation of the library (see https://wandbox.org/permlink/nL6dxSSTVzd9zhZ0). Per Howard's comment:

Actually this is an obsolete requirement and it should be struck. The first time I implemented this I didn't know how to do it without this requirement. I've since reimplemented it without needing this.

Proposed wording: Change the %p entry in the "[tab:time.parse.spec] Meaning of parse flags" as follows: The locale's equivalent of the AM/PM designations associated with a 12-hour clock. The command %I must precede %p in the format string.