briannesbitt / Carbon

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

CarbonInterval::forHumans without weeks #1546

Closed Stadly closed 5 years ago

Stadly commented 5 years ago

Hello,

Would it be an idea to add an option to CarbonInterval::forHumans that outputs dayz instead of weeks and daysExcludeWeeks?

CarbonInterval::instance(new \DateInterval('P30D'))->forHumans(/*noWeeks=true*/); // 30 days
CarbonInterval::instance(new \DateInterval('P30D'))->forHumans();                 // 4 weeks 2 days

I'm not sure whether the $options parameter should be used, a new parameter should be added to the method, or a new method should be created.

As there is no easy way to just honor the spec, I thought just disabling weeks could be a pragmatic solution. See related issue: https://github.com/briannesbitt/Carbon/issues/1402

kylekatarnls commented 5 years ago

Hi @Stadly, let say first that disabling weeks will not "honor the spec" because both new \DateInterval('P21D') and new \DateInterval('P3W') returns the exact same object. So when you pass such an interval to CarbonInterval::instance() we have absolutely no way to know how this interval was created if it was P21D or P3W. We don't even know if it was created from specs of from diff(), manual increments, or any other method that can produce a DateInterval. Our unique information is "21 days".

But you can still tweak the factors to prevent days to be converted to weeks:

CarbonInterval::setCascadeFactors([
  'week' => [99999999999, 'days'],
]);

CarbonInterval::instance(new \DateInterval('P30D'))->forHumans(); // 30 days

And so the new method you want (special forHumans with no week unit) can be a macro using this trick:

CarbonInterval::macro('forHumansWithoutWeeks', function ($syntax = null, $short = false, $parts = -1, $options = null) {
  $factors = CarbonInterval::getCascadeFactors();
  CarbonInterval::setCascadeFactors([
    'week' => [99999999999, 'days'],
  ]);
  $diff = $this->forHumans($syntax, $short, $parts, $options);
  CarbonInterval::setCascadeFactors($factors);

  return $diff;
});

CarbonInterval::instance(new \DateInterval('P30D'))->forHumans(); // 4 weeks 2 days
CarbonInterval::instance(new \DateInterval('P30D'))->forHumansWithoutWeeks(); // 30 days
Stadly commented 5 years ago

I was not aware of the macro functionality. Thanks :)

kylekatarnls commented 5 years ago

One may not like this feature because it creates methods that will not be auto-documented (and so not detected automatically by your IDE) until you use a dedicated tool (barryvdh/laravel-ide-helper or tutorigo/laravel-ide-macros).

But we reached a point where it would be too many methods to maintain in our project if we implemented every possible needed one. So macros and mixins are the quick way to add Carbon methods: https://carbon.nesbot.com/docs/#api-macro

It would be nearly the same to extend Carbon with a sub-class then use traits in this sub-class.