[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdelibs/kparts
From:       David Faure <faure () kde ! org>
Date:       2007-02-21 21:34:50
Message-ID: 200702212234.50788.faure () kde ! org
[Download RAW message or body]

On Wednesday 21 February 2007, Stephan Kulow wrote:
> SVN commit 636048 by coolo:
> 
> whatever happened
> 
> 
> M  +2 -2      mainwindow.cpp  
> 
> 
> --- trunk/KDE/kdelibs/kparts/mainwindow.cpp #636047:636048
> @@ -176,8 +176,8 @@
> void KParts::MainWindow::saveNewToolbarConfig()
> {
> createGUI( d->m_activePart );
> -    KConfigGroup *
> -    applyMainWindowSettings = new KConfigGroup(KGlobal::config(), QByteArray());
> +    KConfigGroup cg(KGlobal::config(), QByteArray());
> +    applyMainWindowSettings(cg);

Hmm, again an empty group... which wasn't empty in kde3.
The code applyMainWindowSettings said:
    KConfigGroupSaver saver( config, configGroup.isEmpty() ? config->group() : \
configGroup ); and configGroup was a parameter with an empty default value.
So it was saving into "the current group". This was quite broken, on retrospect. 
The goal is to always store the settings for a given type of mainwindow (e.g. kmail \
composer window) into the same group (e.g. "composer"). Whether we're doing that \
explicitely, via autosave, or after "OK" in the toolbar config dialog like above.
So I think we should store the group into a member var, a bit like d->autoSaveGroup - \
but even if  not enabling autosave (=> I would rename it to settingsGroup - like the \
method that we found was redundant ;)

But that means changing apply/saveMainWindowSettings to take a KConfig* again :(

-- 
David Faure, faure@kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).


[prev in list] [next in list] [prev in thread] [next in thread]