diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp index 556cb1cf9..062518221 100644 --- a/src/modules/netinstall/Config.cpp +++ b/src/modules/netinstall/Config.cpp @@ -109,7 +109,7 @@ Config::receivedGroupData() cDebug() << "NetInstall group data received" << m_reply->size() << "bytes from" << m_reply->url(); - cqDeleter< QNetworkReply > d{ m_reply }; + cqDeleter< QNetworkReply > d { m_reply }; // If m_required is *false* then we still say we're ready // even if the reply is corrupt or missing. @@ -128,11 +128,23 @@ Config::receivedGroupData() { YAML::Node groups = YAML::Load( yamlData.constData() ); - if ( !groups.IsSequence() ) + if ( groups.IsSequence() ) + { + loadGroupList( CalamaresUtils::yamlSequenceToVariant( groups ) ); + } + else if ( groups.IsMap() ) + { + auto map = CalamaresUtils::yamlMapToVariant( groups ); + loadGroupList( map.value( "groups" ).toList() ); + } + else { cWarning() << "NetInstall groups data does not form a sequence."; } - loadGroupList( CalamaresUtils::yamlSequenceToVariant( groups ) ); + if ( m_model->rowCount() < 1 ) + { + cWarning() << "NetInstall groups data was empty."; + } } catch ( YAML::Exception& e ) { diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp index dad2207b2..dd5047129 100644 --- a/src/modules/netinstall/PackageModel.cpp +++ b/src/modules/netinstall/PackageModel.cpp @@ -232,16 +232,27 @@ PackageModel::setupModelData( const QVariantList& groupList, PackageTreeItem* pa continue; } - PackageTreeItem* item = new PackageTreeItem( groupMap, parent ); + PackageTreeItem* item = new PackageTreeItem( groupMap, PackageTreeItem::GroupTag { parent } ); if ( groupMap.contains( "selected" ) ) { item->setSelected( CalamaresUtils::getBool( groupMap, "selected", false ) ? Qt::Checked : Qt::Unchecked ); } if ( groupMap.contains( "packages" ) ) { - for ( const auto& packageName : groupMap.value( "packages" ).toStringList() ) + for ( const auto& packageName : groupMap.value( "packages" ).toList() ) { - item->appendChild( new PackageTreeItem( packageName, item ) ); + if ( packageName.type() == QVariant::String ) + { + item->appendChild( new PackageTreeItem( packageName.toString(), item ) ); + } + else + { + QVariantMap m = packageName.toMap(); + if ( !m.isEmpty() ) + { + item->appendChild( new PackageTreeItem( m, PackageTreeItem::PackageTag { item } ) ); + } + } } } if ( groupMap.contains( "subgroups" ) ) diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp index edc89c536..30a57ac2a 100644 --- a/src/modules/netinstall/PackageTreeItem.cpp +++ b/src/modules/netinstall/PackageTreeItem.cpp @@ -61,15 +61,26 @@ PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* p { } -PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, PackageTreeItem* parent ) - : m_parentItem( parent ) +PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, PackageTag&& parent ) + : m_parentItem( parent.parent ) + , m_packageName( CalamaresUtils::getString( groupData, "name" ) ) + , m_selected( parentCheckState( parent.parent ) ) + , m_description( CalamaresUtils::getString( groupData, "description" ) ) + , m_isGroup( false ) + , m_isCritical( parent.parent ? parent.parent->isCritical() : false ) + , m_showReadOnly( parent.parent ? parent.parent->isImmutable() : false ) +{ +} + +PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, GroupTag&& parent ) + : m_parentItem( parent.parent ) , m_name( CalamaresUtils::getString( groupData, "name" ) ) - , m_selected( parentCheckState( parent ) ) + , m_selected( parentCheckState( parent.parent ) ) , m_description( CalamaresUtils::getString( groupData, "description" ) ) , m_preScript( CalamaresUtils::getString( groupData, "pre-install" ) ) , m_postScript( CalamaresUtils::getString( groupData, "post-install" ) ) , m_isGroup( true ) - , m_isCritical( parentCriticality( groupData, parent ) ) + , m_isCritical( parentCriticality( groupData, parent.parent ) ) , m_isHidden( CalamaresUtils::getBool( groupData, "hidden", false ) ) , m_showReadOnly( CalamaresUtils::getBool( groupData, "immutable", false ) ) , m_startExpanded( CalamaresUtils::getBool( groupData, "expanded", false ) ) @@ -120,27 +131,16 @@ PackageTreeItem::row() const QVariant PackageTreeItem::data( int column ) const { - if ( isPackage() ) // packages have a packagename, groups don't + switch ( column ) { - switch ( column ) - { - case 0: - return QVariant( packageName() ); - default: - return QVariant(); - } - } - else - { - switch ( column ) // group - { - case 0: - return QVariant( name() ); - case 1: - return QVariant( description() ); - default: - return QVariant(); - } + case 0: + // packages have a packagename, groups don't + return QVariant( isPackage() ? packageName() : name() ); + case 1: + // packages often have a blank description + return QVariant( description() ); + default: + return QVariant(); } } diff --git a/src/modules/netinstall/PackageTreeItem.h b/src/modules/netinstall/PackageTreeItem.h index d443bcdc6..0b2d506d7 100644 --- a/src/modules/netinstall/PackageTreeItem.h +++ b/src/modules/netinstall/PackageTreeItem.h @@ -29,10 +29,23 @@ class PackageTreeItem : public QStandardItem public: using List = QList< PackageTreeItem* >; + ///@brief A tag class to distinguish package-from-map from group-from-map + struct PackageTag + { + PackageTreeItem* parent; + }; + ///@brief A tag class to distinguish group-from-map from package-from-map + struct GroupTag + { + PackageTreeItem* parent; + }; + ///@brief A package (individual package) explicit PackageTreeItem( const QString& packageName, PackageTreeItem* parent = nullptr ); + ///@brief A package (individual package with description) + explicit PackageTreeItem( const QVariantMap& packageData, PackageTag&& parent ); ///@brief A group (sub-items and sub-groups are ignored) - explicit PackageTreeItem( const QVariantMap& groupData, PackageTreeItem* parent = nullptr ); + explicit PackageTreeItem( const QVariantMap& groupData, GroupTag&& parent ); ///@brief A root item, always selected, named "" explicit PackageTreeItem(); ~PackageTreeItem() override; diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp index cfaf20efa..c74e6aafe 100644 --- a/src/modules/netinstall/Tests.cpp +++ b/src/modules/netinstall/Tests.cpp @@ -41,7 +41,10 @@ private Q_SLOTS: void initTestCase(); void testRoot(); + void testPackage(); + void testExtendedPackage(); + void testGroup(); void testCompare(); void testModel(); @@ -77,6 +80,7 @@ ItemTests::testPackage() QCOMPARE( p.isSelected(), Qt::Unchecked ); QCOMPARE( p.packageName(), QStringLiteral( "bash" ) ); QVERIFY( p.name().isEmpty() ); // not a group + QVERIFY( p.description().isEmpty() ); QCOMPARE( p.parentItem(), nullptr ); QCOMPARE( p.childCount(), 0 ); QVERIFY( !p.isHidden() ); @@ -126,6 +130,33 @@ static const char doc_with_expanded[] = // *INDENT-ON* // clang-format on +void +ItemTests::testExtendedPackage() +{ + YAML::Node yamldoc = YAML::Load( doc ); + QVariantList yamlContents = CalamaresUtils::yamlSequenceToVariant( yamldoc ); + + QCOMPARE( yamlContents.length(), 1 ); + + // Kind of derpy, but we can treat a group as if it is a package + // because the keys name and description are the same + PackageTreeItem p( yamlContents[ 0 ].toMap(), PackageTreeItem::PackageTag { nullptr } ); + + QCOMPARE( p.isSelected(), Qt::Unchecked ); + QCOMPARE( p.packageName(), QStringLiteral( "CCR" ) ); + QVERIFY( p.name().isEmpty() ); // not a group + QVERIFY( !p.description().isEmpty() ); // because it is set + QVERIFY( p.description().startsWith( QStringLiteral( "Tools for the Chakra" ) ) ); + QCOMPARE( p.parentItem(), nullptr ); + QCOMPARE( p.childCount(), 0 ); + QVERIFY( !p.isHidden() ); + QVERIFY( !p.isCritical() ); + QVERIFY( !p.isGroup() ); + QVERIFY( p.isPackage() ); + QVERIFY( p == p ); +} + + void ItemTests::testGroup() { @@ -134,7 +165,7 @@ ItemTests::testGroup() QCOMPARE( yamlContents.length(), 1 ); - PackageTreeItem p( yamlContents[ 0 ].toMap(), nullptr ); + PackageTreeItem p( yamlContents[ 0 ].toMap(), PackageTreeItem::GroupTag { nullptr } ); QCOMPARE( p.name(), QStringLiteral( "CCR" ) ); QVERIFY( p.packageName().isEmpty() ); QVERIFY( p.description().startsWith( QStringLiteral( "Tools " ) ) ); @@ -147,7 +178,7 @@ ItemTests::testGroup() QVERIFY( !p.isPackage() ); QVERIFY( p == p ); - PackageTreeItem c( "zsh", nullptr ); + PackageTreeItem c( "zsh", nullptr ); // Single string, package QVERIFY( p != c ); } @@ -184,15 +215,17 @@ ItemTests::testCompare() QVariantList yamlContents = CalamaresUtils::yamlSequenceToVariant( yamldoc ); QCOMPARE( yamlContents.length(), 1 ); - PackageTreeItem p3( yamlContents[ 0 ].toMap(), nullptr ); + PackageTreeItem p3( yamlContents[ 0 ].toMap(), PackageTreeItem::GroupTag { nullptr } ); QVERIFY( p3 == p3 ); QVERIFY( p3 != p1 ); QVERIFY( p1 != p3 ); QCOMPARE( p3.childCount(), 0 ); // Doesn't load the packages: list - PackageTreeItem p4( CalamaresUtils::yamlSequenceToVariant( YAML::Load( doc ) )[ 0 ].toMap(), nullptr ); + PackageTreeItem p4( CalamaresUtils::yamlSequenceToVariant( YAML::Load( doc ) )[ 0 ].toMap(), + PackageTreeItem::GroupTag { nullptr } ); QVERIFY( p3 == p4 ); - PackageTreeItem p5( CalamaresUtils::yamlSequenceToVariant( YAML::Load( doc_no_packages ) )[ 0 ].toMap(), nullptr ); + PackageTreeItem p5( CalamaresUtils::yamlSequenceToVariant( YAML::Load( doc_no_packages ) )[ 0 ].toMap(), + PackageTreeItem::GroupTag { nullptr } ); QVERIFY( p3 == p5 ); } diff --git a/src/modules/netinstall/netinstall.conf b/src/modules/netinstall/netinstall.conf index 24f4f201d..7de577877 100644 --- a/src/modules/netinstall/netinstall.conf +++ b/src/modules/netinstall/netinstall.conf @@ -50,9 +50,11 @@ # # The format of the groups file is the same as the format of the # *groups* key described below, **except** that a stand-alone -# groups file does not have the top-level *groups* key. -# -# TODO: remove that ^^ restriction +# groups file does not have to have the top-level *groups* key. +# (It **may** have one, though, for instance when you copy +# this configuration file to `netinstall.yaml` and key *groups* +# must have a list-of-groups as value; if the file does not have +# a top-level key *groups*, then the file must contain only a list of groups. # # As a special case, setting *groupsUrl* to the literal string # `local` means that the data is obtained from **this** config @@ -118,9 +120,8 @@ label: # groups data is read from this file. The value of *groups* must be # a list. Each item in the list is a group (of packages, or subgroups, # or both). A standalone groups file contains just the list, -# without the top-level *groups* key. -# -# TODO: remove that restriction ^^ +# (without the top-level *groups* key, or just the top-level *groups* +# key and with the list as its value, like in this file). # # Using `local` is recommended only for small static package lists. # Here it is used for documentation purposes. @@ -255,7 +256,9 @@ groups: - konversation - nheko - quaternion - # Setting *selected* is supported. + # Setting *selected* is supported. Here we also show off "rich" + # packages: ones with a package-name (for the package-manager) + # and a description (for the human). - name: Editors description: "Editing" selected: false @@ -263,6 +266,10 @@ groups: - vi - emacs - nano + - name: kate-git + description: Kate (unstable) + - name: kate + description: KDE's text editor # The "bare" package names can be intimidating, so you can use subgroups # to provide human-readable names while hiding the packages themselves. # This also allows you you group related packages -- suppose you feel