Fixed #74298 - IntlDateFormatter->format() doesn't return microseconds/fractions#2432
Fixed #74298 - IntlDateFormatter->format() doesn't return microseconds/fractions#2432andrewnester wants to merge 1 commit intophp:PHP-7.1from
Conversation
|
Hmpf, luckily MariaDB / MySQL and PostgreSQL do 6 digits precision (I am using datetime(6) as a revisioning candidate/unique key part). So AFAIU PHP's datetime objects do (unless buggy) the same; however ICU parsing/formatting - by spec - cuts at 3 digits, right? THanks for caring @ PR. |
|
@inoas yes, 3 digits precision relates only to ICU |
|
|
||
| *millis = U_MILLIS_PER_SECOND * (double)Z_LVAL(retval); | ||
| datetime = Z_PHPDATE_P(z); | ||
| *millis = U_MILLIS_PER_SECOND * ((double)Z_LVAL(retval) + datetime->time->f); |
There was a problem hiding this comment.
The fact that the timestamp is retrieved through a getTimestamp() method while the fraction is obtained directly from the structure feels like a breach of abstraction. On the other hand, it does not seem like DateTime has a direct way of getting the fraction (or timestamp with fraction).
@derickr Do you have any thoughts on this?
There was a problem hiding this comment.
@nikic @derickr I was thinking about this too, but at the same time I took this idea from here https://github.com/andrewnester/php-src/blob/1bf8cfbb9a136ee64303206446ec7e2e97f7ec4c/ext/intl/common/common_date.cpp#L150
There was a problem hiding this comment.
I don't think getTimestamp should have been call as a PHP function at all here. THen again, timelib should have a function for returning timestamp/fraction, and all the other fields really.
There was a problem hiding this comment.
@derickr I think the idea here might have been (I don't know) that people might extend DateTime::getTimestamp() in userland. If this is not something we want to respect, then directly fetching the value from the underlying structure is of course much better.
There was a problem hiding this comment.
@nikic agree, I thought that it's because of extending of DateTime::getTimestamp(). From my perspective it seems reasonable
|
Any news on this? |
|
Merged as 1ce355a, thanks! |
|
This is awesome! ❤️ x 💯 |
FIx for https://bugs.php.net/bug.php?id=74298
udat_formatsupports formatting only milliseconds (3 digits of fractions).Here is the statement from documentation:
Appends zeros if more than 3 letters specified. Truncates at three significant digits when parsing.