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

[5.5] Let Carbon be Macroable. #19771

Merged
merged 3 commits into from Jun 28, 2017
Merged

[5.5] Let Carbon be Macroable. #19771

merged 3 commits into from Jun 28, 2017

Conversation

lucasmichot
Copy link
Contributor

@lucasmichot lucasmichot commented Jun 26, 2017

- Feeback is welcome -

When using \Carbon\Carbon a lot, there is a lack of possibility to create our own macros.
The only way to achieve this was to use two different classes:

use Carbon\Carbon;
use App\Helpers\Carbon as ExtendedCarbon;

This PR allows anyone to add macros to Carbon by using \Carbon\Support\Carbon.


See tests in https://github.com/laravel/framework/pull/19771/files#diff-cdb727ac9aaec34a81ce8fb0479f7f6f


Thus nesbot/carbon is now only required by illuminate/support

@browner12
Copy link
Contributor

would it be first better to try and push this upstream to the main repo?

@lucasmichot
Copy link
Contributor Author

lucasmichot commented Jun 26, 2017

Already tried briannesbitt/Carbon#608 @browner12


Having macros on Laravel side allows to comply the way we use them. People are quite used to it with many objects already Macroable

@browner12
Copy link
Contributor

my only concern is this becomes something of a maintenance nightmare.

do we start subclassing all other packages we bring in? Illuminate\Support\Parsedown, Illuminate\Support\Flysystem, etc?

not saying this doesn't provide a useful feature, just worried a little about the logistics.

@lucasmichot
Copy link
Contributor Author

lucasmichot commented Jun 26, 2017

I see your concern, but you only use Illuminate\Support\Parsedown and Illuminate\Support\Flysystem specific parts of your app.

In the case of Carbon you use it in much more parts, models, cache, queues, etc...

The other possibility is to create a service provider with a config item pointing to an extended Carbon class, taking \Carbon\Carbon as a default, but I believe this PR is making things easier.

@lucasmichot
Copy link
Contributor Author

lucasmichot commented Jun 26, 2017

I don't feel like this is a maintenance nightmare tho, as we do exactly the same with \Illuminate\Http\File

@taylorotwell
Copy link
Member

I'm curious what macros you are adding to Carbon? 😄

@BrandonSurowiec
Copy link
Contributor

BrandonSurowiec commented Jun 27, 2017

@taylorotwell My main use case is to easily localize Eloquent models in the user's timezone without all the extra setup of a trait or mutator in every Eloquent class for each date I want to manipulate. (Or a base class + Carbon subclass).

Alternatively, I've added a function around every output before:
localTime($post->created_at->format('F j, Y @g:iA'))

I suppose I could have made it more readable with this:
{{ localTime($post->created_at, 'F j, Y @g:iA') }}.

But still pretty gross compare to chaining.
{{ $post->created_at->local('F j, Y @g:iA') }}
{{ $post->created_at->local()->format('F j, Y @g:iA') }}

It would also be really nice to alias a couple of date formats. Especially ones that gets rid of the current year, but shows past or future ones:

{{ $post->created_at->toShortDateString() }} 
// Jun 27 
// Jun 13, 2016 
// Jun 8, 2018

Another use case I have is to add some helper methods for Carbon to help me output a date range string when two dates are given. (For events.)

function getFormattedDateAttribute()
{
  return Carbon::toDateRangeString($this->start_date, $this->end_date); 
}

// Outputs based on dates given:
// July 3-5, 2017
// July 3 - August 5, 2017
// July 3, 2017 - July 5, 2018

I've also built a system where we generate PDF brochures for a client with a Vue/Laravel based editor and schedule them to send on the next closest work hour. I made a class that gives me the Carbon instance I want so this is a weaker use-case: Clock::nextWorkHour(). But I could macro it instead: Carbon::nextWorkHour().

I think that is most of my use cases so far for projects I've worked on recently.

@browner12
Copy link
Contributor

in regards to the local timezone use case, I've done the same thing where I have a global helper that all of my dates get wrapped in.

@lucasmichot
Copy link
Contributor Author

lucasmichot commented Jun 27, 2017

I mostly use macros in Carbon for handling milliseconds, specific time-formats and handling DST differently - and all other "normal" Carbon functions of course

@joshmanders
Copy link
Contributor

I'm a big fan of this. Also would be interested in your macros @lucasmichot

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

Successfully merging this pull request may close these issues.

None yet

5 participants