-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
[8.x] Adds datetime parsing to Request
instance
#39945
[8.x] Adds datetime parsing to Request
instance
#39945
Conversation
This example is not very accurate: if ($datetime = $request->input('when')) {
$datetime = Carbon::parse($datetime);
} It can already be written like this: $datetime = Carbon::make($request->input('when')); ... which does not seem too bad to me. |
Same case for |
I'm not sure what you mean by this?
That's not true. |
Just reworked it to return Also, |
I like the idea as I needed this many times, but you should probably take care of few stuff. First, not sure why the use of Date facade, when DateTime and/or Carbon classes are sufficient for this, IMHO. Also, Carbon throws an |
The developer may be using another library. Otherwise, the
Same case as above.
I would back to my original idea: if the format or the value to be formatted is invalid, throw an exception. It will return |
Yeah okay makes sense, if it can't be parsed by Carbon then should throw an exception just like Carbon would. |
@DarkGhostHunter I agree that this should be the behavior. |
It's ready for review. |
👍 |
* @param string|null $tz | ||
* @return \Illuminate\Support\Carbon|null | ||
*/ | ||
public function date($key, $format = null, $tz = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not $timezone
?
I like this but I wish the function handled defaults better sans coalescing when the input isn't null so one could benefit more from the parsing upfront and I think the timezone is not needed in most cases so it could be optional in an array and use the param space for the default. // Old
$startDate = Carbon::parse(Request::input('from', Carbon::now()->subYears(5)))->format('Ymd');
$endDate = Carbon::parse(Request::input('to', $lastTradeDate))->format('Ymd');
// New
$startDate = Request::date('from', 'Ymd') ?? Carbon::now()->subYears(5)->format('Ymd');
$endDate = Request::date('to', 'Ymd') ?? Carbon::parse($lastTradeDate)->format('Ymd');
// Expected?
$startDate = Request::date('from', ['Ymd'], Carbon::now()->subYears(5));
$endDate = Request::date('to', ['Ymd', 'America/Santiago'], $lastTradeDate);
$otherDate = Request::date('other', ['Ymd', 'America/Santiago']);
$anotherDate = Request::date('another', 'Ymd'); |
What?
Allows to conveniently retrieve an input from the Request as a
Carbon
object (orDateTimeInterface
). It uses theDate
facade behind the scenes.Why?
Because you can quickly get a lot of Carbon or Date calls to parse the date from the request, specially from different keys.
It supports for custom formats (otherwise it will try to parse it).
And also supports timezoning.
Since the input can be null, it's easier to create defaults:
BC?
No, only additive.