keyboard: improve live session layout handling

src/modules/keyboard/Config.cpp

@@ -22,11 +22,14 @@
 #include "utils/Variant.h"
 
 #include <QApplication>
+#include <QDir>
 #include <QFile>
+#include <QFileInfo>
 #include <QGuiApplication>
 #include <QProcess>
 #include <QRegularExpression>
 #include <QStandardPaths>
+#include <QTextStream>
 #include <QTimer>
 
 #include <QDBusConnection>
@@ -119,6 +122,30 @@ xkbmap_query_grp_option()
     return outputLine.mid( index, lastIndex - index );
 }
 
+static void
+prepareGroupSwitcher( const BasicLayoutInfo& settings, AdditionalLayoutInfo& extra )
+{
+    if ( extra.additionalLayout.isEmpty() )
+    {
+        extra.groupSwitcher.clear();
+        return;
+    }
+
+    if ( !settings.selectedGroup.isEmpty() )
+    {
+        extra.groupSwitcher = "grp:" + settings.selectedGroup;
+    }
+
+    if ( extra.groupSwitcher.isEmpty() )
+    {
+        extra.groupSwitcher = xkbmap_query_grp_option();
+    }
+    if ( extra.groupSwitcher.isEmpty() )
+    {
+        extra.groupSwitcher = "grp:alt_shift_toggle";
+    }
+}
+
 AdditionalLayoutInfo
 Config::getAdditionalLayoutInfo( const QString& layout )
 {
@@ -230,20 +257,7 @@ applyXkb( const BasicLayoutInfo& settings, AdditionalLayoutInfo& extra )
     QStringList basicArguments = xkbmap_model_args( settings.selectedModel );
     if ( !extra.additionalLayout.isEmpty() )
     {
-        if ( !settings.selectedGroup.isEmpty() )
-        {
-            extra.groupSwitcher = "grp:" + settings.selectedGroup;
-        }
-
-        if ( extra.groupSwitcher.isEmpty() )
-        {
-            extra.groupSwitcher = xkbmap_query_grp_option();
-        }
-        if ( extra.groupSwitcher.isEmpty() )
-        {
-            extra.groupSwitcher = "grp:alt_shift_toggle";
-        }
-
+        prepareGroupSwitcher( settings, extra );
         basicArguments.append(
             xkbmap_layout_args_with_group_switch( { extra.additionalLayout, settings.selectedLayout },
                                                   { extra.additionalVariant, settings.selectedVariant },
@@ -262,7 +276,7 @@ applyXkb( const BasicLayoutInfo& settings, AdditionalLayoutInfo& extra )
     }
 }
 
-static void
+static bool
 applyLocale1( const BasicLayoutInfo& settings, AdditionalLayoutInfo& extra )
 {
     QString layout = settings.selectedLayout;
@@ -271,6 +285,7 @@ applyLocale1( const BasicLayoutInfo& settings, AdditionalLayoutInfo& extra )
 
     if ( !extra.additionalLayout.isEmpty() )
     {
+        prepareGroupSwitcher( settings, extra );
         layout = extra.additionalLayout + "," + layout;
         variant = extra.additionalVariant + "," + variant;
         option = extra.groupSwitcher;
@@ -283,7 +298,7 @@ applyLocale1( const BasicLayoutInfo& settings, AdditionalLayoutInfo& extra )
     if ( !locale1.isValid() )
     {
         cWarning() << "Interface" << locale1.interface() << "is not valid.";
-        return;
+        return false;
     }
 
     // Using convert=true, this also updates the VConsole config
@@ -293,51 +308,97 @@ applyLocale1( const BasicLayoutInfo& settings, AdditionalLayoutInfo& extra )
         if ( !r.isValid() )
         {
             cWarning() << "Could not set keyboard config through org.freedesktop.locale1.X11Keyboard." << r.error();
+            return false;
         }
     }
+
+    return true;
 }
 
-// In a config-file's list of lines, replace lines <key>=<something> by <key>=<value>
+// In kxkbrc content, set a key inside the [Layout] group and create the group if needed.
 static void
-replaceKey( QStringList& content, const QString& key, const QString& value )
+setLayoutKey( QStringList& content, const QString& key, const QString& value )
 {
+    int groupStart = -1;
+    int groupEnd = content.length();
     for ( int i = 0; i < content.length(); ++i )
+    {
+        const QString line = content.at( i ).trimmed();
+        if ( line == QStringLiteral( "[Layout]" ) )
+        {
+            groupStart = i;
+            continue;
+        }
+        if ( groupStart >= 0 && i > groupStart && line.startsWith( '[' ) )
+        {
+            groupEnd = i;
+            break;
+        }
+    }
+
+    if ( groupStart < 0 )
+    {
+        if ( !content.isEmpty() && !content.constLast().isEmpty() )
+        {
+            content.append( QString() );
+        }
+        content.append( QStringLiteral( "[Layout]" ) );
+        groupStart = content.length() - 1;
+        groupEnd = content.length();
+    }
+
+    for ( int i = groupStart + 1; i < groupEnd; ++i )
     {
         if ( content.at( i ).startsWith( key ) )
         {
             content[ i ] = key + value;
+            return;
         }
     }
+
+    content.insert( groupEnd, key + value );
 }
 
 static bool
-rewriteKWin( const QString& path, const QString& model, const QString& layouts, const QString& variants )
+rewriteKWin( const QString& path,
+             const QString& model,
+             const QString& layouts,
+             const QString& variants,
+             const QString& options )
 {
-    if ( !QFile::exists( path ) )
-    {
-        return false;
-    }
-
     QFile config( path );
-    if ( !config.open( QIODevice::ReadOnly ) )
+    QStringList content;
+    if ( config.exists() )
     {
-        return false;
+        if ( !config.open( QIODevice::ReadOnly ) )
+        {
+            return false;
+        }
+        content = []( QFile& f )
+        {
+            QTextStream s( &f );
+            return s.readAll().split( '\n' );
+        }( config );
+        config.close();
     }
-    QStringList content = []( QFile& f )
+    else
     {
-        QTextStream s( &f );
-        return s.readAll().split( '\n' );
-    }( config );
-    config.close();
+        QDir().mkpath( QFileInfo( path ).path() );
+    }
 
     if ( !config.open( QIODevice::WriteOnly ) )
     {
         return false;
     }
 
-    replaceKey( content, QStringLiteral( "Model=" ), model );
-    replaceKey( content, QStringLiteral( "LayoutList=" ), layouts );
-    replaceKey( content, QStringLiteral( "VariantList=" ), variants );
+    setLayoutKey( content, QStringLiteral( "Model=" ), model );
+    setLayoutKey( content, QStringLiteral( "LayoutList=" ), layouts );
+    setLayoutKey( content, QStringLiteral( "VariantList=" ), variants );
+    if ( !options.isEmpty() )
+    {
+        setLayoutKey( content, QStringLiteral( "Options=" ), options );
+    }
+    setLayoutKey( content, QStringLiteral( "Use=" ), QStringLiteral( "true" ) );
 
     config.write( content.join( '\n' ).toUtf8() );
     config.close();
@@ -349,18 +410,20 @@ void
 applyKWin( const BasicLayoutInfo& settings, AdditionalLayoutInfo& extra )
 {
     const auto paths = QStandardPaths::standardLocations( QStandardPaths::ConfigLocation );
+    prepareGroupSwitcher( settings, extra );
 
-    auto join = [ &additional = extra.additionalLayout ]( const QString& s1, const QString& s2 )
-    { return additional.isEmpty() ? s1 : QStringLiteral( "%1,%2" ).arg( s1, s2 ); };
+    auto join = [ &additional = extra.additionalLayout ]( const QString& additionalValue, const QString& selectedValue )
+    { return additional.isEmpty() ? selectedValue : QStringLiteral( "%1,%2" ).arg( additionalValue, selectedValue ); };
 
-    const QString layouts = join( settings.selectedLayout, extra.additionalLayout );
-    const QString variants = join( settings.selectedVariant, extra.additionalVariant );
+    const QString layouts = join( extra.additionalLayout, settings.selectedLayout );
+    const QString variants = join( extra.additionalVariant, settings.selectedVariant );
+    const QString options = extra.groupSwitcher;
 
     bool updated = false;
     for ( const auto& path : paths )
     {
         const QString candidate = path + QStringLiteral( "/kxkbrc" );
-        if ( rewriteKWin( candidate, settings.selectedModel, layouts, variants ) )
+        if ( rewriteKWin( candidate, settings.selectedModel, layouts, variants, options ) )
         {
             updated = true;
             break;
@@ -415,19 +478,7 @@ applyGnome( const BasicLayoutInfo& settings, AdditionalLayoutInfo& extra )
     // gsettings set org.gnome.desktop.input-sources xkb-options "['grp:lalt_lshift_toggle']"
     if ( !extra.additionalLayout.isEmpty() )
     {
-        // Get a reasonable value for the group switcher, defaulting to alt_shift_toggle if nothing else is set
-        if ( !settings.selectedGroup.isEmpty() )
-        {
-            extra.groupSwitcher = "grp:" + settings.selectedGroup;
-        }
-        if ( extra.groupSwitcher.isEmpty() )
-        {
-            extra.groupSwitcher = xkbmap_query_grp_option();
-        }
-        if ( extra.groupSwitcher.isEmpty() )
-        {
-            extra.groupSwitcher = "grp:alt_shift_toggle";
-        }
+        prepareGroupSwitcher( settings, extra );
 
         const QString xkbOptionsValue = QStringLiteral( "['%1']" ).arg( extra.groupSwitcher );
         const QStringList xkbOptionsCommand = QStringList( sudoArguments ) << "xkb-options" << xkbOptionsValue;
@@ -448,13 +499,14 @@ void
 Config::apply()
 {
     m_additionalLayoutInfo = getAdditionalLayoutInfo( m_current.selectedLayout );
-    if ( m_configureXkb )
+    bool locale1Applied = false;
+    if ( m_configureLocale1 )
     {
-        applyXkb( m_current, m_additionalLayoutInfo );
+        locale1Applied = applyLocale1( m_current, m_additionalLayoutInfo );
     }
-    if ( m_configureLocale1 )
+    if ( m_configureXkb && !locale1Applied )
     {
-        applyLocale1( m_current, m_additionalLayoutInfo );
+        applyXkb( m_current, m_additionalLayoutInfo );
     }
     if ( m_configureKWin )
     {
@@ -651,22 +703,23 @@ Config::detectCurrentKeyboardLayout()
 void
 Config::cancel()
 {
-    const auto extra = getAdditionalLayoutInfo( m_original.selectedLayout );
-    if ( m_configureXkb )
+    auto extra = getAdditionalLayoutInfo( m_original.selectedLayout );
+    bool locale1Applied = false;
+    if ( m_configureLocale1 )
     {
-        applyXkb( m_original, m_additionalLayoutInfo );
+        locale1Applied = applyLocale1( m_original, extra );
     }
-    if ( m_configureLocale1 )
+    if ( m_configureXkb && !locale1Applied )
     {
-        applyLocale1( m_original, m_additionalLayoutInfo );
+        applyXkb( m_original, extra );
     }
     if ( m_configureKWin )
     {
-        applyKWin( m_original, m_additionalLayoutInfo );
+        applyKWin( m_original, extra );
     }
     if ( m_configureGnome )
     {
-        applyGnome( m_original, m_additionalLayoutInfo );
+        applyGnome( m_original, extra );
     }
 }
 
@@ -892,6 +945,7 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     m_convertedKeymapPath = getString( configurationMap, "convertedKeymapPath" );
     m_configureEtcDefaultKeyboard = getBool( configurationMap, "writeEtcDefaultKeyboard", true );
     m_configureLocale1 = getBool( configurationMap, "useLocale1", !isX11 );
+    m_configureXkb = isX11;
 
     bool bogus = false;
     const auto configureItems = getSubMap( configurationMap, "configure", bogus );

src/modules/keyboard/SetKeyboardLayoutJob.cpp

@@ -37,8 +37,25 @@ removeEmpty( QStringList&& list )
     list.removeAll( QString() );
     return list;
 }
+
 }  // namespace
 
+STATICTEST QStringList
+variantList( const AdditionalLayoutInfo& additionalLayoutInfo, const QString& variant )
+{
+    if ( additionalLayoutInfo.additionalLayout.isEmpty() )
+    {
+        return removeEmpty( { variant } );
+    }
+
+    QStringList variants { additionalLayoutInfo.additionalVariant, variant };
+    if ( variants.join( QString() ).isEmpty() )
+    {
+        variants.clear();
+    }
+    return variants;
+}
+
 SetKeyboardLayoutJob::SetKeyboardLayoutJob( const QString& model,
                                             const QString& layout,
                                             const QString& variant,
@@ -280,7 +297,7 @@ SetKeyboardLayoutJob::writeX11Data( const QString& keyboardConfPath ) const
 
 
     const QStringList layouts = removeEmpty( { m_additionalLayoutInfo.additionalLayout, m_layout } );
-    const QStringList variants = removeEmpty( { m_additionalLayoutInfo.additionalVariant, m_variant } );
+    const QStringList variants = variantList( m_additionalLayoutInfo, m_variant );
     stream << "        Option \"XkbLayout\" \"" << layouts.join( "," ) << "\"\n";
     stream << "        Option \"XkbVariant\" \"" << variants.join( "," ) << "\"\n";
     if ( !m_additionalLayoutInfo.additionalLayout.isEmpty() )
@@ -314,7 +331,7 @@ SetKeyboardLayoutJob::writeDefaultKeyboardData( const QString& defaultKeyboardPa
     QTextStream stream( &file );
 
     const QStringList layouts = removeEmpty( { m_additionalLayoutInfo.additionalLayout, m_layout } );
-    const QStringList variants = removeEmpty( { m_additionalLayoutInfo.additionalVariant, m_variant } );
+    const QStringList variants = variantList( m_additionalLayoutInfo, m_variant );
     stream << "# KEYBOARD CONFIGURATION FILE\n\n"
               "# Consult the keyboard(5) manual page.\n\n";