[keyboard] Re-do the keyboard physical models model from scratch

src/modules/keyboard/Config.cpp

@@ -50,22 +50,25 @@ xkbmap_layout_args( const QString& layout, const QString& variant )
 }
 
 static inline QStringList
-xkbmap_layout_args( const QStringList& layouts, const QStringList& variants, const QString& switchOption = "grp:alt_shift_toggle")
+xkbmap_layout_args( const QStringList& layouts,
+                    const QStringList& variants,
+                    const QString& switchOption = "grp:alt_shift_toggle" )
 {
     if ( layouts.size() != variants.size() )
     {
-        cError() << "Number of layouts and variants must be equal (empty string should be used if there is no corresponding variant)";
+        cError() << "Number of layouts and variants must be equal (empty string should be used if there is no "
+                    "corresponding variant)";
         return QStringList();
     }
 
-    QStringList r{ "-layout", layouts.join( "," ) };
+    QStringList r { "-layout", layouts.join( "," ) };
 
     if ( !variants.isEmpty() )
     {
         r << "-variant" << variants.join( "," );
     }
 
-    if ( !switchOption.isEmpty())
+    if ( !switchOption.isEmpty() )
     {
         r << "-option" << switchOption;
     }
@@ -88,17 +91,16 @@ xkbmap_query_grp_option()
     do
     {
         outputLine = setxkbmapQuery.readLine();
-    }
+    } while ( setxkbmapQuery.canReadLine() && !outputLine.startsWith( "options:" ) );
-    while( setxkbmapQuery.canReadLine() && !outputLine.startsWith("options:") );
 
-    if( !outputLine.startsWith( "options:" ) )
+    if ( !outputLine.startsWith( "options:" ) )
     {
         return QString();
     }
 
     int index = outputLine.indexOf( "grp:" );
 
-    if( index == -1 )
+    if ( index == -1 )
     {
         return QString();
     }
@@ -106,14 +108,16 @@ xkbmap_query_grp_option()
     //it's either in the end of line or before the other option so \s or ,
     int lastIndex = outputLine.indexOf( QRegExp( "[\\s,]" ), index );
 
-    return outputLine.mid( index, lastIndex-1 );
+    return outputLine.mid( index, lastIndex - 1 );
 }
 
-AdditionalLayoutInfo Config::getAdditionalLayoutInfo( const QString &layout )
+AdditionalLayoutInfo
+Config::getAdditionalLayoutInfo( const QString& layout )
 {
     QFile layoutTable( ":/non-ascii-layouts" );
 
-    if( !layoutTable.open( QIODevice::ReadOnly | QIODevice::Text ) ) {
+    if ( !layoutTable.open( QIODevice::ReadOnly | QIODevice::Text ) )
+    {
         cError() << "Non-ASCII layout table could not be opened";
         return AdditionalLayoutInfo();
     }
@@ -123,10 +127,9 @@ AdditionalLayoutInfo Config::getAdditionalLayoutInfo( const QString &layout )
     do
     {
         tableLine = layoutTable.readLine();
-    }
+    } while ( layoutTable.canReadLine() && !tableLine.startsWith( layout ) );
-    while( layoutTable.canReadLine() && !tableLine.startsWith( layout ) );
 
-    if( !tableLine.startsWith( layout ) )
+    if ( !tableLine.startsWith( layout ) )
     {
         return AdditionalLayoutInfo();
     }
@@ -135,10 +138,10 @@ AdditionalLayoutInfo Config::getAdditionalLayoutInfo( const QString &layout )
 
     AdditionalLayoutInfo r;
 
-    r.additionalLayout = tableEntries[1];
+    r.additionalLayout = tableEntries[ 1 ];
-    r.additionalVariant = tableEntries[2] == "-" ? "" : tableEntries[2];
+    r.additionalVariant = tableEntries[ 2 ] == "-" ? "" : tableEntries[ 2 ];
 
-    r.vconsoleKeymap = tableEntries[3];
+    r.vconsoleKeymap = tableEntries[ 3 ];
 
     return r;
 }
