KXkb: overhaul loading/saving functions

* Fixed optimized loading of initial settings using KxkbConfig::LOAD_INIT_OPTIONS (I had sort of broken it in the past)
* Removed unused KxkbConfig::LOAD_ACTIVE_OPTIONS
* newInstance() now delegates calling setLayout() to readSettings()
* Merged initTray() into readSettings() - has no reason to exist as separate function
* Consistent code style and more appropriate function names and return types
* Commented option sections for clarity
* Removed superfluous debug messages

Signed-off-by: Mavridis Philippe <[email protected]>

5 files changed, 162 insertions, 122 deletions

diff --git a/kxkb/kcmlayout.cpp b/kxkb/kcmlayout.cpp
index 4ee6c172a..94c7b7d49 100644
--- a/kxkb/kcmlayout.cpp
+++ b/kxkb/kcmlayout.cpp
@@ -204,10 +204,8 @@ LayoutConfig::~LayoutConfig()
 
 void LayoutConfig::load()
 {
-	m_kxkbConfig.load(KxkbConfig::LOAD_ALL);
-
+	m_kxkbConfig.load(KxkbConfig::LOAD_ALL_OPTIONS);
 	keys->readSettings();
-
 	initUI();
 }
 
diff --git a/kxkb/kxkb.cpp b/kxkb/kxkb.cpp
index 9c30ac440..8bced3fea 100644
--- a/kxkb/kxkb.cpp
+++ b/kxkb/kxkb.cpp
@@ -105,80 +105,85 @@ KXKBApp::~KXKBApp()
 
 int KXKBApp::newInstance()
 {
-    if (settingsRead()) {
-        setLayout(m_currentLayout);
-    }
+    readSettings();
     return 0;
 }
 
