Drupal doesn't support other calendars, Solar Calendar(Gregorian calendar)
here is some works for supporting it using a module. However, it needs a custom hook to be added, which means editing common.inc

Drupal did a great job supporting RTL languages and include many translations. Now we need to step it up in 8.x
I basically added the custom hoon into common.inc
If you have a better way please do inform me.
PS: this hook can be used for all kinds of Calendar systems, more info http://drupal.org/project/calendar_systems

Files: 
CommentFileSizeAuthor
#106 drupal-format_date_alter-1178342.patch4.52 KBsinasalek
PASSED: [[SimpleTest]]: [MySQL] 41,149 pass(es).
[ View ]
#102 1178342-hook-format-date-alter-D7-do-not-test.patch1.37 KBDave Reid
#85 hook_format_date_alter_4.patch4.1 KBHadi Farnoud
PASSED: [[SimpleTest]]: [MySQL] 57,708 pass(es).
[ View ]
#83 hook_format_date_alter_5.patch4.09 KBHadi Farnoud
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hook_format_date_alter_5.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#74 ab_before2.txt1.26 KBbdgreen
#74 ab_after1.txt1.26 KBbdgreen
#70 before.d8.xhprof.txt972.84 KBcweagans
#70 after.d8.xhprof.txt972.63 KBcweagans
#70 Screenshot at 2013-05-14 15:43:49.png26.27 KBcweagans
#66 ab2_hook_format_date_alter_4.patch_before.txt1.25 KBbdgreen
#66 ab2_hook_format_date_alter_4.patch_after.txt1.25 KBbdgreen
#63 hook_format_date_alter.patch4.18 KBsinasalek
PASSED: [[SimpleTest]]: [MySQL] 55,649 pass(es).
[ View ]
#60 hook_format_date_alter.patch4.21 KBsinasalek
PASSED: [[SimpleTest]]: [MySQL] 53,307 pass(es).
[ View ]
#58 hook_format_date_alter.patch4.2 KBsinasalek
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hook_format_date_alter_2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#55 hook_format_date_alter.patch4.36 KBsinasalek
PASSED: [[SimpleTest]]: [MySQL] 53,272 pass(es).
[ View ]
#51 hook_format_date_alter.patch3.13 KBsinasalek
PASSED: [[SimpleTest]]: [MySQL] 52,500 pass(es).
[ View ]
#49 hook_format_date_alter.patch4.77 KBsinasalek
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hook_format_date_alter.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#43 drupal.1178342.43.hook_format_date_alter.patch4.77 KBGaelan
PASSED: [[SimpleTest]]: [MySQL] 49,413 pass(es).
[ View ]
#43 drupal.1178342.43.hook_format_date_alter.interdiff.txt1008 bytesGaelan
#41 drupal.1178342.39.hook_format_date_alter.patch4.12 KBGaelan
FAILED: [[SimpleTest]]: [MySQL] 49,389 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#41 drupal.1178342.39.hook_format_date_alter.interdiff.txt1.26 KBGaelan
#39 drupal.1178342.39.hook_format_date_alter.patch4.73 KBGaelan
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#39 drupal.1178342.39.hook_format_date_alter.interdiff.txt1.87 KBGaelan
#36 hook_format_date_alter_test.1178342.36.patch2.45 KBwuinfo
FAILED: [[SimpleTest]]: [MySQL] 48,992 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#36 hook_format_date_alter.1178342.36.patch4.28 KBwuinfo
PASSED: [[SimpleTest]]: [MySQL] 48,994 pass(es).
[ View ]
#35 hook_format_date_alter.1178342.35.patch4.28 KBwuinfo
PASSED: [[SimpleTest]]: [MySQL] 48,977 pass(es).
[ View ]
#24 drupal.1178342.24.hook_format_date_alter.patch1.84 KBGaelan
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal.1178342.24.hook_format_date_alter.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#24 drupal.1178342.24.hook_format_date_alter.interdiff.txt2.18 KBGaelan
#18 core-format_date_alter-1178342-18.patch1.66 KBsinasalek
PASSED: [[SimpleTest]]: [MySQL] 48,284 pass(es).
[ View ]
#10 1178342-core_format_date_hook_0.patch884 bytesHadi Farnoud
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1178342-core_format_date_hook_0_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#8 1178342-core_format_date_hook.patch975 bytesHadi Farnoud
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1178342-core_format_date_hook_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 1178342-core_format_date_hook.patch1008 bytesHadi Farnoud
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1178342-core_format_date_hook.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
core_format_date_hook.patch1008 bytesHadi Farnoud
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core_format_date_hook.patch.
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch, core_format_date_hook.patch, failed testing.

