Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time Interval Formatting #2772

Merged
merged 3 commits into from
Apr 24, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Time Interval Formatting
Fix #2768. DateFormatter handles only one of six special formats for time intervals `[h] [hh] [m] [mm] [s] [ss]`. This PR extends support to the rest. There should be no more than one of these in any format string. Although it certainly could make sense to treat `[d] [dd]` in the same manner, Excel does not seem to support those.

Interesting observations - hours and minutes are truncated (presumably because they may be followed by minutes and seconds), but seconds are rounded. Also, there are some floating point issues, which fortunately showed up for the example in the original issue. There, the time interval was 1.15, which should evaluate to a minutes value of 1656 (as it does in Excel). However, on my system it evaluated to 1655 because of a rounding error in the 13th decimal place. To overcome this, values are rounded to 10 decimal places before truncating.
  • Loading branch information
oleibman committed Apr 23, 2022
commit 7fe5ee84ead6e4287c87c647c7a07010e12ed2e4
30 changes: 0 additions & 30 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -4085,36 +4085,6 @@ parameters:
count: 1
path: src/PhpSpreadsheet/Style/ConditionalFormatting/ConditionalFormattingRuleExtension.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\DateFormatter\\:\\:escapeQuotesCallback\\(\\) has parameter \\$matches with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\DateFormatter\\:\\:format\\(\\) has parameter \\$value with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\DateFormatter\\:\\:setLowercaseCallback\\(\\) has parameter \\$matches with no type specified\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php

-
message: "#^Parameter \\#1 \\$format of method DateTime\\:\\:format\\(\\) expects string, string\\|null given\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php

-
message: "#^Parameter \\#2 \\$replace of function str_replace expects array\\|string, int given\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php

-
message: "#^Parameter \\#3 \\$subject of function preg_replace expects array\\|string, string\\|null given\\.$#"
count: 1
path: src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php

