kossebau added a comment.

  Okay, got my test user setup working again and thus finally can test how the 
code behaves in real life, next to the unit test.
  
  Turns out that the "bug" is not opposed to the user by some magic things 
happening. In the normal usage the new setting is somehow synced before the 
color profile is loaded with the global settings mapped into (ex: activating 
the Oxygen LnF theme)
  
    BEFORE cs "Breeze"
    WRITING cs "Oxygen"
    READING cs "Oxygen"
  
  , while in the unit test this does not happen (as triggered e.g. by 
KcmTest::testKCMSave())
  
    QDEBUG : KcmTest::testKCMSave() BEFORE cs "customTestValue"
    QDEBUG : KcmTest::testKCMSave() WRITING cs "TestValue"
    QDEBUG : KcmTest::testKCMSave() READING cs "customTestValue"
  
  Debug output created by these lines added:
  
        KConfigGroup configGroup(&m_config, "General");
    qDebug() << "BEFORE cs" << configGroup.readEntry("ColorScheme"); // added
        configGroup.writeEntry("ColorScheme", scheme);
    qDebug() << "WRITING cs" << configGroup.readEntry("ColorScheme"); // added
    
        KSharedConfigPtr conf = KSharedConfig::openConfig(colorFile);//, 
KSharedConfig::CascadeConfig);
    {  // added
    KConfigGroup cg(conf, "General");  // added
    qDebug() << "READING cs" << cg.readEntry("ColorScheme");  // added
    }  // added
  
  No explanation yet found for the different behaviour,
  
  In any case, more important are these debug lines, which show off that 
without KSharedConfig::CascadeConfig and thus with the mapped global config 
settings, the color scheme loop copies not only the color scheme, but also all 
the global config (each line logging "group (keys)"):
  
    COLORSCHEME "Colors:View" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "WM" ("activeBackground", "activeBlend", "activeFont", 
"activeForeground", "inactiveBackground", "inactiveBlend", "inactiveForeground")
    COLORSCHEME "ColorEffects:Disabled" ("Color", "ColorAmount", "ColorEffect", 
"ContrastAmount", "ContrastEffect", "IntensityAmount", "IntensityEffect")
    COLORSCHEME "Colors:Selection" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "ColorEffects:Inactive" ("ChangeSelectionColor", "Color", 
"ColorAmount", "ColorEffect", "ContrastAmount", "ContrastEffect", "Enable", 
"IntensityAmount", "IntensityEffect")
    COLORSCHEME "PreviewSettings" ("MaximumRemoteSize", "camera", "file", 
"fonts")
    COLORSCHEME "Colors:Button" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "KShortcutsDialog Settings" ("Dialog Size")
    COLORSCHEME "KFileDialog Settings" ("Automatically select filename 
extension", "Breadcrumb Navigation", "Decoration position", "LocationCombo 
Completionmode", "PathCombo Completionmode", "Preview Width", "Previews", "Show 
Bookmarks", "Show Full Path", "Show Preview", "Show Speedbar", "Show hidden 
files", "Sort by", "Sort directories first", "Sort reversed", "Speedbar Width", 
"View Style", "detailedViewIconSize", "listViewIconSize")
    COLORSCHEME "Appmenu Style" ("Style")
    COLORSCHEME "DialogIcons" ("ActiveColor", "ActiveColor2", "ActiveEffect", 
"ActiveSemiTransparent", "ActiveValue", "Animated", "DefaultColor", 
"DefaultColor2", "DefaultEffect", "DefaultSemiTransparent", "DefaultValue", 
"DisabledColor", "DisabledColor2", "DisabledEffect", "DisabledSemiTransparent", 
"DisabledValue", "Size")
    COLORSCHEME "MainToolbarIcons" ("ActiveColor", "ActiveColor2", 
"ActiveEffect", "ActiveSemiTransparent", "ActiveValue", "Animated", 
"DefaultColor", "DefaultColor2", "DefaultEffect", "DefaultSemiTransparent", 
"DefaultValue", "DisabledColor", "DisabledColor2", "DisabledEffect", 
"DisabledSemiTransparent", "DisabledValue", "Size")
    COLORSCHEME "KDE" ("ChangeCursor", "ColorScheme", "LookAndFeelPackage", 
"ShowIconsInMenuItems", "ShowIconsOnPushButtons", "contrast", "widgetStyle")
    COLORSCHEME "Toolbar style" ("ToolButtonStyle", 
"ToolButtonStyleOtherToolbars")
    COLORSCHEME "ToolbarIcons" ("ActiveColor", "ActiveColor2", "ActiveEffect", 
"ActiveSemiTransparent", "ActiveValue", "Animated", "DefaultColor", 
"DefaultColor2", "DefaultEffect", "DefaultSemiTransparent", "DefaultValue", 
"DisabledColor", "DisabledColor2", "DisabledEffect", "DisabledSemiTransparent", 
"DisabledValue", "Size")
    COLORSCHEME "DesktopIcons" ("ActiveColor", "ActiveColor2", "ActiveEffect", 
"ActiveSemiTransparent", "ActiveValue", "Animated", "DefaultColor", 
"DefaultColor2", "DefaultEffect", "DefaultSemiTransparent", "DefaultValue", 
"DisabledColor", "DisabledColor2", "DisabledEffect", "DisabledSemiTransparent", 
"DisabledValue", "Size")
    COLORSCHEME "DirSelect Dialog" ("DirSelectDialog Size", "History Items")
    COLORSCHEME "SmallIcons" ("ActiveColor", "ActiveColor2", "ActiveEffect", 
"ActiveSemiTransparent", "ActiveValue", "Animated", "DefaultColor", 
"DefaultColor2", "DefaultEffect", "DefaultSemiTransparent", "DefaultValue", 
"DisabledColor", "DisabledColor2", "DisabledEffect", "DisabledSemiTransparent", 
"DisabledValue", "Size")
    COLORSCHEME "KDE URL Restrictions" ("rule_1", "rule_count")
    COLORSCHEME "Paths" ("Trash")
    COLORSCHEME "Colors:Complementary" ("BackgroundAlternate", 
"BackgroundNormal", "DecorationFocus", "DecorationHover", "ForegroundActive", 
"ForegroundInactive", "ForegroundLink", "ForegroundNegative", 
"ForegroundNeutral", "ForegroundNormal", "ForegroundPositive", 
"ForegroundVisited")
    COLORSCHEME "Directories" ("dir_pixmap")
    COLORSCHEME "Icons" ("Theme")
    COLORSCHEME "KisShortcutsDialog Settings" ("Dialog Size")
    COLORSCHEME "PanelIcons" ("ActiveColor", "ActiveColor2", "ActiveEffect", 
"ActiveSemiTransparent", "ActiveValue", "Animated", "DefaultColor", 
"DefaultColor2", "DefaultEffect", "DefaultSemiTransparent", "DefaultValue", 
"DisabledColor", "DisabledColor2", "DisabledEffect", "DisabledSemiTransparent", 
"DisabledValue", "Size")
    COLORSCHEME "Colors:Window" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "General" ("BrowserApplication", "ColorScheme", "Name", 
"dbfile", "desktopFont", "fixed", "font", "menuFont", "shadeSortColumn", 
"smallestReadableFont", "taskbarFont", "toolBarFont", "widgetStyle")
    COLORSCHEME "Colors:Tooltip" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "Translations" ("LANGUAGE")
  
  where with the setting KSharedConfig::CascadeConfig it is just the color 
