[netinstall] Chase removal of ItemData

- Simplify creation of PackageTreeItems by interpreting the
  YAML directly (instead of via ItemData),
- Simplify list types,
- Drop superfluous API.

src/modules/netinstall/NetInstallPage.cpp

@@ -167,7 +167,7 @@ NetInstallPage::dataIsHere()
     emit checkReady( true );
 }
 
-PackageModel::PackageItemDataList
+PackageTreeItem::List
 NetInstallPage::selectedPackages() const
 {
     if ( m_groups )
@@ -177,7 +177,7 @@ NetInstallPage::selectedPackages() const
     else
     {
         cWarning() << "no netinstall groups are available.";
-        return PackageModel::PackageItemDataList();
+        return PackageTreeItem::List();
     }
 }
 

src/modules/netinstall/NetInstallPage.h

@@ -74,7 +74,7 @@ public:
     // Returns the list of packages belonging to groups that are
     // selected in the view in this given moment. No data is cached here, so
     // this function does not have constant time.
-    PackageModel::PackageItemDataList selectedPackages() const;
+    PackageTreeItem::List selectedPackages() const;
 
 public slots:
     void dataIsHere();

src/modules/netinstall/NetInstallViewStep.cpp

@@ -127,7 +127,7 @@ NetInstallViewStep::onActivate()
 void
 NetInstallViewStep::onLeave()
 {
-    PackageModel::PackageItemDataList packages = m_widget->selectedPackages();
+    auto packages = m_widget->selectedPackages();
     cDebug() << "Netinstall: Processing" << packages.length() << "packages.";
 
     static const char PACKAGEOP[] = "packageOperations";
@@ -158,13 +158,13 @@ NetInstallViewStep::onLeave()
 
     for ( const auto& package : packages )
     {
-        if ( package.isCritical )
+        if ( package->isCritical() )
         {
-            installPackages.append( package.toOperation() );
+            installPackages.append( package->toOperation() );
         }
         else
         {
-            tryInstallPackages.append( package.toOperation() );
+            tryInstallPackages.append( package->toOperation() );
         }
     }
 

src/modules/netinstall/PackageModel.cpp

@@ -169,29 +169,21 @@ PackageModel::headerData( int section, Qt::Orientation orientation, int role ) c
     return QVariant();
 }
 
-QList< PackageTreeItem::ItemData >
+PackageTreeItem::List
 PackageModel::getPackages() const
 {
-    QList< PackageTreeItem* > items = getItemPackages( m_rootItem );
+    auto items = getItemPackages( m_rootItem );
     for ( auto package : m_hiddenItems )
+    {
         if ( package->hiddenSelected() )
         {
             items.append( getItemPackages( package ) );
         }
-    QList< PackageTreeItem::ItemData > packages;
-    for ( auto item : items )
-    {
-        PackageTreeItem::ItemData itemData;
-        itemData.preScript = item->parentItem()->preScript();  // Only groups have hooks
-        itemData.packageName = item->packageName();  // this seg faults
-        itemData.postScript = item->parentItem()->postScript();  // Only groups have hooks
-        itemData.isCritical = item->parentItem()->isCritical();  // Only groups are critical
-        packages.append( itemData );
     }
-    return packages;
+    return items;
 }
 
-QList< PackageTreeItem* >
+PackageTreeItem::List
 PackageModel::getItemPackages( PackageTreeItem* item ) const
 {
     QList< PackageTreeItem* > selectedPackages;
@@ -232,21 +224,7 @@ PackageModel::setupModelData( const YAML::Node& data, PackageTreeItem* parent )
     for ( YAML::const_iterator it = data.begin(); it != data.end(); ++it )
     {
         const YAML::Node itemDefinition = *it;
-
+        PackageTreeItem* item = new PackageTreeItem( CalamaresUtils::yamlMapToVariant( itemDefinition ), parent );
-        QString name( tr( CalamaresUtils::yamlToVariant( itemDefinition[ "name" ] ).toByteArray() ) );
-        QString description( tr( CalamaresUtils::yamlToVariant( itemDefinition[ "description" ] ).toByteArray() ) );
-
-        PackageTreeItem::ItemData itemData;
-        itemData.name = name;
-        itemData.description = description;
-
-        itemData.preScript = getString( itemDefinition, "pre-install" );
-        itemData.postScript = getString( itemDefinition, "post-install" );
-        itemData.isCritical = getBool( itemDefinition, "critical" );
-        itemData.isHidden = getBool( itemDefinition, "hidden" );
-        itemData.startExpanded = getBool( itemDefinition, "expanded" );
-
-        PackageTreeItem* item = new PackageTreeItem( itemData, parent );
 
         if ( itemDefinition[ "selected" ] )
         {

src/modules/netinstall/PackageModel.h

@@ -37,8 +37,6 @@ class PackageModel : public QAbstractItemModel
     Q_OBJECT
 
 public:
-    using PackageItemDataList = QList< PackageTreeItem::ItemData >;
-
     // Names for columns (unused in the code)
     static constexpr const int NameColumn = 0;
     static constexpr const int DescriptionColumn = 1;
@@ -63,14 +61,14 @@ public:
     int rowCount( const QModelIndex& parent = QModelIndex() ) const override;
     int columnCount( const QModelIndex& parent = QModelIndex() ) const override;
 
-    PackageItemDataList getPackages() const;
+    PackageTreeItem::List getPackages() const;
-    QList< PackageTreeItem* > getItemPackages( PackageTreeItem* item ) const;
+    PackageTreeItem::List getItemPackages( PackageTreeItem* item ) const;
 
 private:
     void setupModelData( const YAML::Node& data, PackageTreeItem* parent );
 
     PackageTreeItem* m_rootItem;
-    QList< PackageTreeItem* > m_hiddenItems;
+    PackageTreeItem::List m_hiddenItems;
 };
 
 #endif  // PACKAGEMODEL_H

src/modules/netinstall/PackageTreeItem.cpp

@@ -20,22 +20,43 @@
 #include "PackageTreeItem.h"
 
 #include "utils/Logger.h"
+#include "utils/Variant.h"
 
-PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* parent )
+static Qt::CheckState
-    : m_parentItem( parent )
+parentCheckState( PackageTreeItem* parent )
 {
-    m_packageName = packageName;
+    if ( parent )
-    if ( parent != nullptr )
     {
         // Avoid partially-checked .. a package can't be partial
-        m_selected = parent->isSelected() == Qt::Unchecked ? Qt::Unchecked : Qt::Checked;
+        return parent->isSelected() == Qt::Unchecked ? Qt::Unchecked : Qt::Checked;
     }
     else
     {
-        m_selected = Qt::Unchecked;
+        return Qt::Unchecked;
     }
 }
 
+PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* parent )
+    : m_parentItem( parent )
+    , m_packageName( packageName )
+    , m_selected( parentCheckState( parent ) )
+{
+}
+
+PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, PackageTreeItem* parent )
+    : m_parentItem( parent )
+    , m_name( CalamaresUtils::getString( groupData, "name" ) )
+    , m_selected( parentCheckState( parent ) )
+    , m_description( CalamaresUtils::getString( groupData, "description" ) )
+    , m_preScript( CalamaresUtils::getString( groupData, "pre-install" ) )
+    , m_postScript( CalamaresUtils::getString( groupData, "post-install" ) )
+    , m_isCritical( CalamaresUtils::getBool( groupData, "critical", false ) )
+    , m_isHidden( CalamaresUtils::getBool( groupData, "hidden", false ) )
+    , m_startExpanded( CalamaresUtils::getBool( groupData, "expanded", false ) )
+{
+}
+
+
 PackageTreeItem::PackageTreeItem::PackageTreeItem()
     : PackageTreeItem( QString(), nullptr )
 {

src/modules/netinstall/PackageTreeItem.h

@@ -27,10 +27,12 @@
 class PackageTreeItem : public QStandardItem
 {
 public:
-    using PackageList = QList< PackageTreeItem* >;
+    using List = QList< PackageTreeItem* >;
 
     ///@brief A package (individual package)
     explicit PackageTreeItem( const QString& packageName, PackageTreeItem* parent = nullptr );
+    ///@brief A group (sub-items and sub-groups are ignored)
+    explicit PackageTreeItem( const QVariantMap& groupData, PackageTreeItem* parent = nullptr );
     ///@brief A root item, always selected, named "<root>"
     explicit PackageTreeItem();
     ~PackageTreeItem() override;
@@ -81,6 +83,12 @@ public:
      */
     bool expandOnStart() const { return m_startExpanded; }
 
+    /** @brief is this item selected?
+     *
+     * Groups may be partially selected; packages are only on or off.
+     */
+    Qt::CheckState isSelected() const { return m_selected; }
+
     /** @brief Turns this item into a variant for PackageOperations use
      *
      * For "plain" items, this is just the package name; items with
@@ -88,7 +96,6 @@ public:
      */
     QVariant toOperation() const;
 
-    Qt::CheckState isSelected() const;
     void setSelected( Qt::CheckState isSelected );
     void setChildrenSelected( Qt::CheckState isSelected );
 
@@ -97,7 +104,7 @@ public:
 
 private:
     PackageTreeItem* m_parentItem;
-    PackageList m_childItems;
+    List m_childItems;
 
     // An entry can be a packkage, or a group.
     QString m_name;