-bool KXKBApp::settingsRead()
+void KXKBApp::readSettings()
 {
-    kxkbConfig.load(KxkbConfig::LOAD_ALL);
-    applySettings();
+    // Xkb options
+    kxkbConfig.load(KxkbConfig::LOAD_INIT_OPTIONS);
 
-    if ( kxkbConfig.m_useKxkb == false ) {
+    if (!kxkbConfig.m_useKxkb)
+    {
+        kdDebug() << "kxkb is disabled, applying xkb options and exiting" << endl;
+        applyXkbOptions();
         kapp->quit();
-        return false;
+        return;
     }
 
-	m_prevWinId = X11Helper::UNKNOWN_WINDOW_ID;
+    kdDebug() << "applying xkb options and layouts" << endl;
+    kxkbConfig.load(KxkbConfig::LOAD_ALL_OPTIONS);
+    applyXkbOptions();
 
-	if( kxkbConfig.m_switchingPolicy == SWITCH_POLICY_GLOBAL ) {
-		delete kWinModule;
-		kWinModule = NULL;
-	}
-	else {
-		TQDesktopWidget desktopWidget;
-		if( desktopWidget.numScreens() > 1 && desktopWidget.isVirtualDesktop() == false ) {
-			kdWarning() << "With non-virtual desktop only global switching policy supported on non-primary screens" << endl;
-			//TODO: find out how to handle that
-		}
+    // Active window watcher
+    m_prevWinId = X11Helper::UNKNOWN_WINDOW_ID;
 
-		if( kWinModule == NULL ) {
-			kWinModule = new KWinModule(0, KWinModule::INFO_DESKTOP);
-			connect(kWinModule, TQ_SIGNAL(activeWindowChanged(WId)), TQ_SLOT(windowChanged(WId)));
-		}
-		m_prevWinId = kWinModule->activeWindow();
-		kdDebug() << "Active window " << m_prevWinId << endl;
-	}
+    if (kxkbConfig.m_switchingPolicy == SWITCH_POLICY_GLOBAL)
+    {
+        delete kWinModule;
+        kWinModule = nullptr;
+    }
 
-	m_layoutOwnerMap->reset();
-	m_layoutOwnerMap->setCurrentWindow( m_prevWinId );
+    else
+    {
+        TQDesktopWidget desktopWidget;
+        if (desktopWidget.numScreens() > 1 && !desktopWidget.isVirtualDesktop())
+        {
+            kdWarning() << "With non-virtual desktop only global switching policy supported on non-primary screens" << endl;
+            //TODO: find out how to handle that
+        }
 
-	if( m_rules == NULL )
-		m_rules = new XkbRules(false);
+        if (!kWinModule)
+        {
+            kWinModule = new KWinModule(nullptr, KWinModule::INFO_DESKTOP);
+            connect(kWinModule, TQ_SIGNAL(activeWindowChanged(WId)), TQ_SLOT(windowChanged(WId)));
+        }
 
-	for(int ii=0; ii<(int)kxkbConfig.m_layouts.count(); ii++) {
-		LayoutUnit& layoutUnit = kxkbConfig.m_layouts[ii];
-	}
+        m_prevWinId = kWinModule->activeWindow();
+        kdDebug() << "Active window " << m_prevWinId << endl;
+    }
 
-	m_currentLayout = kxkbConfig.m_layouts[0];
-	kdDebug() << "default layout is " << m_currentLayout.toPair() << endl;
+    // Init layout owner map
+    m_layoutOwnerMap->reset();
+    m_layoutOwnerMap->setCurrentWindow( m_prevWinId );
 
-	if( kxkbConfig.m_layouts.count() == 1 && !kxkbConfig.m_showSingle) {
-		kapp->quit();
-		return false;
- 	}
+    // Init rules
+    if (!m_rules)
+    {
+        m_rules = new XkbRules(false);
+    }
 
-	initTray();
+    // Init layouts
+    for (int i = 0; i < kxkbConfig.m_layouts.count(); i++)
+    {
+        LayoutUnit& layoutUnit = kxkbConfig.m_layouts[i];
+    }
 
-	TDEGlobal::config()->reparseConfiguration(); // kcontrol modified kdeglobals
-	keys->readSettings();
-	keys->updateConnections();
+    m_currentLayout = kxkbConfig.m_layouts[0];
+    setLayout(m_currentLayout);
 
-	return true;
-}
+    kdDebug() << "default layout is " << m_currentLayout.toPair() << endl;
 
-void KXKBApp::applySettings()
-{
-    XkbOptions options = kxkbConfig.getKXkbOptions();
-    if (!m_extension->setXkbOptions(options)) {
-        kdWarning() << "Setting XKB options failed!" << endl;
+    if (kxkbConfig.m_layouts.count() == 1 && !kxkbConfig.m_showSingle)
+    {
+        kapp->quit();
+        return;
     }
-}
 
-void KXKBApp::initTray()
-{
+    TDEGlobal::config()->reparseConfiguration(); // kcontrol modified kdeglobals
+
+    // Init tray
     if (!m_tray)
     {
         m_tray = new KxkbSystemTray(&kxkbConfig);
@@ -189,13 +194,25 @@ void KXKBApp::initTray()
     m_tray->initLayoutList(kxkbConfig.m_layouts, *m_rules);
     m_tray->setCurrentLayout(m_currentLayout);
     m_tray->show();
+
+    // Init keybindings
+    keys->readSettings();
+    keys->updateConnections();
+}
+
+void KXKBApp::applyXkbOptions()
+{
+    XkbOptions options = kxkbConfig.getKXkbOptions();
+    if (!m_extension->setXkbOptions(options)) {
+        kdWarning() << "Setting XKB options failed!" << endl;
+    }
 }
 
 void KXKBApp::hardwareChanged(TDEGenericDevice *dev)
 {
     if (dev->type() == TDEGenericDeviceType::Keyboard)
     {
-        TQTimer::singleShot(500, this, TQ_SLOT(applySettings()));
+        TQTimer::singleShot(500, this, TQ_SLOT(applyXkbOptions()));
     }
 }
 
@@ -275,11 +292,13 @@ void KXKBApp::slotGroupChanged(uint group)
 {
     kdDebug() << "slotGroupChanged: " << group << ", layout count: " << kxkbConfig.m_layouts.count() << endl;
     if (group >= kxkbConfig.m_layouts.count()) {
-        if (m_tray) {
+        if (m_tray)
+        {
             m_tray->setError(i18n("Unknown"));
         }
 
-        if (kxkbConfig.m_enableNotify) {
+        if (kxkbConfig.m_enableNotify)
+        {
             showErrorNotification(i18n("Unknown"));
         }
 
diff --git a/kxkb/kxkb.h b/kxkb/kxkb.h
index 441844583..b5ac7b7bf 100644
--- a/kxkb/kxkb.h
+++ b/kxkb/kxkb.h
@@ -81,11 +81,8 @@ protected slots:
 
     void hardwareChanged(TDEGenericDevice *dev);
 
-    void applySettings();
-
-protected:
-    // Read settings, and apply them.
-    bool settingsRead();
+    void readSettings();
+    void applyXkbOptions();
 
 private:
     void initTray();
diff --git a/kxkb/kxkbconfig.cpp b/kxkb/kxkbconfig.cpp
index 89e96a0aa..166dab174 100644
--- a/kxkb/kxkbconfig.cpp
+++ b/kxkb/kxkbconfig.cpp
@@ -30,116 +30,141 @@ static const char* switchModes[SWITCH_POLICY_COUNT] = {
 const LayoutUnit DEFAULT_LAYOUT_UNIT = LayoutUnit("us", "");
 const char* DEFAULT_MODEL = "pc104";
 
-bool KxkbConfig::load(int loadMode)
+void KxkbConfig::load(int loadMode)
 {
+	// INITIAL OPTIONS (loaded regardless of whether KXkb is enabled)
 	TDEConfig *config = new TDEConfig("kxkbrc", true, false);
 	config->setGroup("Layout");
 
-	if( loadMode == LOAD_ALL ) {
-		m_resetOldOptions = config->readBoolEntry("ResetOldOptions", true);
-		m_options = config->readEntry("Options", "");
-	}
-
 	m_useKxkb = config->readBoolEntry("Use", false);
-	kdDebug() << "Use kxkb " << m_useKxkb << endl;
 
-	if( (m_useKxkb == false && loadMode == LOAD_ACTIVE_OPTIONS )
-	  		|| loadMode == LOAD_INIT_OPTIONS )
-		return true;
+	m_resetOldOptions = config->readBoolEntry("ResetOldOptions", true);
+	m_options = config->readEntry("Options", "");
 
+	if (loadMode == LOAD_INIT_OPTIONS)
+	{
+		return;
+	}
+
+	// ALL OTHER OPTIONS (loaded only when KXkb is enabled)
 	m_model = config->readEntry("Model", DEFAULT_MODEL);
-	kdDebug() << "Model: " << m_model << endl;
 
+    // Layouts
 	TQStringList layoutList;
-	if( config->hasKey("LayoutList") ) {
+	if (config->hasKey("LayoutList"))
+	{
 		layoutList = config->readListEntry("LayoutList");
 	}
-	else { // old config
+	else
+	{ // old config
 		TQString mainLayout = config->readEntry("Layout", DEFAULT_LAYOUT_UNIT.toPair());
 		layoutList = config->readListEntry("Additional");
 		layoutList.prepend(mainLayout);
 	}
-	if( layoutList.count() == 0 )
+
+	if (layoutList.count() == 0)
+	{
 		layoutList.append("us");
+	}
+
+	TQStringList::ConstIterator it;
 
 	m_layouts.clear();
-	for(TQStringList::ConstIterator it = layoutList.begin(); it != layoutList.end() ; ++it) {
+	for (it = layoutList.begin(); it != layoutList.end(); ++it)
+	{
 		m_layouts.append( LayoutUnit(*it) );
-		kdDebug() << " layout " << LayoutUnit(*it).toPair() << " in list: " << m_layouts.contains( LayoutUnit(*it) ) << endl;
+		kdDebug() << " layout " << LayoutUnit(*it).toPair() << " in list: " << m_layouts.contains(LayoutUnit(*it)) << endl;
 	}
 
 	kdDebug() << "Found " << m_layouts.count() << " layouts" << endl;
 
+	// Display names
 	TQStringList displayNamesList = config->readListEntry("DisplayNames", ',');
-	for(TQStringList::ConstIterator it = displayNamesList.begin(); it != displayNamesList.end() ; ++it) {
+	for (it = displayNamesList.begin(); it != displayNamesList.end() ; ++it)
+	{
 		TQStringList displayNamePair = TQStringList::split(':', *it );
-		if( displayNamePair.count() == 2 ) {
-			LayoutUnit layoutUnit( displayNamePair[0] );
-			if( m_layouts.contains( layoutUnit ) ) {
+		if (displayNamePair.count() == 2)
+		{
+			LayoutUnit layoutUnit(displayNamePair[0]);
+			if (m_layouts.contains(layoutUnit))
+			{
 				m_layouts[m_layouts.findIndex(layoutUnit)].displayName = displayNamePair[1].left(3);
 			}
 		}
 	}
 
+	// Tray indicator
 	m_showSingle = config->readBoolEntry("ShowSingle", false);
+
 	m_showFlag = config->readBoolEntry("ShowFlag", true);
 	m_showLabel = config->readBoolEntry("ShowLabel", true);
+
 	m_fitToBox = config->readBoolEntry("FitFlagToBox", true);
-	m_dimFlag = config->readBoolEntry("DimFlag", true);
 
 	m_useThemeColors = config->readBoolEntry("UseThemeColors", false);
 	m_colorBackground = config->readColorEntry("ColorBackground", new TQColor(TQt::gray));
 	m_bgTransparent = config->readBoolEntry("BgTransparent", false);
 	m_colorLabel = config->readColorEntry("ColorLabel", new TQColor(TQt::white));
+
 	m_labelFont = config->readFontEntry("LabelFont", new TQFont("sans", 10, TQFont::Bold));
 	m_labelShadow = config->readBoolEntry("LabelShadow", true);
 	m_colorShadow = config->readColorEntry("ColorShadow", new TQColor(TQt::black));
 
+	m_dimFlag = config->readBoolEntry("DimFlag", true);
 	m_bevel = config->readBoolEntry("IndicatorBevel", false);
 
+	// Switching policy
 	TQString layoutOwner = config->readEntry("SwitchMode", "Global");
 
-	if( layoutOwner == "WinClass" ) {
+	if (layoutOwner == "WinClass")
+	{
 		m_switchingPolicy = SWITCH_POLICY_WIN_CLASS;
 	}
-	else if( layoutOwner == "Window" ) {
+	else if (layoutOwner == "Window")
+	{
 		m_switchingPolicy = SWITCH_POLICY_WINDOW;
 	}
-	else /*if( layoutOwner == "Global" )*/ {
+	else
+	{
 		m_switchingPolicy = SWITCH_POLICY_GLOBAL;
 	}
 
-	if( m_layouts.count() < 2 && m_switchingPolicy != SWITCH_POLICY_GLOBAL ) {
+	if (m_layouts.count() < 2 && m_switchingPolicy != SWITCH_POLICY_GLOBAL)
+	{
 		kdWarning() << "Layout count is less than 2, using Global switching policy" << endl;
 		m_switchingPolicy = SWITCH_POLICY_GLOBAL;
 	}
 
 	kdDebug() << "Layout owner mode " << layoutOwner << endl;
 
+	// Sticky switching
 	m_stickySwitching = config->readBoolEntry("StickySwitching", false);
-	m_stickySwitchingDepth = config->readEntry("StickySwitchingDepth", "2").toInt();
-	if( m_stickySwitchingDepth < 2 )
+	m_stickySwitchingDepth = config->readNumEntry("StickySwitchingDepth", 2);
+	if (m_stickySwitchingDepth < 2)
+	{
 		m_stickySwitchingDepth = 2;
+	}
 
-	if( m_stickySwitching == true ) {
-		if( m_layouts.count() < 3 ) {
+	if (m_stickySwitching)
+	{
+		if (m_layouts.count() < 3)
+		{
 			kdWarning() << "Layout count is less than 3, sticky switching will be off" << endl;
 			m_stickySwitching = false;
 		}
-		else
-		if( (int)m_layouts.count() - 1 < m_stickySwitchingDepth ) {
+		else if (m_layouts.count() - 1 < m_stickySwitchingDepth)
+		{
 			kdWarning() << "Sticky switching depth is more than layout count -1, adjusting..." << endl;
 			m_stickySwitchingDepth = m_layouts.count() - 1;
 		}
 	}
 
+	// Notifications
 	config->setGroup("Notifications");
 	m_enableNotify = config->readBoolEntry("Enable", false);
 	m_notifyUseKMilo = config->readBoolEntry("UseKMilo", true);
 
 	delete config;
-
-	return true;
 }
 
 void KxkbConfig::save()
@@ -147,59 +172,65 @@ void KxkbConfig::save()
 	TDEConfig *config = new TDEConfig("kxkbrc", false, false);
 	config->setGroup("Layout");
 
-	config->writeEntry("Model", m_model);
+	config->writeEntry("Use", m_useKxkb);
 
 	config->writeEntry("ResetOldOptions", m_resetOldOptions);
 	config->writeEntry("Options", m_options );
 
+	config->writeEntry("Model", m_model);
+
+	// Layouts
 	TQStringList layoutList;
 	TQStringList displayNamesList;
 
 	TQValueList<LayoutUnit>::ConstIterator it;
-	for(it = m_layouts.begin(); it != m_layouts.end(); ++it) {
+	for (it = m_layouts.begin(); it != m_layouts.end(); ++it) {
 		const LayoutUnit& layoutUnit = *it;
+		layoutList.append(layoutUnit.toPair());
 
-		layoutList.append( layoutUnit.toPair() );
-
-		TQString displayName( layoutUnit.displayName );
-		kdDebug() << " displayName " << layoutUnit.toPair() << " : " << displayName << endl;
-		if( displayName.isEmpty() == false && displayName != layoutUnit.layout ) {
+		// Display name
+		TQString displayName(layoutUnit.displayName);
+		if (!displayName.isEmpty() && displayName != layoutUnit.layout)
+		{
 			displayName = TQString("%1:%2").arg(layoutUnit.toPair(), displayName);
-			displayNamesList.append( displayName );
+			displayNamesList.append(displayName);
 		}
 	}
-
 	config->writeEntry("LayoutList", layoutList);
-	kdDebug() << "Saving Layouts: " << layoutList << endl;
+	config->writeEntry("DisplayNames", displayNamesList);
 
-//	if( displayNamesList.empty() == false )
-		config->writeEntry("DisplayNames", displayNamesList);
-// 	else
-// 		config->deleteEntry("DisplayNames");
-
-	config->writeEntry("Use", m_useKxkb);
+	// Tray indicator
 	config->writeEntry("ShowSingle", m_showSingle);
 
 	config->writeEntry("ShowFlag", m_showFlag);
 	config->writeEntry("ShowLabel", m_showLabel);
+
 	config->writeEntry("FitFlagToBox", m_fitToBox);
-	config->writeEntry("DimFlag", m_dimFlag);
 
 	config->writeEntry("UseThemeColors", m_useThemeColors);
 	config->writeEntry("ColorBackground", m_colorBackground);
 	config->writeEntry("BgTransparent", m_bgTransparent);
 	config->writeEntry("ColorLabel", m_colorLabel);
+
 	config->writeEntry("LabelFont", m_labelFont);
 	config->writeEntry("LabelShadow", m_labelShadow);
 	config->writeEntry("ColorShadow", m_colorShadow);
 
+	config->writeEntry("DimFlag", m_dimFlag);
 	config->writeEntry("IndicatorBevel", m_bevel);
 
+	// Switching policy
 	config->writeEntry("SwitchMode", switchModes[m_switchingPolicy]);
 
+	// Sticky switching
 	config->writeEntry("StickySwitching", m_stickySwitching);
 	config->writeEntry("StickySwitchingDepth", m_stickySwitchingDepth);
 
+	// Notifications
+	config->setGroup("Notifications");
+	config->writeEntry("Enable", m_enableNotify);
+	config->writeEntry("UseKMilo", m_notifyUseKMilo);
+
 	// remove old options
  	config->deleteEntry("Variants");
 	config->deleteEntry("Includes");
@@ -208,12 +239,7 @@ void KxkbConfig::save()
 	config->deleteEntry("Additional");
 	config->deleteEntry("Layout");
 
-	config->setGroup("Notifications");
-	config->writeEntry("Enable", m_enableNotify);
-	config->writeEntry("UseKMilo", m_notifyUseKMilo);
-
 	config->sync();
-
 	delete config;
 }
 
@@ -290,7 +316,7 @@ const XkbOptions KxkbConfig::getKXkbOptions() {
 	XkbOptions options;
 	TQStringList layouts;
 	TQStringList variants;
-	for(TQValueList<LayoutUnit>::ConstIterator it = m_layouts.begin(); it != m_layouts.end(); ++it) {
+	for (TQValueList<LayoutUnit>::ConstIterator it = m_layouts.begin(); it != m_layouts.end(); ++it) {
 		const LayoutUnit& layoutUnit = *it;
 		layouts << layoutUnit.layout;
 		variants << layoutUnit.variant;
diff --git a/kxkb/kxkbconfig.h b/kxkb/kxkbconfig.h
index 2eeb25c13..a365ef252 100644
--- a/kxkb/kxkbconfig.h
+++ b/kxkb/kxkbconfig.h
@@ -45,7 +45,7 @@ extern const char* DEFAULT_MODEL;
 class KxkbConfig
 {
 public:
-	enum { LOAD_INIT_OPTIONS, LOAD_ACTIVE_OPTIONS, LOAD_ALL };
+	enum { LOAD_INIT_OPTIONS, LOAD_ALL_OPTIONS };
 
 	bool m_useKxkb;
 	bool m_showSingle;
@@ -74,7 +74,7 @@ public:
 	TQString m_options;
 	TQValueList<LayoutUnit> m_layouts;
 
-	bool load(int loadMode);
+	void load(int loadMode);
 	void save();
 	void setDefaults();