WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 15 months ago

#28636 new enhancement

Add functions for working with local dates and times

Reported by: mboynes Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.9
Component: Date/Time Keywords: 2nd-opinion has-patch has-unit-tests
Focuses: Cc:

Description

Since WordPress sets date_default_timezone_set( 'UTC' ), working with local dates and times can sometimes be a bit of a nuisance. I'd like to propose adding a couple of functions to replace at least date() and strtotime(). We can add replacements for others too, like mktime() if the interest is there.

I've put together two proposals for accomplishing this, one using date_default_timezone_set() to temporarily change the timezone, and another (far more complicated version) using DateTime/DateTimeZone. I think the former is preferable, unless anyone has any reasons, performance or otherwise, why resetting the default timezone might be a bad thing?

I'm not married to the function names I chose (wp_date() and wp_strtotime()), so if it would be preferable to name them e.g. wp_local_date() and wp_local_strtotime(), I'm open to it.

Attachments (6)

28636.1.diff (2.8 KB) - added by mboynes 3 years ago.
Adds wp_date(), wp_strtotime(), and wp_get_timezone_string() to simplify working with local dates/times
28636.2.diff (2.8 KB) - added by mboynes 3 years ago.
Another option for wp_date() and wp_strtotime() which doesn't use date_default_timezone_set()
28636.3.diff (3.4 KB) - added by mboynes 3 years ago.
Leverage DateTime to parse the timezone from the string
28636.4.diff (2.7 KB) - added by mboynes 3 years ago.
Same as 28636.3.diff but without additional line ending edits
28636.5.diff (2.7 KB) - added by mboynes 3 years ago.
Latest update to date functions, removing adjacent @$ to improve readability
28636.6.diff (3.4 KB) - added by MikeHansenMe 3 years ago.
Refreshed with basic unit tests

Download all attachments as: .zip

Change History (29)

@mboynes
3 years ago

Adds wp_date(), wp_strtotime(), and wp_get_timezone_string() to simplify working with local dates/times

@mboynes
3 years ago

Another option for wp_date() and wp_strtotime() which doesn't use date_default_timezone_set()

#1 @nacin
3 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from Awaiting Review to Future Release

This is clever. I wish we could back away from setting UTC but that ship has long sailed.

I think this would break when dealing with arbitrary GMT offsets. Example, UTC+3.5 (Iranian Standard Time). gmt_offset can store any arbitrary float.

We probably need a more rigorous way to handle timezone juggling, which would make this easier.

I'd be curious what rmccue thinks as he is pretty familiar with date/time handling in PHP and is dealing with similar issues in the REST API.

#2 @nacin
3 years ago

  • Version changed from trunk to 2.9

Since 2.9 per #10940.

#3 follow-up: @rmccue
3 years ago

+1 on this ticket in general.

Looking at it, as nacin noted, gmt_offset allows anything. One huge issue is that DateTimeZone does not handle arbitrary timezone offsets; we've run into this, and had to change our handling because of it.

The latter patch seems to handle this fine.

One thing I see missing right now is checking for "Z" in the time string; Z indicates Zulu time (UTC) in ISO8601/RFC3339. Note that ISO8601 can also take timezone offsets in the form of "+hhmm" or "+hh" (ditto -). IMO, detecting this is more trouble than it's worth, and it might be worth reconsidering that approach.

I'll check this out. :)

#4 in reply to: ↑ 3 @mboynes
3 years ago

Great feedback, thanks!

Replying to rmccue:

One thing I see missing right now is checking for "Z" in the time string; Z indicates Zulu time (UTC) in ISO8601/RFC3339. Note that ISO8601 can also take timezone offsets in the form of "+hhmm" or "+hh" (ditto -). IMO, detecting this is more trouble than it's worth, and it might be worth reconsidering that approach.

You're right, this simply wasn't thorough enough. The more I elaborated on the regex, the more I was reinventing the wheel. I'm attaching another patch to leverage DateTime's parser to see if a timezone is present in the string. If one is, or the string fails to parse, it bails. Since this is to be a drop-in replacement for strtotime(), it makes sense that it should use the same parser (duh).

@mboynes
3 years ago

Leverage DateTime to parse the timezone from the string

@mboynes
3 years ago

Same as 28636.3.diff but without additional line ending edits

#5 @rmccue
3 years ago

  • Keywords has-patch added
  • Milestone changed from Future Release to 4.1

New patches slipped under my radar; sorry! Looking good. Only change I'd make is to use '@' . @timestamp instead of @$timestamp; the latter looks similar to the error suppression operator to me, so IMO it's jarring to read.

Otherwise, +1 on this patch.

@mboynes
3 years ago

Latest update to date functions, removing adjacent @$ to improve readability

#6 @mboynes
3 years ago

That's funny, it didn't bother me when I wrote it, but once you pointed it out I twitched a little. Latest patch concatenates.

#7 @johnbillion
3 years ago

  • Milestone changed from 4.1 to Future Release

It's a bit late to get these into 4.1.

I'd like to see some example use cases where these functions come in handy.

#8 follow-up: @mboynes
3 years ago

Here's the use-case which influenced this:

Let's say I have a form with a datepicker, and I want to schedule a cron event based on the date string I enter, perhaps 2014-11-19 at 23:00. If I use strtotime(), for me in EST, this will end up firing 5 hours earlier than I'm expecting. wp_strtotime() takes the trouble out of doing the conversion.

I think benefits/use cases for wp_date() are probably a bit more obvious. If I call date( 'Y-m-d H:i:s' ), I don't get my current time, which I think can be confusing for developers. Calling wp_date( 'Y-m-d H:i:s' ) simply provides the site's local time. Similarly, you can pass a timestamp, and you're going to get back the requested date/time format in the site's local timezone.

Since these aren't going to be used anywhere in core out of the gate, I don't think there's any harm in including them late in beta, but if you still want to punt to 4.2, I understand. Let me know if you have any questions!

#9 @swissspidy
3 years ago

Also related to #18146, which aims to add user-level timezone settings.

This ticket was mentioned in Slack in #core-restapi by rachelbaker. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by rachelbaker. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by rachelbaker. View the logs.


3 years ago

#13 @rachelbaker
3 years ago

  • Milestone changed from Future Release to 4.2

#14 in reply to: ↑ 8 @SergeyBiryukov
3 years ago

I wonder if we could reuse get_date_from_gmt() for wp_date():

function wp_date( $format, $timestamp = false ) {
	if ( ! $timestamp ) {
		$timestamp = time();
	}

	return get_date_from_gmt( date( 'Y-m-d H:i:s', $timestamp ), $format );
}

@MikeHansenMe
3 years ago

Refreshed with basic unit tests

This ticket was mentioned in Slack in #core by drew. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by drew. View the logs.


3 years ago

#17 @DrewAPicture
3 years ago

@rmccue: Could we get you to follow up here with your best recommendation?

This ticket was mentioned in Slack in #core by drew. View the logs.


3 years ago

#19 @DrewAPicture
3 years ago

  • Milestone changed from 4.2 to Future Release

No activity in more than a month, still looking for confirmation from @SergeyBiryukov and/or @rmccue. Punting.

This ticket was mentioned in Slack in #core-restapi by rachelbaker. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by ctalkington. View the logs.


2 years ago

#22 @MikeHansenMe
18 months ago

  • Keywords has-unit-tests added

28636.6.diff still applies adding has tests.

This ticket was mentioned in Slack in #core by sergey. View the logs.


15 months ago

Note: See TracTickets for help on using tickets.