crow-translate / QOnlineTranslator

A library for Qt5 that provides free usage of Google, Yandex and Bing translate API.
GNU General Public License v3.0
79 stars 12 forks source link

Removed duplicate language names in regionName #44

Closed Crissium closed 1 year ago

Crissium commented 1 year ago

... and simplified code

Shatur commented 1 year ago

But it will return just a country name? I.e. "English (United States)" will become "United States".

Shatur commented 1 year ago

Oh, I see, it's because you display it in a combination with language? Makes sense!

Shatur commented 1 year ago

@Crissium maybe the whole function isn't needed? In Crow we probably will iterate over valid regions anyway. So you will have a country in your loop and can just call countyToString right away (with the expection for China, of course).

Crissium commented 1 year ago

Just as QLocale::Language does not include all the languages the translation engines support, also there's no way to tell apart Chinese dialects with merely QLocale::Country. So I think this should be done on the library side instead of doing it directly in Crow. Other projects may also use this feature (I just completed a personal project which reads texts to mp3 with this library).

Shatur commented 1 year ago

also there's no way to tell apart Chinese dialects with merely QLocale::Country

You should have `QOnlineTranslator::Language in this loop to be able to do so.

Other projects may also use this feature

I just thinking that other projects can just call countryToString. This is basically what this function do. And we could save one table lookup and lists creation (you create a new list every validRegions call)

Shatur commented 1 year ago

But let's see how it looks it will look in Crow, we can revisit my suggestion later.

Crissium commented 1 year ago

Yessir, I misunderstood you at first. I now see the point why this function can altogether be eliminated.

But I shall first implement this feature in Crow (without calling regionName). Maybe other ideas will pop up along the line.

Shatur commented 1 year ago

But I shall first implement this feature in Crow (without calling regionName). Maybe other ideas will pop up along the line.

Yes, let's see how it will look :)