briannesbitt / Carbon

A simple PHP API extension for DateTime.
https://carbon.nesbot.com/
MIT License
16.58k stars 1.28k forks source link

⭐ Instantiating CarbonInterval with 0 makes forHumans() report "1s" #2035

Closed Crowdedlight closed 4 years ago

Crowdedlight commented 4 years ago

Hello,

I encountered an issue with the following code:

echo CarbonInterval::seconds(0)->cascade()->forHumans(['short' => true]);
echo CarbonInterval::fromString('0s')->cascade()->forHumans(['short' => true]))

Both with and without short modifer.

Carbon version: 2.29.1

PHP version: 7.4.2

I expected to get a value of 0 or 0s as I gave it an empty interval.

But I actually get:

1s

I have an application where an empty interval can occur if the user has no hours clocked on a platform. For now I am just using an inline if-check to see if the value is 0, before making the CarbonInterval.
I do think it would make sense to expect CarbonInterval to handle the case of 0 so you don't have to use if-sentences in all fields where a 0-interval could occur.

Thanks!

kylekatarnls commented 4 years ago

Hello,

By default the option CarbonInterface::NO_ZERO_DIFF disallow to have 0-diff (it's a legacy feature kept for backward-compatibility).

You can disable this options with:

echo CarbonInterval::seconds(0)->cascade()->forHumans(['short' => true, 'options' => 0]);

Or globally:

CarbonInterval::disableHumanDiffOption(CarbonInterface::NO_ZERO_DIFF);

But be careful with this last solution, it will also affect all the parts of your app and third-party libraries that call forHumans() or derived methods.

I won't change this for 2.x as it would be a breaking change, but I guess it would be relevant to make this option disabled by default in the next major version so I will keep this issue open for when I'll start it.

Crowdedlight commented 4 years ago

Ahh thanks. I was looking in the documentation but didn't find the CarbonInterface::NO_ZERO_DIFF option.

Makes good sense in regards to the default setting.