From b8e61445531f2ec1a56d3720a16755df6c69aafa Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 3 Apr 2018 12:10:05 -0400
Subject: [PATCH] [locale] Document how the locale entry in Global Storage
 works.

 - Make the BCP47 value explicitly lower-case.
 - Add some constness and encapsulation.
 - Fix up documentation in the packages module explaining the
   format of the ${LOCALE} replacement (now forced to lower-case,
   but it is also only the language part, not e.g. en-UK).

FIXES #922
---
 src/modules/locale/LocaleConfiguration.cpp | 14 +++++++++++---
 src/modules/locale/LocaleConfiguration.h   |  9 +++++++--
 src/modules/locale/LocalePage.cpp          |  6 ++++--
 src/modules/packages/packages.conf         | 17 +++++++++++------
 4 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/src/modules/locale/LocaleConfiguration.cpp b/src/modules/locale/LocaleConfiguration.cpp
index b8f5f6a9e..c612c50c4 100644
--- a/src/modules/locale/LocaleConfiguration.cpp
+++ b/src/modules/locale/LocaleConfiguration.cpp
@@ -43,9 +43,11 @@ LocaleConfiguration::fromLanguageAndLocation( const QString& languageLocale,
                                               const QStringList& availableLocales,
                                               const QString& countryCode )
 {
-    LocaleConfiguration lc = LocaleConfiguration();
+    LocaleConfiguration lc;
+
+    // Note that the documentation how this works is in packages.conf
     QString language = languageLocale.split( '_' ).first();
-    lc.myLanguageLocaleBcp47 = QLocale(language).bcp47Name();
+    lc.myLanguageLocaleBcp47 = QLocale(language).bcp47Name().toLower();
 
     QStringList linesForLanguage;
     for ( const QString &line : availableLocales )
@@ -288,7 +290,7 @@ LocaleConfiguration::isEmpty() const
 
 
 QMap< QString, QString >
-LocaleConfiguration::toMap()
+LocaleConfiguration::toMap() const
 {
     QMap< QString, QString > map;
 
@@ -324,3 +326,9 @@ LocaleConfiguration::toMap()
 
     return map;
 }
+
+QString
+LocaleConfiguration::toBcp47() const
+{
+    return myLanguageLocaleBcp47;
+}
diff --git a/src/modules/locale/LocaleConfiguration.h b/src/modules/locale/LocaleConfiguration.h
index 073d19a5b..04d1db456 100644
--- a/src/modules/locale/LocaleConfiguration.h
+++ b/src/modules/locale/LocaleConfiguration.h
@@ -35,16 +35,21 @@ public:
 
     bool isEmpty() const;
 
+    QMap< QString, QString > toMap() const;
+    // Note that the documentation how this works is in packages.conf
+    QString toBcp47() const;
+
     // These become all uppercase in locale.conf, but we keep them lowercase here to
     // avoid confusion with locale.h.
     QString lang, lc_numeric, lc_time, lc_monetary, lc_paper, lc_name, lc_address,
             lc_telephone, lc_measurement, lc_identification;
-    QString myLanguageLocaleBcp47;
-    QMap< QString, QString > toMap();
 
     // If the user has explicitly selected language (from the dialog)
     // or numbers format, set these to avoid implicit changes to them.
     bool explicit_lang, explicit_lc;
+
+private:
+    QString myLanguageLocaleBcp47;
 };
 
 #endif // LOCALECONFIGURATION_H
diff --git a/src/modules/locale/LocalePage.cpp b/src/modules/locale/LocalePage.cpp
index 2172586ff..8b05ccfba 100644
--- a/src/modules/locale/LocalePage.cpp
+++ b/src/modules/locale/LocalePage.cpp
@@ -489,8 +489,10 @@ LocalePage::updateGlobalStorage()
             ->insert( "locationRegion", location.region );
     Calamares::JobQueue::instance()->globalStorage()
             ->insert( "locationZone", location.zone );
-    Calamares::JobQueue::instance()->globalStorage()
-            ->insert( "locale", m_selectedLocaleConfiguration.myLanguageLocaleBcp47);
+
+    const QString bcp47 = m_selectedLocaleConfiguration.toBcp47();
+    Calamares::JobQueue::instance()->globalStorage()->insert( "locale", bcp47 );
+    cDebug() << "Updated locale globals, BCP47=" << bcp47;
 
     // If we're in chroot mode (normal install mode), then we immediately set the
     // timezone on the live system.
diff --git a/src/modules/packages/packages.conf b/src/modules/packages/packages.conf
index 38497c2f5..7e5717b7c 100644
--- a/src/modules/packages/packages.conf
+++ b/src/modules/packages/packages.conf
@@ -76,7 +76,7 @@ update_db: true
 #     pre-script: touch /tmp/installing-vi
 #     post-script: rm -f /tmp/installing-vi
 #
-# The pre- and post-scripts are optional, but not both optional: using
+# The pre- and post-scripts are optional, but you cannot leave both out: using
 # "package: vi" with neither script option will trick Calamares into
 # trying to install a package named "package: vi", which is unlikely to work.
 #
@@ -84,11 +84,16 @@ update_db: true
 # packages for software based on the selected system locale. By including
 # the string LOCALE in the package name, the following happens:
 #
-# - if the system locale is English (generally US English; en_GB is a valid
-#   localization), then the package is not installed at all,
-# - otherwise $LOCALE or ${LOCALE} is replaced by the Bcp47 name of the selected
-#   system locale, e.g. nl_BE. Note that just plain LOCALE will not be replaced,
-#   so foo-LOCALE will be unchanged, while foo-$LOCALE will be changed.
+# - if the system locale is English (any variety), then the package is not
+#   installed at all,
+# - otherwise $LOCALE or ${LOCALE} is replaced by the **lower-cased** BCP47
+#   name of the **language** part of the selected system locale (not the
+#   country/region/dialect part), e.g. selecting *nl_BE* will use *nl*
+#   here.
+#
+# Take care that just plain LOCALE will not be replaced, so foo-LOCALE will
+# be left unchanged, while foo-$LOCALE will be changed. However, foo-LOCALE
+# **will** be removed from the list of packages, if English is selected.
 #
 # The following installs localizations for vi, if they are relevant; if
 # there is no localization, installation continues normally.