Hadi Farnoud’s picture

StatusFileSize
new1008 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1178342-core_format_date_hook.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
aspilicious’s picture

Please...

+++ b/includes/common.incundefined
@@ -1853,6 +1853,19 @@ function format_interval($timestamp, $granularity = 2, $langcode = NULL) {
+// Custom hook for other calendar supports (Arabian, Persian, etc.) originally written by Sina Salek

Come on are you giving yourself credit in the code?

+++ b/includes/common.incundefined
@@ -1853,6 +1853,19 @@ function format_interval($timestamp, $granularity = 2, $langcode = NULL) {
+          foreach (module_implements('format_date') AS $module) {
+            if ($module!='date') {
+                $function = $module .'_format_date';
+                $r=$function($timestamp, $type, $format, $timezone, $langcode);
+
+                if ($r!=false) {
+                    return $r;
+                }
+            }
+          }

- This is formatted very badly.
- AS with capital?
- Why excluding an non core module like date?
- Can't this be done through contrib modules like date?

Powered by Dreditor.

aspilicious’s picture

http://drupal.org/project/calendar_systems is working why need it to be in core?

Hadi Farnoud’s picture

its working if you add a custom hook in common.inc
this patch is not calendar_systems. it's just a hook to make it work. Not possible with Date module unfortunately!

Hadi Farnoud’s picture

I'm giving Sina credit not myself, does it really matter?
Date does not support other calendars

aspilicious’s picture

1) Yes its bad to credit someone in core.
2) It needs to follow the coding standards (don't use tabs, AS ==> as, ...)
3) the date hack has to go, if this becomes an official hook date module has to change their function. Or you should choose another hook name.

Hmm so that module only works if you hack core... Sounds bad...

Hadi Farnoud’s picture

StatusFileSize
new975 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1178342-core_format_date_hook_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I think a core module that supports other calendar systems could be a great boost in Drupal multilingual support.
I agree modifying core files is a bad idea for a module, but I couldn't think of better (proper) way of doing this (since the date in DB should always be Gregorian).

PS: 1 and 2 fixed.

aspilicious’s picture

+++ b/includes/common.incundefined
@@ -1853,6 +1853,19 @@ function format_interval($timestamp, $granularity = 2, $langcode = NULL) {
+
+// Custom hook for other calendar supports (Arabian, Persian, etc.)
+          foreach (module_implements('format_date') as $module) {
+            if ($module!='date') {
+                $function = $module .'_format_date';
+                $r=$function($timestamp, $type, $format, $timezone, $langcode);
+
+                if ($r!=false) {
+                    return $r;
+                }
+            }
+          }

Still to much space on the left side...

Powered by Dreditor.

Hadi Farnoud’s picture

StatusFileSize
new884 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1178342-core_format_date_hook_0_0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
aspilicious’s picture

I stil don't like the date module check, thats nothing we can do in core. And this needs review from some more experienced people. I'll try to get them look at this.

klonos’s picture

Thanx for filling this Hadi and for the provided initial patch.

Hey bram, please give a man a chance:

Hmm so that module only works if you hack core... Sounds bad...

If we reacted like that to every single core alteration, we would never move forth. There must be other examples as well, but simpletest is the one that comes to mind. Not such a bad idea after all huh? That altered core too, but it did happen and it is part of the developing circle now. Bottom line being fear of touching core should not be an argument for not accepting patches/changes.

...Other than that thanx for the code review.

sinasalek’s picture

subscribed

sourcesoft’s picture

I'm really looking forward to this hook since date always stores Gregorian and it's impossible without hacking core. Good job sinasalek with calendar_system, but we still need this at least in D8.

aspilicious’s picture

I still didn't get an answer why there is a date module check.
I have the feeling date_module should provide a hook that accepts other date systems.

Gábor Hojtsy’s picture

Hadi Farnoud’s picture

it is somehow related. they both have the same aim. #1811912: Add pluggable calendar backend to core and centralize date translation is going nowhere?

sinasalek’s picture

Title:Other calendars support» Allowing contributed modules to alter format_date function result
StatusFileSize
new1.66 KB
PASSED: [[SimpleTest]]: [MySQL] 48,284 pass(es).
[ View ]

This is not only for supporting multi calendars , it's a generic hook that can used to alter format_date function.
Two other use cases are
1- To convert numbers into native Unicode numbers depending on site's language
2- To replace specific date formats with another ones on the fly

sinasalek’s picture

Assigned:Hadi Farnoud» sinasalek
Status:Needs work» Needs review

Changing issue status

Hadi Farnoud’s picture

Status:Needs review» Reviewed & tested by the community

It looks good to me Sina.

Hadi Farnoud’s picture

Priority:Normal» Major
xjm’s picture

Title:Allowing contributed modules to alter format_date function result» Allow contributed modules to alter the format_date() function result
Priority:Major» Normal
Status:Reviewed & tested by the community» Needs work
Issue tags:-calendar, -date API+Needs tests

Thanks @Hadi Farnoud and @sinasalek! This looks like it could be a useful feature. I have a few notes to improve the patch. We should also add an automated test that codifies the expected behavior of the alter hook.

  1. +++ b/core/includes/common.incundefined
    @@ -2007,7 +2007,21 @@ function format_date($timestamp, $type = 'medium', $format = '', $timezone = NUL
    -  return $date_time->format($format, $settings);
    +  $formatted_date = $date_time->format($format, $settings);
    +  ¶
    +  $context = array(
    +    'settings' => $settings,
    +    'date_time' => $date_time,
    +    'timezones' => $timezones,
    +    'timestamp' => $timestamp,
    +    'type' => $type,
    +    'format' => $format,
    +    'timezone' => $timezone,
    +    'langcode' => $langcode,
    +  );
    +  drupal_alter('format_date', $formatted_date, $context);
    +  ¶

    There's trailing whitespace in this hunk that needs to be removed.

  2. +++ b/core/modules/system/system.api.phpundefined
    @@ -3946,6 +3946,29 @@ function hook_filetransfer_info_alter(&$filetransfer_info) {
    + * A module may implement this hook in order to alter the formatted date string

    This should end in a period.

  3. +++ b/core/modules/system/system.api.phpundefined
    @@ -3946,6 +3946,29 @@ function hook_filetransfer_info_alter(&$filetransfer_info) {
    + * @param $formatted_date
    ...
    + * @param $context

    The parameters should have datatype documentation. See: http://drupal.org/node/1354#param-return-data-type

  4. +++ b/core/modules/system/system.api.phpundefined
    @@ -3946,6 +3946,29 @@ function hook_filetransfer_info_alter(&$filetransfer_info) {
    + *   Array of format_date function arguments and defined variables .
    + *   - settings
    + *   - date_time
    + *   - timezones
    + *   - timestamp
    + *   - type
    + *   - format
    + *   - timezone
    + *   - langcode

    This documentation is a bit unclear to me. We should probably have documentation for each key of the array explaining what it is and how it's used.

  5. +++ b/core/modules/system/system.api.phpundefined
    @@ -3946,6 +3946,29 @@ function hook_filetransfer_info_alter(&$filetransfer_info) {
    + * @see format_date()

    There should be a blank line above the @see.

I will see if I can find someone to help with the documentation. Thanks!

xjm’s picture

Regarding #1811912: Add pluggable calendar backend to core and centralize date translation, I think it still makes sense to put in this alter now since that isn't ready yet.

Gaelan’s picture

StatusFileSize
new2.18 KB
new1.84 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal.1178342.24.hook_format_date_alter.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Fixed docs and coding standards. Also removed $context['timezones'] and $context['settings'] because I can't think of any use case for them. If you can think of one, feel free to add them back.

Gaelan’s picture

Status:Needs work» Needs review

I am forgetful.

Gaelan’s picture

Status:Needs review» Needs work

Also, I think $context['timestamp'] and $context['timezone'] are duplicated by the DrupalDateTime object, so maybe they can be removed too. Again, if you can think up a use case for either, please tell me. I'm planning to go through $context and make sure there aren't any more duplicates.

sinasalek’s picture

Tx Gaelan, that's fine i put them there just to be sure.

tim.plunkett’s picture

Issue tags:+Needs profiling

This strikes me as something that would be rather slow. Even if I'm wrong, let's back that up with data.

sinasalek’s picture

Issue tags:-Needs profiling

This being fast or slow depends on drupal_alter function speed and the modules that implement it
But profiling is always nice :)

tim.plunkett’s picture

Issue tags:+Needs profiling

Yes, and if the function is called so often that it causes a performance regression then we don't put an alter there.

wuinfo’s picture

Any ideas of what the test case should be for this?

wuinfo’s picture

Failed to apply patch @ #24 to latest code. New code was committed according to this issue: http://drupal.org/node/1571632

Checking patch core/includes/common.inc...
error: while searching for:

  // Call date_format().
  $settings = array('langcode' => $langcode);
  return $date_time->format($format, $settings);
}

/**

error: patch failed: core/includes/common.inc:1930
error: core/includes/common.inc: patch does not apply
Checking patch core/modules/system/system.api.php...
Hunk #1 succeeded at 3809 (offset -141 lines).

sinasalek’s picture

Status:Needs work» Needs review
Issue tags:-Needs tests, -Needs profiling

Status:Needs review» Needs work
Issue tags:+Needs tests, +Needs profiling

The last submitted patch, drupal.1178342.24.hook_format_date_alter.patch, failed testing.

wuinfo’s picture

Assigned:sinasalek» Unassigned
Status:Needs work» Needs review
StatusFileSize
new4.28 KB
PASSED: [[SimpleTest]]: [MySQL] 48,977 pass(es).
[ View ]

Base on @sinasalek and @Gaelan's code, I added test code.

wuinfo’s picture

StatusFileSize
new4.28 KB
PASSED: [[SimpleTest]]: [MySQL] 48,994 pass(es).
[ View ]
new2.45 KB
FAILED: [[SimpleTest]]: [MySQL] 48,992 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

upload a patch with the tests only followed by the full patch. That way we can see the tests failing and the patch fixing it.

hook_format_date_alter.1178342.36.patch is same as hook_format_date_alter.1178342.35.patch.

sinasalek’s picture

Nice test case @wuinfo, tx

Hadi Farnoud’s picture

Issue tags:-Needs tests
Gaelan’s picture

StatusFileSize
new1.87 KB
new4.73 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

This patch removes $context['timestamp']and $context['timezone'] because they are duplicated in the DrupalDateTime object.

Status:Needs review» Needs work

The last submitted patch, drupal.1178342.39.hook_format_date_alter.patch, failed testing.

Gaelan’s picture

Status:Needs work» Needs review
StatusFileSize
new1.26 KB
new4.12 KB
FAILED: [[SimpleTest]]: [MySQL] 49,389 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Whoops… that contains a core hack that I am using as a workaround for #1856008: install.php doesn't work with symlinks. Here's an updated patch.

Status:Needs review» Needs work

The last submitted patch, drupal.1178342.39.hook_format_date_alter.patch, failed testing.

Gaelan’s picture

StatusFileSize
new1008 bytes
new4.77 KB
PASSED: [[SimpleTest]]: [MySQL] 49,413 pass(es).
[ View ]

config(system.date).timezone got moved to config(system.timezone). That makes our tests fail because the timezone isn't what we expect. Here's an updated patch.

Gaelan’s picture

Status:Needs work» Needs review

I have not ceased to be forgetful.

Status:Needs review» Needs work

The last submitted patch, drupal.1178342.43.hook_format_date_alter.patch, failed testing.

wuinfo’s picture

+++ b/core/includes/common.incundefined
@@ -1893,7 +1893,17 @@ function format_date($timestamp, $type = 'medium', $format = '', $timezone = NUL
+  $context = array(
+    'date_time' => $date,
+    'type' => $type,
+    'format' => $format,
+    'langcode' => $langcode,
+  );

+++ b/core/modules/system/system.api.phpundefined
@@ -3797,6 +3797,29 @@ function hook_filetransfer_info_alter(&$filetransfer_info) {
+function hook_format_date_alter(&$formatted_date, array $context) {
+  $formatted_date .= ' ' . $context['timezone'];
+}

Function hook_format_date_alter(&$formatted_date, array $context) is still using $context['timezone'] while timezone was taken out from above code.

Gaelan’s picture

Status:Needs work» Needs review

#43: drupal.1178342.43.hook_format_date_alter.patch queued for re-testing.

@wuinfo: Good point. I'll fix that.

klonos’s picture

Title:Allow contributed modules to alter the format_date() function result» Allow contributed modules to alter the format_date() function result.

...how are things looking here? Are we going to have this in D8? Asking because trying to keep core setup to latest and at the same time keep Calendar Systems working is a really painful task of having to (remember to) re-apply the core patch/hack the contrib module provides with each core upgrade.

sinasalek’s picture

StatusFileSize
new4.77 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hook_format_date_alter.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

corrected according to #46

Status:Needs review» Needs work

The last submitted patch, hook_format_date_alter.patch, failed testing.

sinasalek’s picture

StatusFileSize
new3.13 KB
PASSED: [[SimpleTest]]: [MySQL] 52,500 pass(es).
[ View ]

keeping up with head

sinasalek’s picture

Status:Needs work» Needs review
sinasalek’s picture

marcingy’s picture

Status:Needs review» Needs work

Please provide a patch in the correct format ie a diff

sinasalek’s picture

Status:Needs work» Needs review
StatusFileSize
new4.36 KB
PASSED: [[SimpleTest]]: [MySQL] 53,272 pass(es).
[ View ]

Here it is

sinasalek’s picture

If anything else needs to be done please let me know

sinasalek’s picture

#55: hook_format_date_alter.patch queued for re-testing.

sinasalek’s picture

StatusFileSize
new4.2 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hook_format_date_alter_2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Cleaning up the patch

Status:Needs review» Needs work

The last submitted patch, hook_format_date_alter.patch, failed testing.

sinasalek’s picture

StatusFileSize
new4.21 KB
PASSED: [[SimpleTest]]: [MySQL] 53,307 pass(es).
[ View ]

Patch created using git

sinasalek’s picture

Status:Needs work» Needs review
wuinfo’s picture

+++ b/core/modules/system/tests/modules/system_test/system_test.moduleundefined
@@ -380,3 +380,11 @@ function system_test_authorize_init_page($page_title) {
\ No newline at end of file

There are some spaces in the patch needed to be removed and also missing newline at the end of file.

sinasalek’s picture

StatusFileSize
new4.18 KB
PASSED: [[SimpleTest]]: [MySQL] 55,649 pass(es).
[ View ]

Mentioned problems fixed, here is the updated version

marcingy’s picture

Status:Needs review» Needs work

This needs profiling.

cweagans’s picture

marcingy, are you sure? I don't think it does. `format_date()` isn't called that many times during a page request. I'm inclined to RTBC this, but I'm interested in your rationale for wanting this profiled.

bdgreen’s picture

No significant difference - see attached ;)

Following the Apache Bench (ab) guidance (Benchmarking and profiling Drupal) there's no significant difference after applying the patch:

Before: 374+/-15.9ms

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:   361  374  15.9    370     470
Waiting:      334  346  14.2    342     432
Total:        361  374  15.9    370     470

After patch: 374+/-14.3ms

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:   360  374  14.3    370     454
Waiting:      333  345  12.5    342     414
Total:        360  374  14.3    370     454
tim.plunkett’s picture

What page was this on?
How many nodes were there?
Which content type, was the submitted by date showing?
Was there a datetime field in use?

Ideally the answers to those questions would be:
/node
10
article, yes
yes

marcingy’s picture

Request for profiling is based on http://drupal.org/node/1178342#comment-6781274 which had been nicely ignored until now ;)

cweagans’s picture

Assigned:Unassigned» cweagans

I'll have xhprof results in a few minutes. Thanks for pointing that out :)

cweagans’s picture

StatusFileSize
new26.27 KB
new972.63 KB
new972.84 KB

I used tim's params for this (10 articles nodes viewed on /node with submitted information showing + a date field being displayed on the teaser)

xhprof files attached.

cweagans’s picture

Assigned:cweagans» Unassigned
tim.plunkett’s picture

Was that a complete fluke or are the numbers backwards?

cweagans’s picture

I didn't believe it at first either, so I repeated the whole process 3 or 4 times and got similar results every time. I'm not sure how it can be both more function calls and faster, though.

bdgreen’s picture

StatusFileSize
new1.26 KB
new1.26 KB

Hmm? I've just repeated the Apache Bench (ab) test again following @tim.plunkett recommendation in #67- there's still no significant difference ... :-S

A fresh install of D8, "best" of five runs of 500 requests with Date field displayed (".../node; 10; article, yes; yes") on requested page (/usr/sbin/ab2 -c1 -n500 http://drupal8sb.tld/node i.e. one request at a time). And, then repeated after patch installed and cache cleared.

Before 586+/-21.2sd ms:

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:   561  586  21.2    580     718
Waiting:      534  557  19.9    551     681
Total:        561  586  21.2    580     718

After 588+/-22.5sd ms:

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:   561  588  22.5    581     693
Waiting:      534  558  20.9    552     658
Total:        561  588  22.5    581     693
sinasalek’s picture

Thanks @bdgreen & @cweagans for the benchmarks,
@tim.plunkett is there anything else that needs to be done for this patch to land?

sinasalek’s picture

Status:Needs work» Reviewed & tested by the community

I guess there is nothing left to do

Gaelan’s picture

Status:Reviewed & tested by the community» Needs review

Someone that has not worked on the code must mark the issue RTBC.

bdgreen’s picture

Status:Needs review» Reviewed & tested by the community
sinasalek’s picture

Thanks @bdgreen,
@Gaelan anything else must be done?

Gaelan’s picture

A few small things:

+++ b/core/modules/system/lib/Drupal/system/Tests/Datetime/DrupalDateTimeTest.phpundefined
@@ -109,4 +109,29 @@ public function testDateTimezone() {
+   * Test hook_format_date_alter() function and format_date() function.

I think it would make a bit more sense if this read "Test the hook_format_date_alter() and format_date() functions."

+++ b/core/modules/system/system.api.phpundefined
@@ -3569,6 +3569,27 @@ function hook_filetransfer_info_alter(&$filetransfer_info) {
+ * A module may implement this hook in order to alter the formatted date string.

I think that this line is redundant. The summary line already describes what this hook does.

+++ b/core/modules/system/tests/modules/system_test/system_test.moduleundefined
@@ -284,3 +284,11 @@ function system_test_authorize_init_page($page_title) {
+  // Reverse the date string for testing purpose.

testing purposeS

sinasalek’s picture

Tx @Gaelan Keen eyes and thanks everyone for helping out,
Hope someone continues this patch for Drupal 9. It doesn't seem we can ever make it for Drupal 8
I just realized that applying it manually after each update is way much easier indeed :)

Hanno’s picture

@sinasalek we're almost there to implement this in D8! You did a very good job so far. This patch will solve a need for all the people in countries that are dealing with other calendars then the Georgian one, or even not try to use Drupal because it's useless for them. While you can patch core, most can't or shouldn't. When someone improve the comments a bit it's ready to commit.

Hadi Farnoud’s picture

StatusFileSize
new4.09 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hook_format_date_alter_5.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

@sinasalek I made the changes @Gaelan asked. This patch is essential to a true multi language Drupal. hopefully it gets committed to D8 soon.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, hook_format_date_alter_5.patch, failed testing.

Hadi Farnoud’s picture

StatusFileSize
new4.1 KB
PASSED: [[SimpleTest]]: [MySQL] 57,708 pass(es).
[ View ]

can someone add RTBC tag?

Hadi Farnoud’s picture

Status:Needs work» Needs review
tstoeckler’s picture

+++ b/core/modules/system/system.api.php
@@ -3569,6 +3569,27 @@ function hook_filetransfer_info_alter(&$filetransfer_info) {
+ *   - type: The format to use as passed to format_date().

This should be the date type.

Overall I think it would be better to go for a container-based approach than this, but I won't hold this up.

Status:Needs review» Needs work

The last submitted patch, hook_format_date_alter_4.patch, failed testing.

Hadi Farnoud’s picture

Status:Needs work» Needs review

this patch does not need any more work. comments fixed according to #80. it has been reviewed and tested. can someone add RTBC tag?

tim.plunkett’s picture

#85: hook_format_date_alter_4.patch queued for re-testing.

cweagans’s picture

Status:Needs review» Reviewed & tested by the community

This is great :)

tim.plunkett’s picture

Beware, this will need a reroll once #2003934: Convert format_date to a service goes in.

tstoeckler’s picture

Since that issue is in, can we just mark this won't fix? calendar.module can just swap out the service and provide pluggable calendars, which we can then target for D9 core.

Hadi Farnoud’s picture

why would it be won't fix? it's been a long standing request since D7.

Hadi Farnoud’s picture

Issue tags:-Needs profiling
Gaelan’s picture

@Hadi Farnoud: That issue already provides a solution for this.

cweagans’s picture

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Needs work

You can swap out the date service if you want to override this behavior in Drupal 8, however, this might be worth backporting to Drupal 7.

starchild’s picture

+1 for backporting to drupal 7.

klonos’s picture

Issue tags:+needs backport to D7

..ok then.

sepehr.sadatifar’s picture

what should be done to continue process of backporting to D7?

sinasalek’s picture

Issue summary:View changes

The patch should be rewritten for D7

Dave Reid’s picture

I still think a simple alter hook should be added to the D8 code. I don't know how many times I've repeated this in D8 core issues: altering is a different use case than swapping services because the last module that wants to swap out the service wins and everyone else loses.

Anyway, here's an updated patch against Drupal 7. I'll work on a revised D8 version too.

sinasalek’s picture

Good point, i'll help getting this in :) i'll review the patch but it should against D8 first to be accepted by core commiters

herom’s picture

I would argue otherwise for D8. The usecase here is to allow using custom calendars, and that can be achieved by swapping the service. I can't think of a usecase where two different modules would want to alter the date format result at the same time; Once the first module changes the formatted date, the other ones would probably break.

Can you explain a use-case where the alter hook can be useful, but we can't do by swapping the service?

sinasalek’s picture

Status:Needs work» Needs review

Let test the patch

sinasalek’s picture

StatusFileSize
new4.52 KB
PASSED: [[SimpleTest]]: [MySQL] 41,149 pass(es).
[ View ]

Working patch for Drupal 7 with test coverage attached

Dave Reid’s picture

@sinasalek: I think it's worth leaving $date_time to a separate parameter and not merging it into $context. Could you please revert that change?

@herom: My use case is to have a date format that if the date is less than 24 hours old, outputs with 'x hours ago' instead of the actual time. I wanted this to be available *anywhere* date formats are used. This patch is the only way to make it consistently possible. That cannot be achieved by swapping services, because I might have that use case in several different ways on a site (and on my current site I actually do). I will repeat again: altering is a different use case than swapping services.

sinasalek’s picture

It is pointless to have separate parameter for $date_time since at the stage the alteration is called it's no longer possible to alter the date_time variable, note that you can always access the date_time variable using the $context var.