@@ -153,7 +156,7 @@ Config::Config( QObject* parent )
 
     // Connect signals and slots
     connect( m_keyboardModelsModel, &KeyboardModelsModel::currentIndexChanged, [&]( int index ) {
-        m_selectedModel = m_keyboardModelsModel->item( index ).value( "key", "pc105" );
+        m_selectedModel = m_keyboardModelsModel->modelKey( index );
         //                      Set Xorg keyboard model
         QProcess::execute( "setxkbmap", xkbmap_model_args( m_selectedModel ) );
         emit prettyStatusChanged();
@@ -182,23 +185,20 @@ Config::Config( QObject* parent )
             {
                 m_additionalLayoutInfo.groupSwitcher = xkbmap_query_grp_option();
 
-                if( m_additionalLayoutInfo.groupSwitcher.isEmpty() )
+                if ( m_additionalLayoutInfo.groupSwitcher.isEmpty() )
                 {
                     m_additionalLayoutInfo.groupSwitcher = "grp:alt_shift_toggle";
                 }
 
-                QProcess::execute( "setxkbmap", xkbmap_layout_args(
+                QProcess::execute( "setxkbmap",
-                                       { m_additionalLayoutInfo.additionalLayout, m_selectedLayout },
+                                   xkbmap_layout_args( { m_additionalLayoutInfo.additionalLayout, m_selectedLayout },
-                                       { m_additionalLayoutInfo.additionalVariant, m_selectedVariant },
+                                                       { m_additionalLayoutInfo.additionalVariant, m_selectedVariant },
-                                       m_additionalLayoutInfo.groupSwitcher )
+                                                       m_additionalLayoutInfo.groupSwitcher ) );
-                                   );
 
 
-
+                cDebug() << "xkbmap selection changed to: " << m_selectedLayout << '-' << m_selectedVariant << "(added "
-                cDebug() << "xkbmap selection changed to: " << m_selectedLayout << '-' << m_selectedVariant
+                         << m_additionalLayoutInfo.additionalLayout << "-" << m_additionalLayoutInfo.additionalVariant
-                         << "(added " << m_additionalLayoutInfo.additionalLayout << "-"
+                         << " since current layout is not ASCII-capable)";
-                         << m_additionalLayoutInfo.additionalVariant << " since current layout is not ASCII-capable)";
-
             }
             else
             {
@@ -269,15 +269,15 @@ Config::detectCurrentKeyboardLayout()
                 continue;
             }
 
-            int firstQuote = line.indexOf('"');
+            int firstQuote = line.indexOf( '"' );
-            int lastQuote = line.lastIndexOf('"');
+            int lastQuote = line.lastIndexOf( '"' );
 
-            if (firstQuote < 0 || lastQuote < 0 || lastQuote <= firstQuote)
+            if ( firstQuote < 0 || lastQuote < 0 || lastQuote <= firstQuote )
             {
                 continue;
             }
 
-            QStringList split = line.mid(firstQuote+1, lastQuote-firstQuote).split( "+", SplitSkipEmptyParts );
+            QStringList split = line.mid( firstQuote + 1, lastQuote - firstQuote ).split( "+", SplitSkipEmptyParts );
             cDebug() << split;
             if ( split.size() >= 2 )
             {
@@ -324,7 +324,7 @@ Config::prettyStatus() const
 {
     QString status;
     status += tr( "Set keyboard model to %1.<br/>" )
-                  .arg( m_keyboardModelsModel->item( m_keyboardModelsModel->currentIndex() )[ "label" ] );
+                  .arg( m_keyboardModelsModel->modelLabel( m_keyboardModelsModel->currentIndex() ) );
 
     QString layout = m_keyboardLayoutsModel->item( m_keyboardLayoutsModel->currentIndex() ).second.description;
     QString variant = m_keyboardVariantsModel->currentIndex() >= 0
@@ -381,11 +381,11 @@ Config::guessLayout( const QStringList& langParts )
                 for ( int variantnumber = 0; variantnumber < m_keyboardVariantsModel->rowCount(); ++variantnumber )
                 {
                     if ( m_keyboardVariantsModel->item( variantnumber )[ "key" ].compare( *countryPart,
-                                                                                          Qt::CaseInsensitive ) == 0 )
+                                                                                          Qt::CaseInsensitive )
+                         == 0 )
                     {
                         m_keyboardVariantsModel->setCurrentIndex( variantnumber );
-                        cDebug() << Logger::SubEntry << "matched variant"
+                        cDebug() << Logger::SubEntry << "matched variant" << *countryPart << ' '
-                                 << *countryPart << ' '
                                  << m_keyboardVariantsModel->item( variantnumber )[ "key" ];
                     }
                 }
@@ -497,8 +497,8 @@ Config::finalize()
 
         if ( !m_additionalLayoutInfo.additionalLayout.isEmpty() )
         {
-            gs->insert( "keyboardAdditionalLayout", m_additionalLayoutInfo.additionalLayout);
+            gs->insert( "keyboardAdditionalLayout", m_additionalLayoutInfo.additionalLayout );
-            gs->insert( "keyboardAdditionalLayout", m_additionalLayoutInfo.additionalVariant);
+            gs->insert( "keyboardAdditionalLayout", m_additionalLayoutInfo.additionalVariant );
             gs->insert( "keyboardVConsoleKeymap", m_additionalLayoutInfo.vconsoleKeymap );
         }
     }

src/modules/keyboard/KeyboardLayoutModel.cpp

@@ -10,8 +10,96 @@
 
 #include "KeyboardLayoutModel.h"
 
+#include "utils/Logger.h"
+
 #include <algorithm>
 
+KeyboardModelsModel::KeyboardModelsModel( QObject* parent )
+    : QAbstractListModel( parent )
+{
+    // The models map is from human-readable names (!) to xkb identifier
+    const auto models = KeyboardGlobal::getKeyboardModels();
+    for ( const auto& key : models.keys() )
+    {
+        // So here *key* is the key in the map, which is the human-readable thing,
+        //   while the struct fields are xkb-id, and human-readable
+        m_list << ModelInfo { models[ key ], key };
+    }
+
+    cDebug() << "Loaded" << m_list.count() << "keyboard models";
+}
+
+int
+KeyboardModelsModel::rowCount( const QModelIndex& ) const
+{
+    return m_list.count();
+}
+
+QVariant
+KeyboardModelsModel::data( const QModelIndex& index, int role ) const
+{
+    if ( !index.isValid() )
+    {
+        return QVariant();
+    }
+    if ( index.row() < 0 || index.row() >= m_list.count() )
+    {
+        return QVariant();
+    }
+
+    const auto item = m_list.at( index.row() );
+    switch ( role )
+    {
+    case LabelRole:
+        return item.label;
+    case KeyRole:
+        return item.key;
+    default:
+        return QVariant();
+    }
+    __builtin_unreachable();
+}
+
+QString
+KeyboardModelsModel::modelKey( int index ) const
+{
+    if ( index < 0 || index >= m_list.count() )
+    {
+        return QString();
+    }
+    return m_list[ index ].key;
+}
+
+QString
+KeyboardModelsModel::modelLabel( int index ) const
+{
+    if ( index < 0 || index >= m_list.count() )
+    {
+        return QString();
+    }
+    return m_list[ index ].label;
+}
+
+QHash< int, QByteArray >
+KeyboardModelsModel::roleNames() const
+{
+    return { { Qt::DisplayRole, "label" }, { Qt::UserRole, "key" } };
+}
+
+void
+KeyboardModelsModel::setCurrentIndex( int index )
+{
+    if ( index >= m_list.count() || index < 0 )
+    {
+        return;
+    }
+    if ( m_currentIndex != index )
+    {
+        m_currentIndex = index;
+        emit currentIndexChanged( m_currentIndex );
+    }
+}
+
 
 KeyboardLayoutModel::KeyboardLayoutModel( QObject* parent )
     : QAbstractListModel( parent )
@@ -100,77 +188,6 @@ KeyboardLayoutModel::currentIndex() const
     return m_currentIndex;
 }
 
