Off-shot from #1845546: Implement validation for the TypedData API. Let's use this issue to a) figure out how to integrate both and b) how we could pursue translation in a way potx is aware of them.
Related issue: #1852106: Add the symfony translator interface or translation component.

So, as of now - symfony validator does not handle message translations. It defines messages at the constraint classes using class variables that end with the "message" suffix. E.g. see the Range constraint

/**
 * @Annotation
 *
 * @api
 */
class Range extends Constraint
{
    public $minMessage = 'This value should be {{ limit }} or more.';
    public $maxMessage = 'This value should be {{ limit }} or less.';
    public $invalidMessage = 'This value should be a valid number.';
    public $min;
    public $max;
}

So, we need to figure out whether that is something that is reasonable to parse with potx, or we need to re-define every single translation message in a way it works for us. For that, the requirement would be that it is possible to get the untranslated message also, such that you could log the violation message to watchdog (consider a web service request failling due to validation, so you log the violations).

A possible strategy for potx to work with the symfony pattern could be using a regex that checks for the value of all variables having the Message suffix in a class extending "XXConstraint".

Comments

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Are we sure XXConstraint is coming from a validator component? Do we need to check the namespace of the Constraint class extended? Sounds to me like Constraint is a pretty generic name that could be used in other namespaces. Granted, it might be little for false positives, if we pick up the xzyMessage property of those as well.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

Yeah, I think it shouldn't provide too many false positiives. However, if feasible we could additionally check that the namespaces ends with "Constraints" or "Constraint"?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

That is theoretically possible yes. I did not check what kind of data is provided by the PHP tokenizer for this. You can try with http://hu2.php.net/token_get_all

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

I think we won't be able to support context as is, e.g. all violations arr created via that method:

    public function addViolation($message, array $params = array(), $invalidValue = null, $pluralization = null, $code = null)

So there is no way to pass the context on here. Is that problematic? I'd assume sentences to not need to specify a context as usually they would not clash?

Different pluralizations could be handled already by the validating code by passing on the right message I think.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Right, sentences usually should not need a context.

How would "the right message" be picked on a language dependent basis?

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

How would "the right message" be picked on a language dependent basis?

uhm, I was not aware of locale_get_plural() so assumed a check for $count > 1 is enough. Given it's not - I've no idea how to do that :(

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

Well, we could move on with the concatenated string as message template, i.e. pass on implode(LOCALE_PLURAL_DELIMITER, array($singular, $plural)); if we have a plural count + provide a helper for that.

Then afterwards, when we have a plural count, we could explode the messages and move on...

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Yes, languages have a rich difference in plural formulas so the plural selection logic is language dependent and there might be 4 (eg. Slovenian) or 6 (eg. Arabic) different plural versions depending on the number: http://translate.sourceforge.net/wiki/l10n/pluralforms It is definitely not just $count > 1 :)

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

I posted a suggestion upstream to give us more flexibility here:
https://github.com/symfony/symfony/pull/6137#issuecomment-10840962

With that, we could do something like

$this->context->addViolation(new PluralConstraintViolation($count, $this->singleMessage, $this->pluralMessage, $args));
fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

I noted it's not possible to use format_pural() with watchdog: http://api.drupal.org/api/drupal/core!includes!bootstrap.inc/function/wa...

Maybe, it would make sense to have a kind of TranslatedMessage object which does the translation upon method call + allows access to untranslated messages? Then make watchdog() and our violation accept that?

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

I found a related issue: #1053690: watchdog call at system_cron() needs proper plural format - marked as won't fix :(

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

See http://drupal.org/node/1542144#comment-6790478 for a Translatable object prototype. I think a general solution like that would be preferable to a solutions specific to violations only.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

discussed with bschussek again. We figured out a way we should be able to implement the symfony translation interface: Once validator adopts the regular symfony way of handling plural translation we'll get message that have both using the | delimiter:

This value should have exactly {{ limit }} character.|This value should have exactly {{ limit }} characters.

Thus, we should be able to split by the delimiter and pass it on to format_plural(). This should work for supporting plural translations. As it's a single string we should be able to handle it with potx as well.

One disadvantage is that it comes with a new DX for handling plural messages, but we'll have one anyway as currently there is now way to handle postponed format_plural() translations. (see related #1542144-12: Mark strings as localizable/translatable (new t()-alike string function that isn't t(), only for potx)) Then, the used delimiter is different to ours, but obviously we cannot use a constant in our strings.

Given that, I think the symfony translation interface is not 100% match, but it should be sufficient for handlinv violation constraint message translations as we do not need to support $context. Thoughts?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Well, we don't have a way currently to handle postponed t()-s either, apart of the one-offs in watchdog() and hook_menu. Coming up with a new syntax for them is not a requirement to introduce this but I see you want to do it to conform to Symfony. I'll not say I'm happy about this, given that now translators will need to know if something is "@count comment" and "@count comments" or "{{ limit }} comment" and "{{ limit }} comments" it is essentially the same thing :/ :/

