Skip to content
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

CLI --serverinfo: Add ISO country code support in addition to Qt Country IDs #2362

Closed
hoffie opened this issue Feb 8, 2022 · 5 comments · Fixed by #2841
Closed

CLI --serverinfo: Add ISO country code support in addition to Qt Country IDs #2362

hoffie opened this issue Feb 8, 2022 · 5 comments · Fixed by #2841
Assignees
Labels
feature request Feature request
Milestone

Comments

@hoffie
Copy link
Member

hoffie commented Feb 8, 2022

Currently, --serverinfo requires users to specify Qt Country IDs. This is not user-friendly.
With Qt6, --serverinfo will require users to specify Qt5 Country IDs, even if they are on Qt6. This is really not user-friendly.

Has this feature been discussed and generally agreed?

The ugliness of the status quo, especially when adding support for Qt6, has been officially stated by @pljones here: #2299 (review) :)

Describe the solution you'd like

--serverinfo should support ISO country codes, e.g. --serverinfo 'FooServer;FooCity;de
We should still keep support for the numerical IDs for compatibility.

@pljones
Copy link
Collaborator

pljones commented Feb 8, 2022

The current Locale -> Country letter method could possibly be used in reverse: take a two letter country code (de or zw), then look up the Locale based on the language (EN-us, DE-de, etc) to get the Qt Locale and hence the related country code for the "current" Qt. Then magic mapping gets that into the wire format, as per #2299.

If I've understood it correctly. I could be wrong...

@softins
Copy link
Member

softins commented Feb 8, 2022

--serverinfo should support named countries, e.g. --serverinfo 'FooServer;FooCity;FooCountry We should still keep support for the numerical IDs for compatibility.

For FooCountry, I think we should use the international standard abbreviations, rather than names, as defined in ISO-3316. Either the 2-letter or 3-letter variants (we could accept both). It then becomes independent of language, which could complicate things if we tried to interpret country names (e.g. Germany/Deutschland/Allemagne, Holland/Netherlands/Nederland, Belgium/Belgien/Belgie)

@acerix
Copy link

acerix commented Aug 25, 2022

After updating to 3.9.0 and Qt6, the country of my server changed without any changes to the config, apparently it is using Qt6 Country IDs now. I fixed it by updating to the new code in this list: https://doc.qt.io/qt-6/qlocale.html#Country-enum

Agree it would be more user friendly to specify country names/codes.

@hoffie
Copy link
Member Author

hoffie commented Aug 26, 2022

After updating to 3.9.0 and Qt6, the country of my server changed without any changes to the config, apparently it is using Qt6 Country IDs now.

Thanks for the report! This is unintended. I've opened #2809 to investigate this with rather high priority as it would be a regression.

@hoffie hoffie changed the title CLI --serverinfo: Add named country support in addition to Qt Country IDs CLI --serverinfo: Add ISO country code support in addition to Qt Country IDs Aug 30, 2022
hoffie added a commit to hoffie/jamulus that referenced this issue Aug 30, 2022
With this change, --serverinfo supports two-letter ISO country codes
such as "de", "gb", "nl", "fr".
Numeric Qt5-style codes keep working as expected.

Fixes: jamulussoftware#2362
@hoffie
Copy link
Member Author

hoffie commented Aug 30, 2022

I've prepared a branch which adds support for two-letter ISO country codes here: hoffie@serverinfo-two-letter-country-codes

Will submit after #2829 is merged as the branch is based on that PR.

@hoffie hoffie added this to the Release 3.9.1 milestone Aug 30, 2022
@pljones pljones added the feature request Feature request label Sep 7, 2022
hoffie added a commit to hoffie/jamulus that referenced this issue Sep 7, 2022
With this change, --serverinfo supports two-letter ISO country codes
such as "de", "gb", "nl", "fr".
Numeric Qt5-style codes keep working as expected.

Fixes: jamulussoftware#2362
hoffie added a commit to hoffie/jamulus that referenced this issue Sep 7, 2022
With this change, --serverinfo gains support for two-letter ISO country codes
such as "de", "gb", "nl", "fr".
Numeric Qt5-style codes keep working as expected.

Fixes: jamulussoftware#2362
hoffie added a commit to hoffie/jamulus that referenced this issue Sep 12, 2022
With this change, --serverinfo gains support for two-letter ISO country codes
such as "de", "gb", "nl", "fr".
Numeric Qt5-style codes keep working as expected.

Fixes: jamulussoftware#2362
hoffie added a commit to hoffie/jamulus that referenced this issue Sep 14, 2022
With this change, --serverinfo gains support for two-letter ISO country codes
such as "de", "gb", "nl", "fr".
Numeric Qt5-style codes keep working as expected.

Fixes: jamulussoftware#2362
hoffie added a commit to hoffie/jamulus that referenced this issue Sep 14, 2022
With this change, --serverinfo gains support for two-letter ISO country codes
such as "de", "gb", "nl", "fr".
Numeric Qt5-style codes keep working as expected.

Fixes: jamulussoftware#2362
hoffie added a commit to hoffie/jamulus that referenced this issue Sep 18, 2022
With this change, --serverinfo gains support for two-letter ISO country codes
such as "de", "gb", "nl", "fr".
Numeric Qt5-style codes keep working as expected.

Fixes: jamulussoftware#2362
ann0see pushed a commit to ann0see/jamulus that referenced this issue Nov 9, 2022
With this change, --serverinfo gains support for two-letter ISO country codes
such as "de", "gb", "nl", "fr".
Numeric Qt5-style codes keep working as expected.

Fixes: jamulussoftware#2362
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants