@fabpot Thank you for the review. I've added your comment for the translation for the money type. As for the translations, do you mean all the files insrc\Symfony\Component\Form\Resources\translations\validators.*.xlf? Of course I can add the translation keys there, but I cannot do translations in all those different languages (except Dutch and maybe German).

@fabian I've added the translations to the English file. However, I am not sure what the correct precedure is for all the other languages (of which I can only translate 2 myself).

I am not sure that it is a good idea to change the default value for this option for all built-in form types. This means that custom translations in project wouldn't be working anymore after updating to a Symfony version containing this feature. That's not a good DX IMO.

@xabbuh You are right, these changes could be seen as backwards incompatible. What do you propose: close the PR or leave it open until there is a major new version?

I didn't thought about the BC issue here. Looks like a deal breaker :( Let's close then. Sorry@hiddewie

Perhaps we could reopen and find a proper upgrade path?

For instance, register abetter_invalid_messages or [INSERT_BETTER_NAME] option fromFormTypeValidatorExtension. Then if, ifbetter_invalid_messages isfalse, trigger a deprecation.
Aframework.form.better_invalid_messages framework option can be used to configure the default value used byFormTypeValidatorExtension globally, so any new project can be free of deprecations.

Then, in 5.1, deprecatebetter_invalid_messages andframework.form.better_invalid_messages to be removed in 6.0 as it won't have any effect anymore.

This seems a lot, but I find this PR valuable. Actually, IMHO, the BC break here is sensible.

@ogizanagi That would be possible! Would an option name ofimproved_validation_messages work?

What is the proper precedure for deprecating something in a far future release? Just open a new issue after this PR is merged (if it is merged at all)?

What is the proper procedure for deprecating something in a far future release? Just open a new issue after this PR is merged (if it is merged at all)?

Actually I don't think we have any specific workflow for this.
An issue could be great, but can be easily forgotten.
We could open a note on a project inhttps://github.com/symfony/symfony/projects in order to not forget this, but this kind of situation doesn't happen really often.
But I'm confident anyone doing the PR to remove the BC layer in 5.0 will think about doing one to deprecate it in 5.1 :)

@ogizanagi I've made some changes. Review is very welcome. The PR has grown considerably in code and also in complexity, but I hope everything works out.

Also see the updated description of the PR for some notes on the upgrade path. Mostly what you described.

I've enabled the improved messages in most of the tests, to avoid breaking the tests because of the deprecation message. Marking them all as legacy does not seem as a good idea to me. I've made some new tests that check the backwards deprecation notice both for framework and form).

@ogizanagi I've processed your comments. Thanks for the input.

• I will look into updating the receipe.
• I also want to write tests for at least some form types in the Core extension and using the Type Validator Extension which check the old and the new error messages.
• I have to update the changelog files for all these new framework/form parameters/options.

@ro0NL@ogizanagi Processed some comments.

@HeahDude I've processed your comments. Didn't think of the types which cannot be invalid.

Thank you@hiddewie!

I've reviewed it again and it looks good to me. But we need to adjust some things now that#28731 as been merged. Because the option did not exist in core before, only added by the validator extension (https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Validator/Type/FormTypeValidatorExtension.php#L59).

So here's what I spotted so far:

• the option can be handled here nowhttps://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Core/EventListener/TransformationFailureListener.php#L54.
• the check here need to be changedhttps://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Core/Type/TransformationFailureExtension.php#L33
  Maybe we should rely on theconstraints option only.

ping@xabbuh what do you think?

@HeahDude Thanks. Processed comment.

I'm unsure if I should remove the same code fromhttps://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php#L117 (I don't think so since any forms with theconstraints option are excluded from the new extension).

@hiddewie For this pull request to be merged, you need to rebase it (to remove the merge commits).

@fabpot Done.

@HeahDude Thanks for the review. I've added a load of tests for the custom validation messages, including the legacy variant.

I also noticed that the logic for legacy / non-legacy was flipped in the form types. Has been corrected.

Also rebased for new array syntax changes.

@xabbuh Done!

Thank you very much for your pull request!

As part of the Symfony EU funded hackathon (https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming), we were able to assemble most of the core team and big contributors in one place. Our goal is to finish as many open issues and PRs as possible.

Your commits will not be lost and you will therefore get credit for your work. All that will happen is that your commits will be moved to a new PR (see#30931) where all remaining concerns will be addressed.

Without your work this would not have been possible. So thank you once again!

No problem, thanks for the work!