I noticed the recent changes in this file, so time to chime in.
This approach only works if each English text (which happens to be the very first for each country, so that's why ISOtoCountry() "works") is exactly the same (including capitalization) as the country name specified in iso-codes. Also considering the history of this file, it is very much likely to miss changes done in the latest years...

IMHO this code is simply beyond any repair -- users ought to use iso-codes for these tasks, and not just the catalogs but also the JSON/XML files for all the mappings.

I have locally a prototype of library that uses only QtCore, QtXml, and KI18n for reading iso-codes data; I can publish it somewhere, and fix these issues once and for all.

In D14163#293194, @pino wrote:

I noticed the recent changes in this file, so time to chime in.
This approach only works if each English text (which happens to be the very first for each country, so that's why ISOtoCountry() "works") is exactly the same (including capitalization) as the country name specified in iso-codes. Also considering the history of this file, it is very much likely to miss changes done in the latest years...

IMHO this code is simply beyond any repair -- users ought to use iso-codes for these tasks, and not just the catalogs but also the JSON/XML files for all the mappings.

I have locally a prototype of library that uses only QtCore, QtXml, and KI18n for reading iso-codes data; I can publish it somewhere, and fix these issues once and for all.

You are right, there are definitely more problems in this code.

Replacing the data table currently in use here is next on the list, it's indeed heavily outdated. I'm not sure the iso-codes xml/json files are directly usable for this though, as this code can also map localized country names to ISO codes.
This change is indeed not perfect in case the spelling doesn't match exactly, but that seems to be the exception. So it's still a low-risk step forward IMHO.

Should this functionality become available elsewhere, e.g. in KF5, porting KContacts to that is of course an option too :)

In D14163#293198, @vkrause wrote:

In D14163#293194, @pino wrote:

I noticed the recent changes in this file, so time to chime in.
This approach only works if each English text (which happens to be the very first for each country, so that's why ISOtoCountry() "works") is exactly the same (including capitalization) as the country name specified in iso-codes. Also considering the history of this file, it is very much likely to miss changes done in the latest years...

IMHO this code is simply beyond any repair -- users ought to use iso-codes for these tasks, and not just the catalogs but also the JSON/XML files for all the mappings.

I have locally a prototype of library that uses only QtCore, QtXml, and KI18n for reading iso-codes data; I can publish it somewhere, and fix these issues once and for all.

You are right, there are definitely more problems in this code.

Replacing the data table currently in use here is next on the list, it's indeed heavily outdated. I'm not sure the iso-codes xml/json files are directly usable for this though, as this code can also map localized country names to ISO codes.

ISOtoCountry() IMHO ought to be deprecated altogether, and replaced by a pure iso-codes lookup done elsewhere (e.g. in an external library, as I mentioned). This is definitely possible using iso-codes.

OTOH countryToISO() is a more complex situation, and sadly iso-codes does not help here -- you can lookup English names, but that's about it.

This change is indeed not perfect in case the spelling doesn't match exactly, but that seems to be the exception. So it's still a low-risk step forward IMHO.

There seems to be 20-21 entries of countries that either a) do not exist anymore b) their spelling does not match any of name / common name / official name in iso-codes, which makes 9% of the entries... Not to mention around 26 countries that countrytransl.map does not know about.

In D14163#293231, @pino wrote:

ISOtoCountry() IMHO ought to be deprecated altogether, and replaced by a pure iso-codes lookup done elsewhere (e.g. in an external library, as I mentioned). This is definitely possible using iso-codes.

Once we have such a replacement, sure.

OTOH countryToISO() is a more complex situation, and sadly iso-codes does not help here -- you can lookup English names, but that's about it.

Right. I'm looking at CLDR for generating a sorted lookup table to ISO codes for this.

This change is indeed not perfect in case the spelling doesn't match exactly, but that seems to be the exception. So it's still a low-risk step forward IMHO.

There seems to be 20-21 entries of countries that either a) do not exist anymore b) their spelling does not match any of name / common name / official name in iso-codes, which makes 9% of the entries... Not to mention around 26 countries that countrytransl.map does not know about.

Yep, replacing countrytansl.map is definitely needed. Besides the outdated and incomplete content it's also too expensive to parse for how it's used atm.

However, until that's done, this patch gives us working translations for the majority of countries, by using the data set we agree we want to use for that. Not perfect obviously, but it's not meant to be the final state. I agree with all your points, but please give me the chance to implement this step by step :)

This change improves the existing code. I agree that a better solution is needed, but until we have that, I don't see any reason to not go forward with this, so +1 from me.

@pino, if you have anything that could replace this at some point, please share it. Maybe it could be adapted and put into Frameworks at some point so this imperfect thing can be dropped completely.