[gen_l10n] Support plurals and selects inside string - #86167

It looks like this PR would also help with #81981.

Hey there!

So the other day I discovered this great PR by @asashour which helped me a lot implementing internationalization in my Flutter app!

On my way, I encountered a few "gotchas". For example, defining a string containing both plural and normal placeholder generate an invalid output.
I first planned to patch and submit as an update to this PR.... Before I realized that I actually had to change the whole formatting logic with the aim of providing even more effective internationalization.

I based my work on the this PR which led me to the following commit: https://github.com/Delgan/flutter/commit/1398287af59c0f49ae445606e896e17f098f819b

Due to the refactor I made, I guess these modifications need to be discussed further.
Do you think I should open a new pull request towards flutter:master or towards asashour:75094_nested_plurals?

I just want to avoid duplicating the review and correction work if it is already taken into account in the commit I proposed. 👍

Actually, this PR doesn't change the behavior of the plural, there are already many tests for it, e.g.

The PR only handles the strings around it. On the other hand, it also adds the functionality of select.

But there is no harm to change the new tests to match that, which was done.

Another thing discovered in the last commit, we should escape the variable if the suffix can be merged with it, e.g.

    return 'ES - Oh, she found ${pluralString}ES - !';

instead of

    return 'ES - Oh, she found $pluralStringES - !';

This looks good; just some random formatting stuff to deal with. That and the problem noted in #86167 (review) which is OK to handle in a separate PR.

@asashour - @jonahwilliams provided some feedback on #85116 - #85116 (review) which may still apply here.

This looks good; just some random formatting stuff to deal with. That and the problem noted in #86167 (review) which is OK to handle in a separate PR.

Thanks for your feedback.

If I understand correctly, there is a case

"pluralInString": "Oh, she found {count, plural, =1 {1 item} other {all {count} items} }!",

in which other has {count} variable, and this is what I understood from the comment in the original case.

If this is not enough, please put a comment/issue with a test case, and cc/assign me.

Thanks @jonahwilliams for feedback, I also changed already existing tests for the uneeded try/catch.

@asashour - Per #86167 (comment), I changed "fixes" to "addresses some of the issues" when referring to
#75094 because, AFAICT, the countString substitution needed by the sample in that issue isn't covered by the changes here.

If you're willing to tackle the rest of #75094, please assign yourself to it. I tried to do so just now; for some reason GitHub wouldn't allow it.

Now I see the issue, it is whether count or countSring in

  other: 'all ${count} items',

I would work on it, should be delivered in 24 hours, either in this PR (if not merged), or another one.

It seems that issues can not be assigned to non-team members.

Wow, we certainly would appreciate that. I think a separate PR would be best, since this one is already queued up for a landing.

@HansMuller any chance this PR can make it to the next (upcoming) stable version?

The next stable release is a ways off, but yes it should be part of that.

@Delgan

Since this is merged, you can raise another PR to discuss your changes.

I guess the next big thing is supporting plurals inside selects and vice versa. I guess regular expressions wouldn't help much, and possibly a parser needs to be built, but I think this should be discussed with the team before putting efforts.

@HansMuller a hot fix for the current releases are not planned? This is the recommended way for multi language in flutter and it would be helpful to support all tags there as fast as possible, because the intl_translation package can not added as dependencie to flutter2 projects trough incompatible intl packages and null saftey.

Thank you @asashour for your effort to this pull request.

@asashour Thanks for your answer. You nailed it: I had to replace regexes with mini-parser to implement my POC for multiple/nested select and plurals. This change is not trivial and I will open a ticket to discuss it, now that your PR is merged. Thanks again for your work. ;)

@Knupper - A new beta release is due out any minute now and it's possible that would could apply this PR (and #86842) as a hot-fix to that. Would that help, or do you need to wait for a stable release?

@HansMuller this would help to verify that it now works correctly with our use cases. But we are not going to a beta release for our product so we need to wait until it is part of a stable release.