Skip to content
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

TASK: Translator uses locale chain #1451

Merged
merged 4 commits into from
May 21, 2019
Merged

TASK: Translator uses locale chain #1451

merged 4 commits into from
May 21, 2019

Conversation

ComiR
Copy link
Contributor

@ComiR ComiR commented Nov 19, 2018

This change makes getTranslationById and getTranslationByOriginalLabel use the configured
locale chain.

This is an updated version of #327 and #328. Please see the discussions there. May be retargeted on master.

@ComiR
Copy link
Contributor Author

ComiR commented Nov 19, 2018

Excuse my use of the force (especially because it gets documented now in the PR 👍), but I had to fix the tests with the help of Travis ;)

@kdambekalns
Copy link
Member

Huh, I created a different solution "by accident", see #1555

@ComiR
Copy link
Contributor Author

ComiR commented Apr 24, 2019

Well, not quite. In the case of the XLIFF source language being the same as the fallback language in the locale chain: yes. But for all other cases: no.
Simple example: Locale chain "de_CH, de_DE, en". Your solution would jump from "de_CH" straight to "en" (supposing it also is the source language).

@DavidSporer
Copy link
Contributor

Hi,
I've tested it today and can confirm that it works in the way @ComiR has described it. The fallback uses the locale chain as expected.
Let me know if you need additional information.

Thanks,
David

@albe
Copy link
Member

albe commented May 16, 2019

Thanks @DavidSporer for the verification and thanks @ComiR for the long lasting effort! I do think this is good to go when the originalLabel interpolation is in place again (which I'll add just now). But technically it's a new feature (changes behaviour) as is, so it should probably be retargeted to master. I'd love to have this in lower versions too, it just makes so much sense after all, but can't play with semver just for this.

Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

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

Looks good for me!

@bwaidelich bwaidelich removed their request for review May 20, 2019 10:10
Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

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

Looks good…

@kdambekalns kdambekalns merged commit dacdd15 into neos:4.3 May 21, 2019
@kdambekalns kdambekalns deleted the patch-4 branch May 21, 2019 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants