Merge branch 'issue-1369'

FIXES #1369 - Rich package descriptions were easy to do; added some tests as well; also make it viable to copy a once-tested "local" file to the downloadable YAML format.
2020-04-16 23:26:15 +02:00 · 2020-04-16 23:26:15 +02:00 · 8304ce93bf
commit 8304ce93bf
parent 39ed591414 a4c5e18c4b
6 changed files with 119 additions and 43 deletions
--- 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 )
    {
--- 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" ) )
--- 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();
    }
 }

--- 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 "<root>"
    explicit PackageTreeItem();
    ~PackageTreeItem() override;
--- 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 );
 }

--- 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