Skip to content

Conversation

@cmahopper
Copy link
Contributor

Closes #258. @1ec5

Updated Danish translations and fixed some abbreviations.

Copy link
Contributor

@yuryleb yuryleb left a comment

Choose a reason for hiding this comment

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

Thanks for fix but it will be good to update ChangeLog also 😉

@yuryleb
Copy link
Contributor

yuryleb commented Aug 23, 2019

And translations should be updated on https://www.transifex.com/project-osrm/osrm-text-instructions/language/da/ first not directly in languages/translations/da.json file

@cmahopper
Copy link
Contributor Author

@yuryleb I see. The problem with translating on Transifex is that I cannot add modifiers as separate strings. See this for example:
https://github.com/Project-OSRM/osrm-text-instructions/pull/286/files#diff-2c8beca969ab6776d6f6d01bc61ea58aR208-R211

I cannot use the default modifier. It may work in English, but it doesn't work in Danish. The same goes for sharp/slight left/right. It's just not possible in Danish.

What would you suggest instead?

@cmahopper
Copy link
Contributor Author

@yuryleb I've updated the Transifex translations to the best of my ability, but the issues above are still present.

@yuryleb
Copy link
Contributor

yuryleb commented Aug 26, 2019

Thank you. Could you please now update files again but from Transifex (and don't forget ChangeLog 😉) and point me some fixture files with these incorrect sharp/slight left/right strings and comment what should be there instead - this allows me to understand the issue more completely. IMHO this could be fixed with post-processing by new grammar rules for Danish but I need more info yet.

Copy link
Contributor Author

@cmahopper cmahopper left a comment

Choose a reason for hiding this comment

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

@yuryleb I've added a Changelog entry!

I've also added a comment for each of the tests where they are currently incorrect. Note that some strings were actually correct before my changes, but I fixed other incorrect strings instead. So it's impossible to fix everything with the current system.

Could you elaborate on what post-processing is in this case? Is it something I can help with?

@yuryleb
Copy link
Contributor

yuryleb commented Aug 27, 2019

So let's try to reduce total number of incorrect phrases in fixures with Transifex first:

  • Move til inside modifier values
  • Update left/right words in sharp/slight modifiers
  • Use most common phrases beginnings (will solve different beginnings for straight/u-turn after)

@cmahopper
Copy link
Contributor Author

@yuryleb I've updated Transifex and pushed the changes, I'm gonna go through the tests now and note any incorrect translations.

Copy link
Contributor Author

@cmahopper cmahopper left a comment

Choose a reason for hiding this comment

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

@yuryleb Done! Way fewer errors now :)

"instructions": {
"ar": " توجّه مباشرة",
"da": "Hold til ligeud ved udfletningen",
"da": "Hold ligeud ved udfletningen",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Fortsæt ligeud ved udfletningen"

"instructions": {
"ar": "توجّه نحو مباشرة لمسافة {distance} ",
"da": "Hold mod ligeud mod Destination 1",
"da": "Hold ligeud mod Destination 1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Fortsæt ligeud mod Destination 1"

"instructions": {
"ar": "الزم مباشرة على Way Name",
"da": "Hold mod ligeud på Way Name",
"da": "Hold ligeud på Way Name",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Fortsæt ligeud på Way Name"

"instructions": {
"ar": "توجّه نحو مباشرة لمسافة {distance} ",
"da": "Hold mod ligeud mod Destination 1",
"da": "Hold ligeud mod Destination 1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Fortsæt ligeud mod Destination 1"

"instructions": {
"ar": "الزم مباشرة على Way Name",
"da": "Hold mod ligeud på Way Name",
"da": "Hold ligeud på Way Name",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Fortsæt ligeud på Way Name"

"instructions": {
"ar": "انعطف نحو الدوران للخلف",
"da": "Foretag et U-vending",
"da": "Drej U-vending",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Foretag en U-vending"

"instructions": {
"ar": "انعطف الدوران للخلف نحو Destination 1",
"da": "Foretag et U-vending mod Destination 1",
"da": "Drej U-vending mod Destination 1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Foretag en U-vending mod Destination 1"

"instructions": {
"ar": "انعطف الدوران للخلف على Way Name",
"da": "Foretag et U-vending ad Way Name",
"da": "Drej U-vending ad Way Name",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Foretag en U-vending ad Way Name"

"instructions": {
"ar": "انعطف الدوران للخلف نحو Destination 1",
"da": "Foretag et U-vending mod Destination 1",
"da": "Drej U-vending mod Destination 1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Foretag en U-vending mod Destination 1"

"instructions": {
"ar": "انعطف الدوران للخلف على Way Name",
"da": "Foretag et U-vending ad Way Name",
"da": "Drej U-vending ad Way Name",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be "Foretag en U-vending ad Way Name"

Copy link
Contributor

@yuryleb yuryleb left a comment

Choose a reason for hiding this comment

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

Thanks! So just 15 mistakes and just 2 replacements are necessary 👍

@1ec5, I recommend to merge this PR but don't close #258 - then I could propose another PR with initial grammar rules for Danish to fix rest issues.

@yuryleb
Copy link
Contributor

yuryleb commented Aug 29, 2019

@cmahopper, actually I already prepared a commit to push initial grammar rules right into your branch here but has no permissions to do this 😞 Maybe you could try to give me write access while we're waiting @1ec5 for approval?

@cmahopper
Copy link
Contributor Author

@yuryleb You should be added as a collaborator now :)

@yuryleb
Copy link
Contributor

yuryleb commented Aug 29, 2019

Thanks! Please review proposed grammar rules and resulting fixtures. I didn't add special grammar tests so as all results are controlled with them.

@cmahopper
Copy link
Contributor Author

@yuryleb The test results look perfect!

There's a final thing that still results in a weird speak, but I'm not sure if it can be fixed in the same way. I wrote it in this comment: #286 (comment)

The arrival string that starts with "Du vil ..." should be the only case where the first two words need to be switched. Would you be able to take a look at it?

@yuryleb
Copy link
Contributor

yuryleb commented Aug 29, 2019

Currently I has no idea how to fix this - current grammar rules implementation may not help in this case 😞 IMHO it's better to create separate issue specially for this to keep it in backlog at least.

@1ec5, we're ready to merge and to close #258 😉

@cmahopper
Copy link
Contributor Author

@yuryleb Alright, thanks for the help anyway :)

Copy link
Member

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

Thanks for your attention to detail! I wasn’t able to follow all the comments, but the changes look reasonable.

@1ec5 1ec5 merged commit b185d23 into Project-OSRM:master Sep 3, 2019
danpaz added a commit that referenced this pull request Sep 16, 2019
- Added a Yoruba localization. [#284](#284)
- Renamed “traffic circle” to “roundabout” in the English localization. [#285](#285)
- Rewrote the Burmese localization. [#282](#282)
- Fixed typographical errors in Italian. [#281](#281)
- Fixed grammatical errors in Danish. [#286](#286)
@danpaz danpaz mentioned this pull request Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Redundant maneuver modifiers in Danish

3 participants