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
Comments
Comment #2
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedComment #3
aspilicious CreditAttribution: aspilicious commentedPlease...
Come on are you giving yourself credit in the code?
- 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.
Comment #4
aspilicious CreditAttribution: aspilicious commentedhttp://drupal.org/project/calendar_systems is working why need it to be in core?
Comment #5
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedits 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!
Comment #6
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedI'm giving Sina credit not myself, does it really matter?
Date does not support other calendars
Comment #7
aspilicious CreditAttribution: aspilicious commented1) 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...
Comment #8
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedI 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.
Comment #9
aspilicious CreditAttribution: aspilicious commentedStill to much space on the left side...
Powered by Dreditor.
Comment #10
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedComment #11
aspilicious CreditAttribution: aspilicious commentedI 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.
Comment #12
klonosThanx for filling this Hadi and for the provided initial patch.
Hey bram, please give a man a chance:
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.
Comment #13
sinasalek CreditAttribution: sinasalek commentedsubscribed
Comment #14
sourcesoft CreditAttribution: sourcesoft commentedI'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.
Comment #15
aspilicious CreditAttribution: aspilicious commentedI 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.
Comment #16
Gábor HojtsyThis might be a duplicate of #1811912: Add pluggable calendar backend to core and centralize date translation?
Comment #17
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedit is somehow related. they both have the same aim. #1811912: Add pluggable calendar backend to core and centralize date translation is going nowhere?
Comment #18
sinasalek CreditAttribution: sinasalek commentedThis 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
Comment #19
sinasalek CreditAttribution: sinasalek commentedChanging issue status
Comment #20
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedIt looks good to me Sina.
Comment #21
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedComment #22
xjmThanks @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.
There's trailing whitespace in this hunk that needs to be removed.
This should end in a period.
The parameters should have datatype documentation. See: http://drupal.org/node/1354#param-return-data-type
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.
There should be a blank line above the
@see
.I will see if I can find someone to help with the documentation. Thanks!
Comment #23
xjmRegarding #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.
Comment #24
Gaelan CreditAttribution: Gaelan commentedFixed 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.Comment #25
Gaelan CreditAttribution: Gaelan commentedI am forgetful.
Comment #26
Gaelan CreditAttribution: Gaelan commentedAlso, I think
$context['timestamp']
and$context['timezone']
are duplicated by theDrupalDateTime
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.Comment #27
sinasalek CreditAttribution: sinasalek commentedTx Gaelan, that's fine i put them there just to be sure.
Comment #28
tim.plunkettThis strikes me as something that would be rather slow. Even if I'm wrong, let's back that up with data.
Comment #29
sinasalek CreditAttribution: sinasalek commentedThis being fast or slow depends on drupal_alter function speed and the modules that implement it
But profiling is always nice :)
Comment #30
tim.plunkettYes, and if the function is called so often that it causes a performance regression then we don't put an alter there.
Comment #31
wuinfo - Bill Wu CreditAttribution: wuinfo - Bill Wu commentedAny ideas of what the test case should be for this?
Comment #32
wuinfo - Bill Wu CreditAttribution: wuinfo - Bill Wu commentedFailed to apply patch @ #24 to latest code. New code was committed according to this issue: http://drupal.org/node/1571632
Comment #33
sinasalek CreditAttribution: sinasalek commented#24: drupal.1178342.24.hook_format_date_alter.patch queued for re-testing.
Comment #35
wuinfo - Bill Wu CreditAttribution: wuinfo - Bill Wu commentedBase on @sinasalek and @Gaelan's code, I added test code.
Comment #36
wuinfo - Bill Wu CreditAttribution: wuinfo - Bill Wu commentedhook_format_date_alter.1178342.36.patch is same as hook_format_date_alter.1178342.35.patch.
Comment #37
sinasalek CreditAttribution: sinasalek commentedNice test case @wuinfo, tx
Comment #38
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedComment #39
Gaelan CreditAttribution: Gaelan commentedThis patch removes
$context['timestamp']
and$context['timezone']
because they are duplicated in theDrupalDateTime
object.Comment #41
Gaelan CreditAttribution: Gaelan commentedWhoops… 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.
Comment #43
Gaelan CreditAttribution: Gaelan commentedconfig(system.date).timezone
got moved toconfig(system.timezone)
. That makes our tests fail because the timezone isn't what we expect. Here's an updated patch.Comment #44
Gaelan CreditAttribution: Gaelan commentedI have not ceased to be forgetful.
Comment #46
wuinfo - Bill Wu CreditAttribution: wuinfo - Bill Wu commentedFunction hook_format_date_alter(&$formatted_date, array $context) is still using $context['timezone'] while timezone was taken out from above code.
Comment #47
Gaelan CreditAttribution: Gaelan commented#43: drupal.1178342.43.hook_format_date_alter.patch queued for re-testing.
@wuinfo: Good point. I'll fix that.
Comment #48
klonos...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.
Comment #49
sinasalek CreditAttribution: sinasalek commentedcorrected according to #46
Comment #51
sinasalek CreditAttribution: sinasalek commentedkeeping up with head
Comment #52
sinasalek CreditAttribution: sinasalek commentedComment #53
sinasalek CreditAttribution: sinasalek commentedComment #54
marcingy CreditAttribution: marcingy commentedPlease provide a patch in the correct format ie a diff
Comment #55
sinasalek CreditAttribution: sinasalek commentedHere it is
Comment #56
sinasalek CreditAttribution: sinasalek commentedIf anything else needs to be done please let me know
Comment #57
sinasalek CreditAttribution: sinasalek commented#55: hook_format_date_alter.patch queued for re-testing.
Comment #58
sinasalek CreditAttribution: sinasalek commentedCleaning up the patch
Comment #60
sinasalek CreditAttribution: sinasalek commentedPatch created using git
Comment #61
sinasalek CreditAttribution: sinasalek commentedComment #62
wuinfo - Bill Wu CreditAttribution: wuinfo - Bill Wu commentedThere are some spaces in the patch needed to be removed and also missing newline at the end of file.
Comment #63
sinasalek CreditAttribution: sinasalek commentedMentioned problems fixed, here is the updated version
Comment #64
marcingy CreditAttribution: marcingy commentedThis needs profiling.
Comment #65
cweagansmarcingy, 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.
Comment #66
bdgreen CreditAttribution: bdgreen commentedNo 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
After patch: 374+/-14.3ms
Comment #67
tim.plunkettWhat 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
Comment #68
marcingy CreditAttribution: marcingy commentedRequest for profiling is based on http://drupal.org/node/1178342#comment-6781274 which had been nicely ignored until now ;)
Comment #69
cweagansI'll have xhprof results in a few minutes. Thanks for pointing that out :)
Comment #70
cweagansI 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.
Comment #71
cweagansComment #72
tim.plunkettWas that a complete fluke or are the numbers backwards?
Comment #73
cweagansI 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.
Comment #74
bdgreen CreditAttribution: bdgreen commentedHmm? 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:
After 588+/-22.5sd ms:
Comment #75
sinasalek CreditAttribution: sinasalek commentedThanks @bdgreen & @cweagans for the benchmarks,
@tim.plunkett is there anything else that needs to be done for this patch to land?
Comment #76
sinasalek CreditAttribution: sinasalek commentedI guess there is nothing left to do
Comment #77
Gaelan CreditAttribution: Gaelan commentedSomeone that has not worked on the code must mark the issue RTBC.
Comment #78
bdgreen CreditAttribution: bdgreen commentedComment #79
sinasalek CreditAttribution: sinasalek commentedThanks @bdgreen,
@Gaelan anything else must be done?
Comment #80
Gaelan CreditAttribution: Gaelan commentedA few small things:
I think it would make a bit more sense if this read "Test the hook_format_date_alter() and format_date() functions."
I think that this line is redundant. The summary line already describes what this hook does.
testing purposeS
Comment #81
sinasalek CreditAttribution: sinasalek commentedTx @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 :)
Comment #82
Hanno CreditAttribution: Hanno commented@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 than 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.
Comment #83
Hadi Farnoud CreditAttribution: Hadi Farnoud commented@sinasalek I made the changes @Gaelan asked. This patch is essential to a true multi language Drupal. hopefully it gets committed to D8 soon.
Comment #85
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedcan someone add RTBC tag?
Comment #86
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedComment #87
tstoecklerThis 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.
Comment #89
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedthis patch does not need any more work. comments fixed according to #80. it has been reviewed and tested. can someone add RTBC tag?
Comment #90
tim.plunkett#85: hook_format_date_alter_4.patch queued for re-testing.
Comment #91
cweagansThis is great :)
Comment #92
tim.plunkettBeware, this will need a reroll once #2003934: Convert format_date to a service goes in.
Comment #93
tstoecklerSince 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.
Comment #94
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedwhy would it be won't fix? it's been a long standing request since D7.
Comment #95
Hadi Farnoud CreditAttribution: Hadi Farnoud commentedComment #96
Gaelan CreditAttribution: Gaelan commented@Hadi Farnoud: That issue already provides a solution for this.
Comment #97
cweagansYou 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.
Comment #98
starchild CreditAttribution: starchild commented+1 for backporting to drupal 7.
Comment #99
klonos..ok then.
Comment #100
sepehr.sadatifar CreditAttribution: sepehr.sadatifar commentedwhat should be done to continue process of backporting to D7?
Comment #101
sinasalek CreditAttribution: sinasalek commentedThe patch should be rewritten for D7
Comment #102
Dave ReidI 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.
Comment #103
sinasalek CreditAttribution: sinasalek commentedGood point, i'll help getting this in :) i'll review the patch but it should against D8 first to be accepted by core commiters
Comment #104
herom CreditAttribution: herom commentedI 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?
Comment #105
sinasalek CreditAttribution: sinasalek commentedLet test the patch
Comment #106
sinasalek CreditAttribution: sinasalek commentedWorking patch for Drupal 7 with test coverage attached
Comment #107
Dave Reid@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.
Comment #108
sinasalek CreditAttribution: sinasalek commentedIt 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.
Comment #109
sinasalek CreditAttribution: sinasalek as a volunteer and commented@dave-reid , you can achieve your use case by the way using the current path. All you have to do is to data_time object in context and update the date format the way you need.
Patch is ready guys, we just need someone other than me to confirm that it's working
Comment #110
sinasalek CreditAttribution: sinasalek as a volunteer and commentedComment #111
sinasalek CreditAttribution: sinasalek as a volunteer and commentedComment #112
sinasalek CreditAttribution: sinasalek as a volunteer and commentedComment #114
sinasalek CreditAttribution: sinasalek as a volunteer and commentedComment #115
sinasalek CreditAttribution: sinasalek as a volunteer and commentedComment #117
sepehr.sadatifar CreditAttribution: sepehr.sadatifar commentedComment #119
Hadi Farnoud CreditAttribution: Hadi Farnoud as a volunteer commentedthings move so slowly in Drupal world. we gave up on it a few years ago.
too bad this issue is still not solved.
Comment #120
stefan.r CreditAttribution: stefan.r commentedComment #122
sinasalek CreditAttribution: sinasalek at Practicalidea commentedRe-roll and tab space fix
Comment #123
sinasalek CreditAttribution: sinasalek at Practicalidea commentedComment #124
sinasalek CreditAttribution: sinasalek at Practicalidea commentedCorrecting the patch extension
Comment #125
sinasalek CreditAttribution: sinasalek at Practicalidea commentedJust tested it on a clean install, applies cleanly and works fine.
I'm marking it as RTBC since we've been only re-rolling it for quite some time now.
Comment #126
Fabianx CreditAttribution: Fabianx as a volunteer commentedNice work! - Unfortunately it will need a little more work to be ready.
The $original_format line should probably be above the comment, else the comment wrongly applies to this line, which does not make sense.
Would it make sense to pass both the preg_replaced format and the original format here?
It seems weird to pass the latter as 'format' key, but not pass the actual format.
system_test is used for many things.
We only want to affect this one test.
We typically do that by checking for a variable here:
if (variable_get('system_test_reverse_date')) {
// ...
}
Else all dates are reversed always, which is not what we want for other tests and which could lead to strange side effects in the future.
Are we sure we use this elsewhere in the code?
e.g.
I would expect:
"node/" . $node1->nid
or
"node/{$node1->nid}"
but not directly putting it in.
Could we please check what other code / tests are using.
I might be wrong here though.
This line is not necessary.
Comment #127
sinasalek CreditAttribution: sinasalek at Practicalidea commentedNice review thanks, i applied almost all your suggestions. here are some notes
Comment #129
sinasalek CreditAttribution: sinasalek at Practicalidea commentedCorrection
Comment #130
Fabianx CreditAttribution: Fabianx as a volunteer commentedAlmost there:
Please revert your changes to the other function and keep the "node/$node1->nid" intact as there is pre-existing code using this.
nit - unrelated whitespace change
As this is pre-existing, there is no need to change it. Lets revert it.
I had not been sure if this code existed elsewhere already ...
This could use a comment to enable the reversing of the date.
Comment #131
sinasalek CreditAttribution: sinasalek at Practicalidea commented1. Removed
2. You are correct, i opened the file with PHPStorm and that code is highlighted which means that PHP recognizes it
3. I did a full code search before adding it, there wasn't any reference to system_test_reverse_date other than what this patch adds. I also did a bit of research and it seems that using variable_set/get is usual for the purpose you suggested. What do you mean by comment? how?
Comment #132
Fabianx CreditAttribution: Fabianx as a volunteer commentedTwo more nits and then we have it :).
Keep up the great work!
We miss the original_format in here.
I did not see that in my last review.
Can we add a comment before that, that says:
// Enable reversing of the date format.
Comment #133
sinasalek CreditAttribution: sinasalek at Practicalidea commentedHere it is again :)
Comment #134
Fabianx CreditAttribution: Fabianx as a volunteer commentednit - Contains (use uppercase); needs fullstop at end of sentence.
---
Marking for commit, though it might not happen before 7.52 is released and we target 7.60.
Comment #135
Fabianx CreditAttribution: Fabianx as a volunteer commentedComment #136
sinasalek CreditAttribution: sinasalek at Practicalidea commentedThanks @Fabianx
Corrected according to #134
Comment #138
sinasalek CreditAttribution: sinasalek at Practicalidea commentedComment #139
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedThanks for all the work on this issue.
The filter_xss_admin() change does not seem related to this issue, and is also probably the wrong thing to do (see discussion in #1892640: XSS in date formats admin page ). Why is it being added here?
Also, I am not sure I get where this issue ended up... If this is swappable in Drupal 8 (but does not have an alter hook) and that is why the issue was moved back from Drupal 8 to Drupal 7, then why are we adding an alter hook to Drupal 7 rather than making it swappable there too? (See drupal_http_request() for an example of how to make a procedural function swappable.) As Dave Reid pointed out in several of his comments above, swappable services and alter hooks are not the same thing... and I'm not sure if his concerns were really addressed.
So it seems to me like either this issue should go back to Drupal 8 to add an alter hook there first, or the Drupal 7 patch should be making the function swappable instead?
Comment #140
Fabianx CreditAttribution: Fabianx as a volunteer commentedComment #141
sinasalek CreditAttribution: sinasalek at Practicalidea commentedThanks for the review,
1. filter_xss_admin hasn't been changed, only a variable is used to make alteration possible, please read all the changes in the patch and not only a single line.
2. To backport date service to Drupal 7, service injection should be back ported first! Is there an issue for that?. The patch was first for Drupal 8 but it has been rejected because of swap-able date service.
I agree with you that swapping services is not the same as alter hook and Dupal 8 should also have this hook, but we've been on the right track at the beginning and the patch has to be rewritten for Drupal 8 to catch up with core, the latest Drupal 8 patch is before swap-able service landed in.
I probably can update the patch for Drupal 8 but i need confirmation of the person that's actually going to commit this, We've already spent too much time on this tiny little patch to go wrong once more :D Hopefully you're that person ? :)
Comment #142
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedThanks for your work on this patch.
I can make decisions for Drupal 7, but not Drupal 8 :) Adding the "Needs framework manager review" tag can sometimes help get direction on this kind of issue, so let's try that for starters. And let's move the issue to Drupal 8 if you think that makes sense? (From my personal point of view, either method is fine - add hook to both or add swappable method to Drupal 7, just mainly want to make sure they don't get out of sync with Drupal 7 having capabilities that Drupal 8 doesn't.)
I don't see an existing filter_xss_admin() in the patch, or in the current code at https://api.drupal.org/api/drupal/includes%21common.inc/function/format_.... Where does it happen in the current code?
I don't think that's necessary; see my comment above about how this can be done for a procedural function using the pattern found in drupal_http_request(). In particular, this part:
Comment #143
sinasalek CreditAttribution: sinasalek at Practicalidea commentedThanks that would be a good idea.
I think i misunderstood your point sorry. You were right, filter_xss_admin should not be there.
I see, but as we agreed, swapping service is a difference use case than alter hook. I think it's better to add the hook to Drupal 8.
Comment #144
sinasalek CreditAttribution: sinasalek at Practicalidea commentedHere is the list of D8 maintainers :
https://api.drupal.org/api/drupal/core%21MAINTAINERS.txt/8.2.x
Comment #145
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedOK, based on that and the fact that this issue basically needs feedback for Drupal 8, let's move this to Drupal 8 for now to see if that can get it moving.
Comment #149
bgilhome CreditAttribution: bgilhome commentedHere's a patch for 8.4.x-dev - let me know if it's suitable. I wasn't sure exactly where to put the hook example in core.api.php - I just added it last in the addtohooks section.
Comment #151
bgilhome CreditAttribution: bgilhome commentedWhoops - need to set the right version to test against. Patch attached again.
Comment #152
bgilhome CreditAttribution: bgilhome commentedNote that this is invoked when validating values in a datetime / range field - this should be skipped since altering the value may result in an invalid format. Should we pass something in $context in this case? e.g. $context['mode'] = 'form' / 'display'?
Comment #154
MustangGB CreditAttribution: MustangGB commentedBumping the D7 target until D8 patch is committed.
Comment #155
deviantintegral CreditAttribution: deviantintegral at Lullabot commentedThe last patch is failing because it requires the module handler service and it's not injected through the constructor. Best thing I can see without making a significant behaviour change would be to add a ModuleHandlerTrait so tests can at least set or mock the module handler as they see fit. Any other ideas?
Comment #156
deviantintegral CreditAttribution: deviantintegral at Lullabot commentedFor now, here's a patch that won't fail tests by checking that a module handler is available before altering.
Comment #165
catch#151 added hook documentation, but #156 doesn't.
I'm also a bit confused why we need an alter hook as opposed to a module or site adding its own date formats. Is it for when someone is using a hard-coded custom format instead of a configured date format? But in that case, an issue against the contrib module might be better.
Tagging for issue summary update for the above.