-
Notifications
You must be signed in to change notification settings - Fork 64
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
Added European Portuguese localization #229
Conversation
Also, it would be good to add an abbreviation file for Portuguese at some point, but that can happen in a separate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 one comment, LGTM.
@@ -21,7 +21,8 @@ | |||
"it": "Svolta a sinistra verso Destination 1", | |||
"nl": "Ga linksaf richting Destination 1", | |||
"pl": "Skręć w lewo w kierunku Destination 1", | |||
"pt-BR": "Vire à esquerda sentido Destination 1", | |||
"pt-BR": "Exit the traffic circle towards Destination 1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just double checking: the Brazilian Portugese translations are expected to be in English here because the translation is missing, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Previously, the translation file lacked exit rotary
strings, since that maneuver type was added after this localization was first committed to the repository. The tests were passing until I ran UPDATE=1
, which tripped a test that all the translation files have keys for all the same maneuver types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although it’s unfortunate that this string has reverted to English, the previous Portuguese string was suboptimal anyways (“Turn left towards Destination 1”). I suppose it would be possible to pare back this PR to bring back the old string and test fixture, but I’m not sure that should be the bar for adding a separate localization.
Look fine to me. I guess any unnatural language quirks will only be noticed with it being used without the placeholders. I'll see if tomorrow I can put a couple hours aside and take care of the abbreviatons. |
Sounds good. Feel free to open a PR or issue if you spot any problems or need any strings updated here. |
OK, I need more than a couple hours for the abbreviations. Need to parse a list with thousands of them and get only the ones that are used in toponymics. Take will take more time than I was expecting. It will take a few days. Will have to go through the list letter by letter, one at a time, when I get some free time, or when I get tired of translating and need something else to do. |
No worries, it’s totally fine to start out with just the most common abbreviations, things like “Pq” for “parque”. There are plenty of abbreviations in English that we omit because we’re not sure a user would comprehend it correctly at a glance, and also plenty that we omit because they’re only applicable to a particular region. |
- Added a European Portuguese localization. [#229](#229) - The Spanish localization now uses the informal imperative form instead of the formal imperative form, for consistency with the Castillian Spanish localization. [#230](#230) - Added some abbreviations in German, Hebrew, Hungarian, Slovenian, and Ukrainian. [#226](#226) - Added support for named waypoints in arrival instructions. [#235](#235)
Issue
Added a European Portuguese localization from Transifex. Also updated the Brazilian Portuguese localization, which still has some gaps in translation.
@Rui-Santos, thanks for the translations! Please take a look at the test fixtures to ensure the generated sentences make sense. Feel free to keep refining the translations; we can update this PR in parallel until you think it’s ready for an initial release.
Tasklist
Requirements / Relations
This supersedes the Portuguese portion of #200. It complements the UI localizations added in mapbox/mapbox-navigation-ios#1252.
/cc @lyzidiamond @danpaz