scheme data, as desired;
  
    COLORSCHEME "ColorEffects:Disabled" ("Color", "ColorAmount", "ColorEffect", 
"ContrastAmount", "ContrastEffect", "IntensityAmount", "IntensityEffect")
    COLORSCHEME "Colors:Complementary" ("BackgroundAlternate", 
"BackgroundNormal", "DecorationFocus", "DecorationHover", "ForegroundActive", 
"ForegroundInactive", "ForegroundLink", "ForegroundNegative", 
"ForegroundNeutral", "ForegroundNormal", "ForegroundPositive", 
"ForegroundVisited")
    COLORSCHEME "ColorEffects:Inactive" ("ChangeSelectionColor", "Color", 
"ColorAmount", "ColorEffect", "ContrastAmount", "ContrastEffect", "Enable", 
"IntensityAmount", "IntensityEffect")
    COLORSCHEME "Colors:View" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "KDE" ("contrast")
    COLORSCHEME "Colors:Window" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "WM" ("activeBackground", "activeBlend", "activeForeground", 
"inactiveBackground", "inactiveBlend", "inactiveForeground")
    COLORSCHEME "Colors:Button" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "Colors:Tooltip" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "Colors:Selection" ("BackgroundAlternate", "BackgroundNormal", 
"DecorationFocus", "DecorationHover", "ForegroundActive", "ForegroundInactive", 
"ForegroundLink", "ForegroundNegative", "ForegroundNeutral", 
"ForegroundNormal", "ForegroundPositive", "ForegroundVisited")
    COLORSCHEME "General" ("ColorScheme", "Name", "shadeSortColumn")
  
  Debug output created by this line added:
  
        foreach (const QString &grp, conf->groupList()) {
            KConfigGroup cg(conf, grp);
    qDebug() << "COLORSCHEME" << grp << cg.keyList(); // added
            KConfigGroup cg2(&m_config, grp);
            cg.copyTo(&cg2);
        }
  
  So even if by chance(?) not resulting in a bug for the user, still the old 
code does unneeded things. And triggers the fail of the unit test 
KcmTest::testKCMSave().
  
  So IMHO this is a patch good to have in. And similarly also to be applied for 
other places which load color schemes in a KSharedConfig object.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D10259

To: kossebau, broulik, davidedmundson, mart
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

Reply via email to