-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
language #426
language #426
Conversation
@@ -196,6 +196,7 @@ | |||
".pn", | |||
".ps", | |||
".pt", | |||
".pt_BR", |
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.
This is a list of Top-Level Domains, not languages. The Google provider already will fetch the languages list from Google's website (see #411), but that hasn't been released yet.
@@ -109,6 +109,7 @@ async def init_trans(self): | |||
"fa", | |||
"pl", | |||
"pt", | |||
"pt_BR", |
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.
Have you tested this? I see that Yandex web UI only shows Brazilian Portuguese as a destination language, so maybe that could break something, since this will add the language in both source and destination lists. I will try to test whenever I get some free time.
If it needs fixing you can check the DeepL provider on how to manage divergent source and destination languages lists.
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.
Yandex translates into European Portuguese. The translation is not the same as Brazilian Portuguese. Only Bing offers the option for European Portuguese. Providers do not offer the option to choose between Brazilian Portuguese and European Portuguese.
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.
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.
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.
In Google, as I told you in the other comment, that works since this PR (#411) that hasn't been released yet, so if you remove your change it will still work in the master
version. Also in your change you're adding .pt_BR
to the TLDs list, so when we pick a random Google domain you're adding translate.google.pt_BR
to the options, which is an invalid domain.
About Yandex, what I mean is that they just list "Portuguese" as a source language, and in you change you'll get both "Brazilian Portuguese" and "Portuguese" in both lists. So you say that Auto -> Brazilian Portuguese
works fine, what I want you to test is if Brazilian Portuguese -> English (or any other lang)
also works.
Also the selector will allow you to select Brazilian Portuguese -> Portuguese
in Yandex, test if that doesn't breaks anything.
In cases like Yandex with divergent source and destination lists, as I told you, ideally you need to do what we do for DeepL.
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.
I already modified this in the Fly project, but I don't remember how I made this correction. However, it is giving error in the request, as you mentioned. Hahaha.
The Portuguese language is missing in translation providers