Skip to content
New issue

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

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

Already on GitHub? # to your account

Improve 'duration' filter #205

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

iatanasov77
Copy link

No description provided.

Copy link
Contributor

@kbond kbond left a comment

Choose a reason for hiding this comment

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

In general, I'm not sure about this. I wanted to keep the same logic as symfony/console, which is a very approximate duration. I admit at lower durations, the approximation can be way off though.

@@ -53,7 +53,7 @@ public function formatDiff(
*
* @source https://github.com/symfony/symfony/blob/ad72245261792c6b5d2db821fcbd141b11095215/src/Symfony/Component/Console/Helper/Helper.php#L97
*/
public function formatDuration(float $seconds, string $locale = null): string
public function formatDuration(float $seconds, int $precision = 2, string $locale = null): string
Copy link
Contributor

Choose a reason for hiding this comment

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

What about

Suggested change
public function formatDuration(float $seconds, int $precision = 2, string $locale = null): string
public function formatDuration(float $seconds, string $locale = null, ?int $precision = null): string

Adding the $precision before $locale is a BC break. Default to null to keep current behavior by default.

@@ -78,7 +78,7 @@ public function formatDuration(float $seconds, string $locale = null): string

return $this->translator->trans(
$format[1],
['%count%' => floor($seconds / $format[2])],
['%count%' => number_format($seconds / $format[2], $precision)],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
['%count%' => number_format($seconds / $format[2], $precision)],
['%count%' => null === $precision ? floor($seconds / $format[2]) : number_format($seconds / $format[2], $precision)],

To keep the current behaviour.

Copy link
Author

Choose a reason for hiding this comment

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

My previous logic was wrong :( I remove the precision parameter and add a private method that format the seconds into float number.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants