Feature homonyms translations #103
Conversation
…h gettext msgctxt base functionality on catalog, added attribute translate-context to directive and handled possibility of extra argument in the translate filter. Tests added for filter catalog and directive.
|
There are indeed changes to the catalog. The reason I did it this way instead of prepending or appending the context in the key is related with possible collisions and choosing the right separator for it. In my opinion this is the right way to do it but I'am willing to abandon it if strong practical reasons arise. I know that this has more impact on the gettext-tools (currently working on that). If you think the custom prefix approach is better i can change it if that avoids lots of work in the tools. |
| @@ -17,6 +17,13 @@ describe("Directive", function () { | |||
| catalog.setStrings("af", { | |||
| "This link: <a class=\"extra-class\" ng-href=\"{{url}}\">{{url}}</a> will have the 'ng-binding' class attached before the translate directive can capture it.": "Die skakel: <a ng-href=\"{{url}}\">{{url}}</a> sal die 'ng-binding' klass aangevoeg hê voor die translate directive dit kan vasvat." | |||
| }); | |||
| catalog.setStrings("pt-BR", { | |||
| Developer: [{ male: "Programador", female: "Programadora" }, | |||
There was a problem hiding this comment.
How does this work if I have <span translate>Developer</span> somewhere else in the code?
There was a problem hiding this comment.
It will just return Developer (if not in debug mode). See Line 50 .
It's just like it is not translated. Because per gettext specification if 2 equal msgid exists every entry must have a context, if you don't provide a context i consider that case as a non translated string.
There was a problem hiding this comment.
Hmmm, that's an annoying constraint. Certainly something we should document well.
I've been thinking we should add a validation mode to flag common errors, this is certainly one check we should include there as well.
There was a problem hiding this comment.
If you think about it there is no way of disambiguate it. It would be an arbitrary choice. As is indicates that something is missing just like a missing translation.
There was a problem hiding this comment.
Because per gettext specification if 2 equal msgid exists every entry must have a context, if you don't provide a context i consider that case as a non translated string.
I've been trying to find a reference for this but couldn't find one. Do you have a source for this?
Sounds like a very annoying restriction.
There was a problem hiding this comment.
It seems an undifened behaviour. Check Bug report and The context serves to disambiguate messages with the same untranslated-string. It is possible to have several entries with the same untranslated-string in a PO file, provided that they each have a different context. Note that an empty context string and an absent msgctxt line do not mean the same thing.
|
I'm afraid we have a problem. Have a look at #104. I'd love to have plural support for filters, but that would be incompatible with these changes. Not sure yet how we can combine the two |
|
Hum. I think we have to pass an object with the arguments as properties. The problem is in the tools really. I will make the change to use an object as an argument to the filter but I will not be able to implement the extraction on the tools. Right now I've implemented the extraction (except for the filter see) and will soon implement the compile. I can't spent more time right now (maybe later) implementing the filter extraction in the tools as is not a priority (you can bind the attribute to a property and use getPlural to set the value). |
Will try to do this today and modify the compile in the tools tomorrow. |
and translateContext properties and calls the appropriate catalog method This change allows more flexibility to accept functionality like #104
Previously the format of the catalog for translations with context was
[{ctxA: t1, ctxB: t2}, {ctxA: t1_plural, ctxB: t2_plural}] which was not
very clever (duplicate info, bad structure, sorry!).
Now the format is the more concise [{ctxA: [t1, t1_plural], ctxB: [t2, t2_plural]}].
The wrapping arrays in the context object values are mandatory even if
only one element is present (no plurals).
The external array wrapping is optional but allowed to achieve
consistency with how things were already done before context
translations were allowed.
|
I've been thinking about this and it should be possible to combine the two. My initial idea was that plural filters should look like this: {{"1 car" | translate:n:"$count cars"}}This would be incompatible with this pull request. It can all be resolved by naming the plural filter {{"1 car" | translatePlural:n:"$count cars"}}
{{"1 car" | translatePlural:n:"$count cars":"context"}} // With a contextLet's get this merged :-) |
|
I've merged this with a simple string syntax. Next up is the tools support. |
|
Correction: I've merged this to the @cristiano2lopes Apologies for this taking so long, I've been very busy with project work (and I'll be out on a vacation soon, so more delays expected :-/ ) |
|
@cristiano2lopes Nonetheless, excellent work this adds real functionality, so happy to see it come in. Next steps: see #53. |
Relative to issue #53 .