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

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

Closed
Crowdedlight opened this issue Mar 8, 2020 · 2 comments
Closed
Assignees
Labels
available on 3.x-dev Can yet be test requiring "nesbot/carbon": "3.x-dev" enhancement
Milestone

Comments

@Crowdedlight
Copy link

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
Copy link
Collaborator

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.

@kylekatarnls kylekatarnls added the postponed Not yet possible but candidate for a next major release label Mar 9, 2020
@Crowdedlight
Copy link
Author

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.

@kylekatarnls kylekatarnls removed the postponed Not yet possible but candidate for a next major release label Apr 21, 2020
@kylekatarnls kylekatarnls added this to the 3.0.0 milestone Apr 21, 2020
@kylekatarnls kylekatarnls self-assigned this May 17, 2020
@kylekatarnls kylekatarnls added available on 3.x-dev Can yet be test requiring "nesbot/carbon": "3.x-dev" enhancement labels May 17, 2020
@kylekatarnls kylekatarnls changed the title Instantiating CarbonInterval with 0 makes forHumans() report "1s" ⭐ Instantiating CarbonInterval with 0 makes forHumans() report "1s" Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
available on 3.x-dev Can yet be test requiring "nesbot/carbon": "3.x-dev" enhancement
Projects
None yet
Development

No branches or pull requests

2 participants