Make WordPress Core

Opened 9 years ago

Last modified 3 years ago

#29849 accepted enhancement

Better human_time_diff()

Reported by: kovshenin's profile kovshenin Owned by: rarst's profile Rarst
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Date/Time Keywords: has-patch needs-refresh
Focuses: Cc:

Description

Given #9272 #18495 and #27271 I think it's time for a better alternative to human_time_diff(). My proposal is an entirely new function which:

  • Should be simple - no bazillion-dimensional arrays for arguments
  • Should not lack context in translatable strings - "from now", "ago" should be part of the resulting strings
  • Should have consistent strings, no abbreviations such as mins, yrs, hrs, secs
  • Should assume all timestamps have no offsets

Attachments (3)

29849.diff (2.3 KB) - added by kovshenin 9 years ago.
29849.2.diff (11.2 KB) - added by akirk 9 years ago.
29849-codebase-replacements.diff (4.7 KB) - added by akirk 9 years ago.

Download all attachments as: .zip

Change History (13)

@kovshenin
9 years ago

#1 @kovshenin
9 years ago

  • Keywords has-patch dev-feedback added

First pass in 29849.diff.

#2 @SergeyBiryukov
9 years ago

  • Component changed from General to Date/Time

#3 @johnbillion
9 years ago

  • Keywords dev-feedback removed
  • Type changed from defect (bug) to enhancement

A few notes on 29849.diff:

  • The $to argument in human_time_diff() should be retained. Removing it makes it less functional.
  • $diff should be included in the wp_natural_time filter arguments.
  • I'd like to see some places this can be used in core in place of human_time_diff() to improve the i18n.
  • For this to be really useful it should support timestamps in MySQL format, so you can plug in dates straight from the post_date column.

@akirk
9 years ago

#4 @akirk
9 years ago

Maybe we can add some momentum to this again: human_time_diff() is often used in a wrong way, beginning with the example in the Codex: it suggests a string concattenation of ago:

<?php echo human_time_diff( get_the_time('U'), current_time('timestamp') ) . ' ago'; ?>

I'd argue that this is the only way that people use human_time_diff(), many don't know or care that a printf( __( '%s ago' ), human_time_diff( $timestamp ) ); is needed, and use the . ' ago' from the example.

wp_natural_time() would handle this out of the box, while including the other optimizations by @kovshenin. I have changed one thing in the behavior though: it only allows adjacent time units to avoid output like "1 year, 3 minutes ago".

I have added two diffs, one that shows where human_time_diff() could be replaced in the current codebase, the other one incorporates the suggestions by @johnbillon and also adds unit tests.

I'd also suggest to deprecate human_time_diff() and suggest the usage of this new function, also update the Codex and remove the misleading example of string concattenation.

#5 @GaryJ
9 years ago

Codex updated. I've added the internationalized version as an extra example to the one that was already there, since the point isn't to document how to incorporate internationalized strings, but show a more basic usage which can then be built upon.

http://codex.wordpress.org/Function_Reference/human_time_diff#Examples

#6 @akirk
9 years ago

Thank you very much for picking up the ball! I don't want to start a edit war in the Codex, so maybe we can discuss here: in general I think the context is not the place to explain variables, this should be done in a /* translators: */ comment.

Also I am unsure if adding a text-domain to the example is really helpful, part of the idea of wp_natural_time() is to leverage the common text usage (and its translation) of "%s ago".

Last but not least, in my opinion the "internationalized" version should be the only way to use it, we get the "%s ago" string in a translated version for free through WordPress, there is no need to advocate the usage of the concattenating version.

So I'd suggest to simply change the code example in the codex to this:

To print an entry's time ("2 days ago"):
<?php printf( __( '%s ago' ), human_time_diff( get_the_time('U'), current_time('timestamp') ) ); ?>
For comments:
<?php printf( __( '%s ago' ), human_time_diff( get_comment_time('U'), current_time('timestamp') ) ); ?>

#7 @DrewAPicture
9 years ago

I think we're getting to a point now where effort would be best served keeping the inline and curated docs associated with the code reference page updated, rather than the Codex.

@GaryJ: Would you mind also submitting the example(s) to the code reference article? We're just going to have to move it over manually from the Codex anyway.

https://developer.wordpress.org/reference/functions/human_time_diff

#8 @Viper007Bond
8 years ago

I'd encourage you all to take another look at #18495. For purposes of translation, we may be required to support a context parameter.

When I rewrote human_time_diff() for #9272, it got complicated fast but I'm not sure it can be avoided. See the switch ( $args['context'] ) { and following code in my patch:

https://core.trac.wordpress.org/attachment/ticket/9272/18495.patch

#9 @desrosj
3 years ago

  • Keywords needs-refresh added
  • Milestone set to Awaiting Review

The latest patch needs to be refreshed with today's code base.

@rarst would you happen to be interested in this or any thoughts to share for someone else to get started?

#10 @Rarst
3 years ago

  • Owner set to Rarst
  • Status changed from new to accepted

Yeah, I need to do a pass on remainder of Date/Time stuff.

Note: See TracTickets for help on using tickets.