From e311d7a89346ad61ee10a2da48cb1a7a76559725 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 3 Sep 2021 23:59:11 +0200 Subject: [PATCH] [packagechooser] Remove 'id' configuration setting - Setting 'id' (which changes the Global Storage key that gets used) is a kludge when the existing module-instance name can be used instead -- and **was** already used, as a fallback when 'id' is not set. There's no point in having two places to set a particular name. - Rip out the docs for 'id' as well. - Add documentation on the difference between single-selection (the QML implementation) and model-selection (what the Widgets version does). --- src/modules/packagechooser/Config.cpp | 35 ++++++------------- src/modules/packagechooser/Config.h | 2 -- .../packagechooser/packagechooser.conf | 12 +++---- .../packagechooserq/packagechooserq.conf | 31 ++++++++-------- 4 files changed, 29 insertions(+), 51 deletions(-) diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp index 0001d9fc6..5c0db5d91 100644 --- a/src/modules/packagechooser/Config.cpp +++ b/src/modules/packagechooser/Config.cpp @@ -95,6 +95,12 @@ Config::introductionPackage() const return *defaultIntroduction; } +static inline QString +make_gs_key( const Calamares::ModuleSystem::InstanceKey& key ) +{ + return QStringLiteral( "packagechooser_" ) + key.id(); +} + void Config::updateGlobalStorage( const QStringList& selected ) const { @@ -105,8 +111,8 @@ Config::updateGlobalStorage( const QStringList& selected ) const if ( m_method == PackageChooserMethod::Legacy ) { QString value = selected.join( ',' ); - Calamares::JobQueue::instance()->globalStorage()->insert( m_id, value ); - cDebug() << m_id << "selected" << value; + Calamares::JobQueue::instance()->globalStorage()->insert( make_gs_key( m_defaultId ), value ); + cDebug() << m_defaultId << "selected" << value; } else if ( m_method == PackageChooserMethod::Packages ) { @@ -133,11 +139,11 @@ Config::updateGlobalStorage() const auto* gs = Calamares::JobQueue::instance()->globalStorage(); if ( m_packageChoice.has_value() ) { - gs->insert( m_id, m_packageChoice.value() ); + gs->insert( make_gs_key( m_defaultId ), m_packageChoice.value() ); } else { - gs->remove( m_id ); + gs->remove( make_gs_key( m_defaultId ) ); } } else if ( m_method == PackageChooserMethod::Packages ) @@ -240,26 +246,7 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) if ( m_method == PackageChooserMethod::Legacy ) { - const QString configId = CalamaresUtils::getString( configurationMap, "id" ); - const QString base = QStringLiteral( "packagechooser_" ); - if ( configId.isEmpty() ) - { - if ( m_defaultId.id().isEmpty() ) - { - // We got nothing to work with - m_id = base; - } - else - { - m_id = base + m_defaultId.id(); - } - cDebug() << "Using default ID" << m_id << "from" << m_defaultId.toString(); - } - else - { - m_id = base + configId; - cDebug() << "Using configured ID" << m_id; - } + cDebug() << "Using module ID" << m_defaultId; } if ( configurationMap.contains( "items" ) ) diff --git a/src/modules/packagechooser/Config.h b/src/modules/packagechooser/Config.h index 85b8de83f..32f1e8b57 100644 --- a/src/modules/packagechooser/Config.h +++ b/src/modules/packagechooser/Config.h @@ -110,8 +110,6 @@ private: PackageChooserMode m_mode = PackageChooserMode::Optional; /// How this module stores to GS PackageChooserMethod m_method = PackageChooserMethod::Legacy; - /// Id (used to identify settings from this module in GS) - QString m_id; /// Value to use for id if none is set in the config file Calamares::ModuleSystem::InstanceKey m_defaultId; /** @brief QML selection (for single-selection approaches) diff --git a/src/modules/packagechooser/packagechooser.conf b/src/modules/packagechooser/packagechooser.conf index 2bde1369c..231826cd3 100644 --- a/src/modules/packagechooser/packagechooser.conf +++ b/src/modules/packagechooser/packagechooser.conf @@ -15,11 +15,10 @@ mode: required # # - "legacy" or "custom" or "contextualprocess" # When set to "legacy", writes a GlobalStorage value for the choice that -# has been made. The key is *packagechooser_*. Normally, the module's +# has been made. The key is *packagechooser_*. The module's # instance name is used; see the *instances* section of `settings.conf`. # If there is just one packagechooser module, and no special instance is set, -# resulting GS key is probably *packagechooser@packagechooser*. -# You can set *id* to change that, but it is not recommended. +# resulting GS key is probably *packagechooser_packagechooser*. # # The GS value is a comma-separated list of the IDs of the selected # packages, or an empty string if none is selected. @@ -33,15 +32,12 @@ mode: required # consumption by the *packages* module (which should appear later # in the `exec` section. These package settings will then be handed # off to whatever package manager is configured there. -# The *id* key is not used. # # There is no need to put this module in the `exec` section. There # are no jobs that this module provides. You should put **other** # modules, either *contextualprocess* or *packages* or some custom # module, in the `exec` section to do the actual work. method: legacy -# The *id* key is used only in "legacy" mode -# id: "" # Human-visible strings in this module. These are all optional. @@ -51,13 +47,13 @@ method: legacy # Each key can have a [locale] added to it, which is used as # the translated string for that locale. For the strings # associated with the "no-selection" item, see *items*, below -# with the explicit id "". +# with the explicit item-*id* "". # labels: step: "Packages" step[nl]: "Pakketten" -# (Optional) 'id' of pre-selected list-view item. +# (Optional) item-*id* of pre-selected list-view item. # Pre-selects one of the items below. # default: kde diff --git a/src/modules/packagechooserq/packagechooserq.conf b/src/modules/packagechooserq/packagechooserq.conf index 8fd3a08f6..803c6f670 100644 --- a/src/modules/packagechooserq/packagechooserq.conf +++ b/src/modules/packagechooserq/packagechooserq.conf @@ -1,23 +1,26 @@ # SPDX-FileCopyrightText: no # SPDX-License-Identifier: CC0-1.0 # -# Configuration for the low-density software chooser ---- -# Software selection mode, to set whether the software packages -# can be chosen singly, or multiply. +# Configuration for the low-density software chooser, QML implementation +# +# The example QML implementation uses single-selection, rather than +# a model for the available packages. That makes it simpler: the +# QML itself codes the available options, descriptions and images +# -- after all, this is **low density** selection, so a custom UI +# can make sense for the few choices that need to be made. +# # -# The example QML module does not use a model, and ignores this value. -mode: required +--- # Software installation method: # # - "legacy" or "custom" or "contextualprocess" # When set to "legacy", writes a GlobalStorage value for the choice that -# has been made. The key is *packagechooser_*. Normally, the module's +# has been made. The key is *packagechooser_*. The module's # instance name is used; see the *instances* section of `settings.conf`. -# If there is just one packagechooser module, and no special instance is set, -# resulting GS key is probably *packagechooser@packagechooser*. -# You can set *id* to change that, but it is not recommended. +# If there is just one packagechooserq module, and no special instance is set, +# resulting GS key is probably *packagechooser_packagechooserq*. +# (Do note that the prefix of the GS key remains "packagechooser_") # # The GS value is a comma-separated list of the IDs of the selected # packages, or an empty string if none is selected. @@ -31,19 +34,13 @@ mode: required # consumption by the *packages* module (which should appear later # in the `exec` section. These package settings will then be handed # off to whatever package manager is configured there. -# The *id* key is not used. # # There is no need to put this module in the `exec` section. There # are no jobs that this module provides. You should put **other** # modules, either *contextualprocess* or *packages* or some custom # module, in the `exec` section to do the actual work. -method: legacy - -# The *id* key is used only in "legacy" mode and changes the GlobalStorage -# key used to store the package choices. It is not recommended to use -# this, since the module instance key does the same job. # -# id: "" +method: legacy # The *packageChoice* value is used for setting the default selection # in the QML view; this should match one of the keys used in the QML