attiks’s picture

attiks CreditAttribution: attiks commented

I'm still hesitant about introducing another (SF) syntax for translatable strings, if this was something that was going to be used by not only Constraints I think it might be appropriate, but adding this only to be able to use some of SF Constraints directly is a bit over kill.

I would rather see us solve #1542144-12: Mark strings as localizable/translatable (new t()-alike string function that isn't t(), only for potx) and use our own coding standards for translations.

The other problem with 'blindly' using SF Constraints is that we need a way to handle string changes inside their Constraints, otherwise it will break existing sites.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

We can fix the replacement patters as the API doesn't enforce the pattern (as no one stepped in for symfony replacements yet, I guess we'll have Drupal style), but I think we'd have to stick to the "single|plural" pattern.

Coming up with a new syntax for them is not a requirement to introduce this but I see you want to do it to conform to Symfony.

Well, we need to come up with a way to do format_plural() such that translation is postponed and potx works, i.e. we could do something like violation_message_plural() and build in potx support for that. That would invent a new way to specify the translation also. So yes, we do not necesarily have to come up with a new syntax, but with a new way to specify the plural messages as format_plural() doesn't get it.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

The other problem with 'blindly' using SF Constraints is that we need a way to handle string changes inside their Constraints, otherwise it will break existing sites.

I don't think symfony will have lots of string changes once its stable, but still git will expose all of them once we decide to update. Thus, if they'd really occur we can still easily prevent them by overridding messages.

Given we'll probably have Drupal replacements, we'll have to override most symfony messages anyway. So this is not really about re-using symfony messages, but finding a way we can specify translations such that a) it works with the symfony validator API b) supports all Drupal translation features we need, i.e. pluralization (see #4).

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

So with Drupal replacements we'd have:

$message = 'This value should have exactly %limit character.|This value should have exactly %limit characters.';

I think that's a reasonable approach as it makes translation quite natural to Drupal, while giving use plural translation and possibly postponed translation?

attiks’s picture

attiks CreditAttribution: attiks commented

To make sure I understand it correctly, the only 'new' thing is the pipe, if so that's something I can live with.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

ad #19: exactly!

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented

Gabor: any feedback on #18?

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented
Priority: Normal » Major
Issue tags: +WSCCI, +D8MI

Resolving this (whether by #18, or something else, or deciding not to use Symfony Validation) is ultimately necessary for #1696648: [META] Untie content entity validation from form validation, which is necessary for web services. Therefore, raising to major and tagging.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

@effulgentsia: as said above all is needed for the extraction of those strings is a reasonably identifiable code pattern and the ability to find these strings with static code analysis up front, so we can identify the strings and expose for community translation. #18 combined with prior proposals of placement of those messages within classes and following a special naming scheme looks to satisfy that.

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented

From the issue summary:

Let's use this issue to a) figure out how to integrate both and b) how we could pursue translation in a way potx is aware of them.

If this issue's scope is "figure out", and given #23, what's left to do on this issue?

webmozart’s picture

webmozart CreditAttribution: webmozart commented

Some feedback from the Symfony camp. The validator now uses two different message styles by default:

  • message for regular messages. Replaceable params follow the format {{ param }}.
  • singular|plural for singular/plural messages. Params are supported like above. singular is used for the exactly one case, plural is used for zero and more than one.

All core constraints use these two formats by default.

Now the but!

These two formats are not sufficient for languages other than English! So the Symfony Translator supports, in addition to the above formats, a third format that allows to specify intervals. The English messages in the above two formats are mapped to translated messages that potentially, but not necessarily use the more capable interval notation in translation files.

That means that

  • if English messages are sufficient, developers can use the DefaultTranslator shipped with the Validator that simply replaces params in the above two message formats
  • if actual translations are required (including loading them from the file system and all that fuzz), the developer must use a more capable translator, such as the Symfony Translator or a Drupal translator

For Drupal this means that you can map the English messages in the above two formats to translated messages in a more complex format that is understood by your translator.

// map English messages to another language
$messages['some singular|some plural'] = '[0] whatever|[1] notation|[+] you want to use';
fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented

Thanks for the update - that works out! I'll implement this and merge it into the main validation patch.

fago’s picture

fago
Primary language German
Location Vienna
CreditAttribution: fago commented
Status: Active » Fixed

ok, I've implemented this now. Please check the patch/interdiff over at #1845546-101: Implement validation for the TypedData API.

I'm going to call this issue fixed now, as the coding will be handled as part of #1845546-101: Implement validation for the TypedData API. If anything left is to be discussed regarding translations please just re-open it.

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

Updated issue summary.