-
message: "#^Method PhpOffice\\\\PhpSpreadsheet\\\\Style\\\\NumberFormat\\\\Formatter\\:\\:splitFormat\\(\\) has no return type specified\\.$#"
count: 1
Expand Down
104 changes: 76 additions & 28 deletions src/PhpSpreadsheet/Style/NumberFormat/DateFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@ class DateFormatter
{
/**
* Search/replace values to convert Excel date/time format masks to PHP format masks.
*
* @var array
*/
private static $dateFormatReplacements = [
private const DATE_FORMAT_REPLACEMENTS = [
// first remove escapes related to non-format characters
'\\' => '',
// 12-hour suffix
Expand All @@ -32,10 +30,6 @@ class DateFormatter
// It isn't perfect, but the best way I know how
':mm' => ':i',
'mm:' => 'i:',
// month leading zero
'mm' => 'm',
// month no leading zero
'm' => 'n',
// full day of week name
'dddd' => 'l',
// short day of week name
Expand All @@ -44,32 +38,85 @@ class DateFormatter
'dd' => 'd',
// days no leading zero
'd' => 'j',
// seconds
'ss' => 's',
// fractional seconds - no php equivalent
'.s' => '',
];

/**
* Search/replace values to convert Excel date/time format masks hours to PHP format masks (24 hr clock).
*
* @var array
*/
private static $dateFormatReplacements24 = [
private const DATE_FORMAT_REPLACEMENTS24 = [
'hh' => 'H',
'h' => 'G',
// month leading zero
'mm' => 'm',
// month no leading zero
'm' => 'n',
// seconds
'ss' => 's',
];

/**
* Search/replace values to convert Excel date/time format masks hours to PHP format masks (12 hr clock).
*
* @var array
*/
private static $dateFormatReplacements12 = [
private const DATE_FORMAT_REPLACEMENTS12 = [
'hh' => 'h',
'h' => 'g',
// month leading zero
'mm' => 'm',
// month no leading zero
'm' => 'n',
// seconds
'ss' => 's',
];

private const HOURS_IN_DAY = 24;
private const MINUTES_IN_DAY = 60 * self::HOURS_IN_DAY;
private const SECONDS_IN_DAY = 60 * self::MINUTES_IN_DAY;
private const INTERVAL_PRECISION = 10;
private const INTERVAL_LEADING_ZERO = [
'[hh]',
'[mm]',
'[ss]',
];
private const INTERVAL_ROUND_PRECISION = [
// hours and minutes truncate
'[h]' => self::INTERVAL_PRECISION,
'[hh]' => self::INTERVAL_PRECISION,
'[m]' => self::INTERVAL_PRECISION,
'[mm]' => self::INTERVAL_PRECISION,
// seconds round
'[s]' => 0,
'[ss]' => 0,
];
private const INTERVAL_MULTIPLIER = [
'[h]' => self::HOURS_IN_DAY,
'[hh]' => self::HOURS_IN_DAY,
'[m]' => self::MINUTES_IN_DAY,
'[mm]' => self::MINUTES_IN_DAY,
'[s]' => self::SECONDS_IN_DAY,
'[ss]' => self::SECONDS_IN_DAY,
];

/** @param mixed $value */
private static function tryInterval(bool &$seekingBracket, string &$block, $value, string $format): void
{
if ($seekingBracket) {
if (false !== strpos($block, $format)) {
$hours = (string) (int) round(
self::INTERVAL_MULTIPLIER[$format] * $value,
self::INTERVAL_ROUND_PRECISION[$format]
);
if (strlen($hours) === 1 && in_array($format, self::INTERVAL_LEADING_ZERO, true)) {
$hours = "0$hours";
}
$block = str_replace($format, $hours, $block);
$seekingBracket = false;
}
}
}

/** @param mixed $value */
public static function format($value, string $format): string
{
// strip off first part containing e.g. [$-F800] or [$USD-409]
Expand All @@ -90,20 +137,21 @@ public static function format($value, string $format): string
$blocks = explode('"', $format);
foreach ($blocks as $key => &$block) {
if ($key % 2 == 0) {
$block = strtr($block, self::$dateFormatReplacements);
$block = strtr($block, self::DATE_FORMAT_REPLACEMENTS);
if (!strpos($block, 'A')) {
// 24-hour time format
// when [h]:mm format, the [h] should replace to the hours of the value * 24
if (false !== strpos($block, '[h]')) {
$hours = (int) ($value * 24);
$block = str_replace('[h]', $hours, $block);

continue;
}
$block = strtr($block, self::$dateFormatReplacements24);
$seekingBracket = true;
self::tryInterval($seekingBracket, $block, $value, '[h]');
self::tryInterval($seekingBracket, $block, $value, '[hh]');
self::tryInterval($seekingBracket, $block, $value, '[mm]');
self::tryInterval($seekingBracket, $block, $value, '[m]');
self::tryInterval($seekingBracket, $block, $value, '[s]');
self::tryInterval($seekingBracket, $block, $value, '[ss]');
$block = strtr($block, self::DATE_FORMAT_REPLACEMENTS24);
} else {
// 12-hour time format
$block = strtr($block, self::$dateFormatReplacements12);
$block = strtr($block, self::DATE_FORMAT_REPLACEMENTS12);
}
}
}
Expand All @@ -112,23 +160,23 @@ public static function format($value, string $format): string
// escape any quoted characters so that DateTime format() will render them correctly
/** @var callable */
$callback = ['self', 'escapeQuotesCallback'];
$format = preg_replace_callback('/"(.*)"/U', $callback, $format);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I'm reminded: callbacks in the format ['self', 'escapeQuotesCallback'] are being deprecated in PHP 8.2; the valid replacement format is [self::class, 'escapeQuotesCallback']

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also worth noting:

$format = preg_replace_callback('/"(.*)"/U', $callback, $format);

can return null or a string; and phpstan may subsequently complain when that value is passed as an argument to another function. Rather than doing anotherFunction($format ?? ''), it's probably better to enforce the string earlier with an explicit cast of the return from preg_replace_callback()

$format = (string) preg_replace_callback('/"(.*)"/U', $callback, $format);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes applied in latest commit. It looks like there are only 2 other places in PhpSpreadsheet where 'self' is used.

$format = preg_replace_callback('/"(.*)"/U', $callback, $format) ?? '';

$dateObj = Date::excelToDateTimeObject($value);
// If the colon preceding minute had been quoted, as happens in
// Excel 2003 XML formats, m will not have been changed to i above.
// Change it now.
$format = \preg_replace('/\\\\:m/', ':i', $format);
$format = \preg_replace('/\\\\:m/', ':i', $format) ?? '';

return $dateObj->format($format);
}

private static function setLowercaseCallback($matches): string
private static function setLowercaseCallback(array $matches): string
{
return mb_strtolower($matches[0]);
}

private static function escapeQuotesCallback($matches): string
private static function escapeQuotesCallback(array $matches): string
{
return '\\' . implode('\\', str_split($matches[1]));
}
Expand Down
90 changes: 90 additions & 0 deletions tests/data/Style/NumberFormatDates.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,94 @@
12345.6789,
'[DBNum3][$-zh-CN]yyyymmdd;@',
],
'hour with leading 0 and minute' => [
'03:36',
1.15,
'hh:mm',
],
'hour without leading 0 and minute' => [
'3:36',
1.15,
'h:mm',
],
'hour truncated not rounded' => [
'27',
1.15,
'[hh]',
],
'interval hour > 10 so no need for leading 0 and minute' => [
'27:36',
1.15,
'[hh]:mm',
],
'interval hour > 10 no leading 0 and minute' => [
'27:36',
1.15,
'[h]:mm',
],
'interval hour with leading 0 and minute' => [
'03:36',
0.15,
'[hh]:mm',
],
'interval hour no leading 0 and minute' => [
'3:36',
0.15,
'[h]:mm',
],
'interval hours > 100 and minutes no need for leading 0' => [
'123:36',
5.15,
'[hh]:mm',
],
'interval hours > 100 and minutes no leading 0' => [
'123:36',
5.15,
'[h]:mm',
],
'interval minutes > 10 no need for leading 0' => [
'1656',
1.15,
'[mm]',
],
'interval minutes > 10 no leading 0' => [
'1656',
1.15,
'[m]',
],
'interval minutes < 10 leading 0' => [
'07',
0.005,
'[mm]',
],
'interval minutes < 10 no leading 0' => [
'7',
0.005,
'[m]',
],
'interval minutes and seconds' => [
'07:12',
0.005,
'[mm]:ss',
],
'interval seconds' => [
'432',
0.005,
'[ss]',
],
'interval seconds rounded up leading 0' => [
'09',
0.0001,
'[ss]',
],
'interval seconds rounded up no leading 0' => [
'9',
0.0001,
'[s]',
],
'interval seconds rounded down' => [
'6',
0.00007,
'[s]',
],
];