Skip to content

Allow add_format() in flutter gen-l10n DateTime format #156297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Nov 23, 2024

Conversation

Albert221
Copy link
Contributor

@Albert221 Albert221 commented Oct 6, 2024

This Pull Request extends the functionality of the flutter gen-l10n command (and its behavior during hot restart/reload) related to DateFormat type placeholders and their format. Until now, it was impossible to take advantage of intl's DateFormat.something().add_somethingElse(). The .add_x() part was impossible to achieve. This PR adds the ability to take advantage of these methods over DateFormat, by adding the add_ formats after the + character in the format in placeholder configuration. You can even have multiple added format parts if needed. All within a single placeholder.

Before the PR After the PR
{
    "bookingsPage_camo_dataLoaded": "CAMO data from {date} {time}.",
    "@bookingsPage_camo_dataLoaded": {
        "placeholders": {
            "date": {
                "type": "DateTime",
                "format": "yMMMd"
            },
            "time": {
                "type": "DateTime",
                "format": "jm"
            }
        }
    },
}
{
    "bookingsPage_camo_dataLoaded": "CAMO data from {date}.",
    "@bookingsPage_camo_dataLoaded": {
        "placeholders": {
            "date": {
                "type": "DateTime",
                "format": "yMMMd+jm"
            }
        }
    },
}

Resolves #155817.

Next steps

After this PR is merged, an update to i18n | Flutter > Messages with dates (source) shall be made to include a mention of this new addition.

Pre-launch Checklist

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 6, 2024
@Albert221 Albert221 force-pushed the gen-l10n-datetime-added-formats branch 6 times, most recently from 19d6e64 to d84221d Compare October 7, 2024 18:54
@Albert221 Albert221 force-pushed the gen-l10n-datetime-added-formats branch from d84221d to 0b39b57 Compare October 7, 2024 18:56
@Albert221 Albert221 marked this pull request as ready for review October 7, 2024 19:44
@Albert221
Copy link
Contributor Author

CC @andrewkolos, that's ready for Code Review now. The implementation turned out to be extremely simple. As for the tests, I added a case for the setup that already existed for the gen-l10n.

@Albert221 Albert221 force-pushed the gen-l10n-datetime-added-formats branch from 0b39b57 to 0e35032 Compare October 7, 2024 19:48
@andrewkolos andrewkolos self-requested a review October 8, 2024 20:27
Copy link
Contributor

@andrewkolos andrewkolos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

The approach looks good, and the code is delightfully simple. That being said, I think we should have test(s) that define the behavior around odd formats that could be made with this new syntax. For example, consider the format "yMd+jms+", which has a dangling +. What should the user see here? An error message? Would the last + be implicitly removed?

@bkonyi
Copy link
Contributor

bkonyi commented Oct 24, 2024

@Albert221 do you have plans to pick this back up?

@Albert221
Copy link
Contributor Author

@bkonyi I plan to return back to this PR in a week or two, yes; unless you'd like to add the remaining things, then tell me and fell free :)

@Albert221
Copy link
Contributor Author

Albert221 commented Nov 14, 2024

@andrewkolos tests added! Code ready for re-review. I'm not sure what's the issue with some of the unrelated tests failing though, should I merge from upstream everytime I get that?

EDIT: Merged latest main into PR. Now Windows tool_integration_tests_2_9 passes but Mac_x64 framework_tests_misc does not. I don't think that's because of my changes

Merged via the queue into flutter:master with commit da18845 Nov 23, 2024
145 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 23, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 24, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 24, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 26, 2024
@reidbaker reidbaker mentioned this pull request Dec 13, 2024
11 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Proposal] Date AND time DateFormat in ARB files DateTime placeholders format
3 participants