Date token descriptions are confusing

Hello @joachim,

Using 'a date' is suitable since the format can be applied to any date, not exclusively the current date.

I am planning to mark this issue as resolved. However, if you have an alternate input, please don't hesitate to share.

Thank you!

@joachim is a prolific core contributor so it probably is adequately described.

By “it” I mean “this issue”.

The various date tokens insert the current date, using the particular format -- at least that's what hook_tokens() looks like it's doing.

I'm working on this at DrupalCon 2023

I tried to work on this, but the existing fork does not install correctly - looks like an issue with composer dependencies.

Should I ignore the fork and make another?

------------------------------

 Problem 1
    - Root composer.json requires drupal/core-recommended == 11.9999999.9999999.9999999-dev -> satisfiable by drupal/core-recommended[11.x-dev].
    - drupal/core-recommended 11.x-dev requires drupal/core 10.2.x-dev -> satisfiable by drupal/core[10.2.x-dev] from composer repo (https://repo.packagist.org) but drupal/core[dev-3380239-date-token-descriptions, 11.x-dev] from path repo (repos/drupal/core) has higher repository priority. The packages from the higher priority repository do not match your constraint and are therefore not installable. That repository is canonical so the lower priority repo's packages are not installable. See https://getcomposer.org/repoprio for details and assistance.

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.
Failed to execute command composer update --lock: exit status 2
Failed to run exec_dir composer update --lock; error=exit status 1

Hi,
I have created a patch for the issue mentioned where I have changed the descriptions in system_token_info(). Could you please review it and provide the feedback.

Latest patch LGTM. Thanks!

Please remember to queue tests and verify there's a passing run before RTBCing stuff. I am queuing tests now.

Actually, this is not quite committable yet. It needs a small grammar fix for each string:

+++ b/core/modules/system/system.tokens.inc
@@ -57,27 +57,27 @@ function system_token_info() {
-    'description' => t("A date in 'short' format. (%date)", ['%date' => $date_formatter->format($request_time, 'short')]),
+    'description' => t("Current date in 'short' format. (%date)", ['%date' => $date_formatter->format($request_time, 'short')]),

All of these need to begin with "the".

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other improvements is not recommended and will not receive credit.)

Made changes as per comment #12.

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Remember to hide old patch files when converting to an MR. Thanks!

Rebasing as the MR didn't have the gitlab stuff.

Reroll was incorrect it seems feedback from #12 was left out, which was included in #13.

Just a reminder this is still a novice tagged issue.

Edit seems it was addressed, not sure it wasn't showing up at first..

Thank @stephencamilo for your assistance on this issue.

Please take note of the paragraph in #14:

Converting an issue to a merge request without other contributions to the issue will not receive credit.

Simple rerolls, rebases, merges, or conversions to merge requests no longer receive issue credit. Only updates that address a merge conflict or that include other fixes will be credited. For merge conflicts, the merge conflict that was resolved must be documented in the text of an issue comment.

In general, we ask that contributors not convert issues to merge requests unless they are making other fixes. To receive credit for contributing to this issue, assist with other outstanding tasks or unaddressed feedback, or provide a review. Thanks!

See the issue credit guidelines → for more information.

Crediting @joachim for the initial issue report, @anweshasinha for the original patch, @pradhumanjain2311 for improvements, @smustgrave for fixing the MR, and myself and @cilefen for issue management.

In the future, when using a patch workflow, provide interdiffs → for your patches. That allows reviewers to evaluate your changes. (Or, better, use merge requests so interdiffs are not needed.) Thanks!

OK, this looks pretty good now. I reviewed it locally with git diff --color-words to validate that all the changes are just this improvement.

I also checked to see if there were any other instances of this wording in core. There are two other strings, but in those cases they're referring to generic dates:

[ayrton:maintainer | Wed 13:39:41] $ grep -r -C2 "A date in"
./core/modules/update/src/ProjectSecurityData.php-   *
./core/modules/update/src/ProjectSecurityData.php-   * Two types of constants are supported:
./core/modules/update/src/ProjectSecurityData.php:   * - SECURITY_COVERAGE_END_DATE_[VERSION_MAJOR]_[VERSION_MINOR]: A date in
./core/modules/update/src/ProjectSecurityData.php-   *   'Y-m-d' or 'Y-m' format.
./core/modules/update/src/ProjectSecurityData.php-   * - SECURITY_COVERAGE_ENDING_WARN_DATE_[VERSION_MAJOR]_[VERSION_MINOR]: A
--
./core/modules/views/src/Plugin/views/filter/Date.php-        '#title' => $this->t('Value type'),
./core/modules/views/src/Plugin/views/filter/Date.php-        '#options' => [
./core/modules/views/src/Plugin/views/filter/Date.php:          'date' => $this->t('A date in any machine readable format. CCYY-MM-DD HH:MM:SS is preferred.'),
./core/modules/views/src/Plugin/views/filter/Date.php-          'offset' => $this->t('An offset from the current time such as "@example1" or "@example2"', ['@example1' => '+1 day', '@example2' => '-2 hours -30 minutes']),
./core/modules/views/src/Plugin/views/filter/Date.php-        ],
Binary file ./vendor/phpstan/phpstan/phpstan.phar matches

Finally, I read core/modules/system/system.tokens.inc and confirmed that there are no other issues with date token info that I can see.

However, there is still a small problem on the MR. It says "The Current date". "Current" should be lowercase: "The current date."

Hi @xjm @smustgrave
I intentionally deferred work on these issues. All the updates were made on November 14, 2023, and I've been addressing them sequentially in the Drupal issue queue without initially considering the tags assigned to each. I acknowledge this oversight and apologize for any confusion. Moving forward, I will ensure to prioritize and address issues based on their respective tags.

Keshav Patel → made their first commit to this issue’s fork.

Changes made as per comment #24 in MR. Please review.

Current has been lowercased

Reviewed locally one last time with git diff --color-words.

Committed to 11.x and 10.2.x as a minor-only UI string improvement. Thanks!

xjm → closed merge request !5411

Automatically closed - issue fixed for 2 weeks with no activity.