-KeyboardModelsModel::KeyboardModelsModel( QObject* parent )
-    : QAbstractListModel( parent )
-{
-    detectModels();
-}
-
-void
-KeyboardModelsModel::detectModels()
-{
-    beginResetModel();
-    const auto models = KeyboardGlobal::getKeyboardModels();
-    auto index = -1;
-    for ( const auto& key : models.keys() )
-    {
-        index++;
-        m_list << QMap< QString, QString > { { "label", key }, { "key", models[ key ] } };
-        if ( models[ key ] == "pc105" )
-        {
-            this->setCurrentIndex( index );
-        }
-    }
-    endResetModel();
-}
-
-void
-KeyboardModelsModel::refresh()
-{
-    m_list.clear();
-    setCurrentIndex( -1 );
-    detectModels();
-}
-
-QVariant
-KeyboardModelsModel::data( const QModelIndex& index, int role ) const
-{
-    if ( !index.isValid() )
-    {
-        return QVariant();
-    }
-    const auto item = m_list.at( index.row() );
-    return role == Qt::DisplayRole ? item[ "label" ] : item[ "key" ];
-}
-
-int
-KeyboardModelsModel::rowCount( const QModelIndex& ) const
-{
-    return m_list.count();
-}
-
-QHash< int, QByteArray >
-KeyboardModelsModel::roleNames() const
-{
-    return { { Qt::DisplayRole, "label" }, { Qt::UserRole, "key" } };
-}
-
-int
-KeyboardModelsModel::currentIndex() const
-{
-    return m_currentIndex;
-}
-
-const QMap< QString, QString >
-KeyboardModelsModel::item( const int& index ) const
-{
-    if ( index >= m_list.count() || index < 0 )
-    {
-        return QMap< QString, QString >();
-    }
-
-    return m_list.at( index );
-}
 
 const QMap< QString, QString >
 KeyboardVariantsModel::item( const int& index ) const
@@ -183,18 +200,6 @@ KeyboardVariantsModel::item( const int& index ) const
     return m_list.at( index );
 }
 
-void
-KeyboardModelsModel::setCurrentIndex( const int& index )
-{
-    if ( index >= m_list.count() || index < 0 )
-    {
-        return;
-    }
-
-    m_currentIndex = index;
-    emit currentIndexChanged( m_currentIndex );
-}
-
 KeyboardVariantsModel::KeyboardVariantsModel( QObject* parent )
     : QAbstractListModel( parent )
 {
@@ -253,4 +258,3 @@ KeyboardVariantsModel::setVariants( QMap< QString, QString > variants )
     }
     endResetModel();
 }
-

src/modules/keyboard/KeyboardLayoutModel.h

@@ -17,6 +17,60 @@
 #include <QMetaType>
 #include <QObject>
 
+/** @brief A list model of the physical keyboard formats ("models" in xkb)
+ *
+ * This model acts like it has a single selection, as well.
+ */
+class KeyboardModelsModel : public QAbstractListModel
+{
+    Q_OBJECT
+    Q_PROPERTY( int currentIndex WRITE setCurrentIndex READ currentIndex NOTIFY currentIndexChanged )
+
+public:
+    enum
+    {
+        LabelRole = Qt::DisplayRole,  ///< Human-readable
+        KeyRole = Qt::UserRole  ///< xkb identifier
+    };
+
+    explicit KeyboardModelsModel( QObject* parent = nullptr );
+
+    int rowCount( const QModelIndex& ) const override;
+    QVariant data( const QModelIndex& index, int role ) const override;
+    /** @brief xkb key for a given index (row)
+     *
+     * This is like calling data( QModelIndex( index ), KeyRole ).toString(),
+     * but shorter and faster. Can return an empty string if index is invalid.
+     */
+    QString modelKey( int index ) const;
+
+    /** @brief human-readable label for a given index (row)
+     *
+     * This is like calling data( QModelIndex( index ), LabelRole ).toString(),
+     * but shorter and faster. Can return an empty string if index is invalid.
+     */
+    QString modelLabel( int index ) const;
+
+    QHash< int, QByteArray > roleNames() const override;
+
+    void setCurrentIndex( int index );
+    int currentIndex() const { return m_currentIndex; }
+
+signals:
+    void currentIndexChanged( int index );
+
+private:
+    struct ModelInfo
+    {
+        /// XKB identifier
+        QString key;
+        /// Human-readable
+        QString label;
+    };
+    QVector< ModelInfo > m_list;
+    int m_currentIndex;
+};
+
 class KeyboardLayoutModel : public QAbstractListModel
 {
     Q_OBJECT
@@ -51,35 +105,6 @@ signals:
     void currentIndexChanged( int index );
 };
 
-class KeyboardModelsModel : public QAbstractListModel
-{
-    Q_OBJECT
-    Q_PROPERTY( int currentIndex WRITE setCurrentIndex READ currentIndex NOTIFY currentIndexChanged )
-
-public:
-    explicit KeyboardModelsModel( QObject* parent = nullptr );
-    int rowCount( const QModelIndex& = QModelIndex() ) const override;
-    QVariant data( const QModelIndex& index, int role ) const override;
-
-    void setCurrentIndex( const int& index );
-    int currentIndex() const;
-    const QMap< QString, QString > item( const int& index ) const;
-
-public slots:
-    void refresh();
-
-protected:
-    QHash< int, QByteArray > roleNames() const override;
-
-private:
-    int m_currentIndex = -1;
-    QVector< QMap< QString, QString > > m_list;
-    void detectModels();
-
-signals:
-    void currentIndexChanged( int index );
-};
-
 class KeyboardVariantsModel : public QAbstractListModel
 {
     Q_OBJECT