Skip to content

Conversation

smg6511
Copy link
Collaborator

@smg6511 smg6511 commented Jun 1, 2025

What does it do?

Removes usage of the deprecated strftime function, implementing the recently-added modManagerDateFormatter class formatting methods in its place.

Why is it needed?

PHP plans to remove strftime in v9.0.

How to test

There were five areas where strftime was replaced:

  1. Output filters (date, strftime, fuzzydate),
  2. Date TV (Input Options Disabled Dates field examples and Output Options Date Format field & examples),
  3. Resource Overview (data controller),
  4. Package Browser (search results released date), and
  5. modFIle, which is limited in its implementation; it's not the primary file system handler (which is flysystem based). The one usage I played with was the Export functionality for Form Customization sets. Here I'm not sure that the date is ever used anywhere, so this seems to be more a matter of verifying the Export functionality itself is working as expected.

Note that a new static method was added to modManagerDateFormatter to convert existing strftime formats (e.g., '%A, %d %B %Y') to their datetime equivalents (e.g., 'l, d F Y') for BC.

That said, just play around in these areas and ensure all works as expected.

Related issue(s)/PR(s)

Resolves the issue raised in #15864. The subject of moving to the Intl extension and its IntlDateFormatter methods discussed in that same issue should probably be a separate feature proposal, which would ultimately be implemented within MODX's modManagerDateFormatter.

@smg6511 smg6511 requested review from opengeek and Mark-H as code owners June 1, 2025 04:09
@halftrainedharry
Copy link
Contributor

The subject of moving to the Intl extension and its IntlDateFormatter methods discussed in that same issue should probably be a separate feature proposal, which would ultimately be implemented within MODX's modManagerDateFormatter.

So what exactly is the plan here?
I'm a bit concerned about the PR is this current form. We have a lot of non-english sites and it's quite common to output the month (or day of the week) as a "full textual representation". E.g. like this:

[[*my_date_tv:strtotime:date=`%d. %B %Y`]]

With this PR, the displayed output changes, and the name of the month is always in english. Which shouldn't be the case.

@smg6511
Copy link
Collaborator Author

smg6511 commented Jun 1, 2025

@halftrainedharry - Good point, I was kind of "running with it" given that everything else had been changed away from strftime. The couple places I need to reconsider are the Output Filter and TV Output (Date) Option since their used to format public-facing content. Let me look at integrating Intl in now; does anyone object to doing that? If we're not ready to fully commit to Intl I could add some checks in and still use strftime if Intl is unavailable and php < 9. Thoughts?

smg6511 added 13 commits August 1, 2025 12:09
Replace strftime with modManagerDateFormatter
Update doc blocks, ref datetime instead of strftime format
Make how formatter is instantiated consistent with other usages
Replace strftime with formatter
Replace strftime with formatter
Code style/quality fixes
Replace strftime with formatter and add backward compat support for existing TVs using strftime formatting strings (new formatter static method to convert these to their datetime equivalents)
Code style/quality fixes
Fix default format in date and update format conflict prevention between string and date output filter types
Replace strftime with formatter
Code style/quality fixes
Progress commit (some work still tbd): Backs out changes to output filters and date tv props that made them non-localized by using php datetime. Adds ability to convert strftime to Intl date.
@smg6511 smg6511 force-pushed the 3.x-replace-strftime branch from b63c08f to 457ab3d Compare August 1, 2025 16:09
@smg6511
Copy link
Collaborator Author

smg6511 commented Aug 1, 2025

Note that the latest commit reflects work-in-progress to address the need for localized date display. At this point I'm looking more for opinion on where I'm going with this, structure/stragegy-wise.

I'm thinking of adding a system setting allowing for explicit adoption of Intl now, with options for: [1] backend (core manager areas), [2] frontend (tv output), or [3] both. Also I think a new idate output filter would be useful (but maybe more appropriate to put in a follow-up PR)—this would be a date filter that exclusively uses IntlDateFormatter.

The backend would be much easier for users to move to, as they'd only have to change one formatting setting (manager_date_format) to an Intl compatible one. The frontend change could be more involved, where folks might have multiple places they've set a strftime (or maybe datetime, but I don't think so?) formatting string. That said, in this update I baked in an auto conversion for when strftime is no longer available (so far it's looking like php 9 is when it'll be removed); that might be leveraged to enable auto-converted frontend implementation, allowing users to incrementally change their strftime patterns to Intl ones.

I don't believe Extras are a factor here (?), as ones that use a date in one way or another have their own formatting settings and implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants