allows to specify the output format of CakeTime::timeAgoInWords #951

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
5 participants
@challet
Contributor

challet commented Nov 12, 2012

There's already a format option, but this one was used with the end option,
when there is a cutoff point to no longer will use words.

Default value remain the same as before : "%s ago"

lib/Cake/Utility/CakeTime.php
@@ -690,6 +699,7 @@ public static function timeAgoInWords($dateTime, $options = array()) {
$format = self::$wordFormat;
$end = self::$wordEnd;
$accuracy = self::$wordAccuracy;
+ $time_ago_format = self::$timeAgoFormat;

This comment has been minimized.

@ADmad

ADmad Nov 12, 2012

Member

As per Cake's coding standards the variable $time_ago_format needs to be in camel backed form $timeAgoFormat and so does the options key.

@ADmad

ADmad Nov 12, 2012

Member

As per Cake's coding standards the variable $time_ago_format needs to be in camel backed form $timeAgoFormat and so does the options key.

lib/Cake/Utility/CakeTime.php
@@ -715,6 +725,9 @@ public static function timeAgoInWords($dateTime, $options = array()) {
$end = $options['end'];
}
unset($options['end'], $options['format']);
+
+ $time_ago_format = isset($options['time_ago_format']) ? $options['time_ago_format'] : self::$timeAgoFormat;
+

This comment has been minimized.

@ADmad

ADmad Nov 12, 2012

Member

Extra blank line.

@ADmad

ADmad Nov 12, 2012

Member

Extra blank line.

lib/Cake/Utility/CakeTime.php
@@ -844,7 +856,7 @@ public static function timeAgoInWords($dateTime, $options = array()) {
}
if (!$backwards) {
- return __d('cake', '%s ago', $relativeDate);
+ return __d('cake', $timeAgoFormat, $relativeDate);

This comment has been minimized.

@markstory

markstory Nov 13, 2012

Member

This means that '%s ago' will not longer be caught by the i18n extract task and any timeAgoFormat will not show up in po files for sites using i18n files. Why not make the formatting conditional, and remove the default property that has been added. Either that or set the default property in the constructor so it can properly be translated.

@markstory

markstory Nov 13, 2012

Member

This means that '%s ago' will not longer be caught by the i18n extract task and any timeAgoFormat will not show up in po files for sites using i18n files. Why not make the formatting conditional, and remove the default property that has been added. Either that or set the default property in the constructor so it can properly be translated.

@challet

This comment has been minimized.

Show comment
Hide comment
@challet

challet Dec 14, 2012

Contributor

I think the last commit corrects the i18n usage for '%s ago'.
I would be glad to have an other opinion on that.

Contributor

challet commented Dec 14, 2012

I think the last commit corrects the i18n usage for '%s ago'.
I would be glad to have an other opinion on that.

lib/Cake/Utility/CakeTime.php
@@ -844,7 +857,13 @@ public static function timeAgoInWords($dateTime, $options = array()) {
}
if (!$backwards) {
- return __d('cake', '%s ago', $relativeDate);
+ if($timeAgoFormat) {

This comment has been minimized.

@dereuromark

dereuromark Dec 14, 2012

Member

missing space after if

@dereuromark

dereuromark Dec 14, 2012

Member

missing space after if

lib/Cake/Utility/CakeTime.php
+ } else {
+ // default format, needs to be translated
+ return __d('cake', '%s ago', $relativeDate);
+ }

This comment has been minimized.

@markstory

markstory Dec 14, 2012

Member

You don't need the else here. You can just return when the if closes.

@markstory

markstory Dec 14, 2012

Member

You don't need the else here. You can just return when the if closes.

This comment has been minimized.

@challet

challet Dec 14, 2012

Contributor

or maybe a ternary operator ? that's waht I used first before I added the comments line

@challet

challet Dec 14, 2012

Contributor

or maybe a ternary operator ? that's waht I used first before I added the comments line

This comment has been minimized.

@markstory

markstory Dec 15, 2012

Member

I don't think a ternary will make the code easier to read. The else however, is not doing anything right now.

@markstory

markstory Dec 15, 2012

Member

I don't think a ternary will make the code easier to read. The else however, is not doing anything right now.

This comment has been minimized.

@dereuromark

dereuromark Dec 15, 2012

Member

ternary might be too long here regarding the line length.

@dereuromark

dereuromark Dec 15, 2012

Member

ternary might be too long here regarding the line length.

@challet

This comment has been minimized.

Show comment
Hide comment
@challet

challet Dec 26, 2012

Contributor

anything else to review ?

Contributor

challet commented Dec 26, 2012

anything else to review ?

@lorenzo lorenzo closed this Jan 28, 2013

@lorenzo

This comment has been minimized.

Show comment
Hide comment
@lorenzo

lorenzo Jan 28, 2013

Member

Please reopen targeting the 2.4 branch

Member

lorenzo commented Jan 28, 2013

Please reopen targeting the 2.4 branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment