> On Sept. 30, 2012, 8:26 a.m., David Faure wrote: > > Looks good to me (feel free to commit if nobody else has comments). > > > > But then, what if someone is annoyed by icon views, and wants a details > > view everywhere? [switch this sentence around if the default is details > > view] > > He'll have to switch the view mode of kfiledialog in every single KDE app, > > right? > > Tough problem, there's never a perfect solution with these things (making > > everyone happy leads to too many config options, and alternatives leave > > some people unhappy). > > > > Aurélien Gâteau wrote: > I have been using it for a while, and I have to admit it is surprising to > have the dialog show up with different layouts depending on the application. > I found a way to force KDirOperator to write its config to kdeglobals without > extending the API, by replacing the call to KDirOperator::writeConfig() with > this: > > KConfig tmp( QString(), KConfig::SimpleConfig ); > KConfigGroup tmpGroup( &tmp, configGroup.name() ); > ops->writeConfig( tmpGroup ); > tmpGroup.copyTo( &configGroup, globalFlags ); > > I can update the patch to do this if you prefer. It should even be > possible to avoid adding globalFlags to all writeEntry() calls by making all > the code of the function write to the tmpGroup. That would be closer to the > way the code was supposed to work. > > David Faure wrote: > Sounds good. But then... if we write everything to kdeglobals, then > what's the usefulness of the KConfigGroup passed as parameter? > Ah, to select the group name? So much for sharing the settings between > apps, then. > > I'm just a bit confused as to the goals here; there's a conflict between > "using the KConfigGroup parameter" and "storing all this in kdeglobals", IMHO. > Which is what you're trying to fix, but it seems to me that we're exactly > where we started from, just with different code.
Oh right, if the settings are going to be shared among apps, then the code should not be able to use a different group name. I am going to update the patch. Regarding being back to where we started from, that's not exactly true: we started with KFileDialog not remembering settings at all because it was using a separate, unshared, instance of KConfig to access kdeglobals. - Aurélien ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106581/#review19620 ----------------------------------------------------------- On Sept. 26, 2012, 4:19 p.m., Aurélien Gâteau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106581/ > ----------------------------------------------------------- > > (Updated Sept. 26, 2012, 4:19 p.m.) > > > Review request for kdelibs. > > > Description > ------- > > This patch makes KFileDialog remember settings such as which view mode is > selected and whether the places sidebar should be visible. > > Original code tried to save those to kdeglobals so that changes would be > shared among all applications but it did so the wrong way. The patch writes > the configuration to kdeglobals correctly, but saves the KDirOperator to the > application config file (KDirOperator configuration settings are sort > settings, show preview, show hidden files, view style (icon, detail, > treeview)) > > There are two reasons for not saving KDirOperator config to kdeglobals: > > 1. It is right now not possible to tell KDirOperator::writeConfig() to save > to kdeglobals. It could be done by adding a new version of writeConfig() > which would accept a KConfigBase::WriteFlags argument though. > > 2. It probably would not be a good idea to remember KDirOperator settings > globally anyway because depending on the application one may want to use > different settings. > For example if user wants to select images or videos he might set the file > dialog to show big icons and the preview pane (so that videos can be played). > This setup would however not be adapted in an application where one wants to > select a text file. > > > This addresses bug 139475. > http://bugs.kde.org/show_bug.cgi?id=139475 > > > Diffs > ----- > > kfile/kfilewidget.cpp 8e2f967 > > Diff: http://git.reviewboard.kde.org/r/106581/diff/ > > > Testing > ------- > > Tested with two different KDE applications. Settings are correctly remembered. > > > Thanks, > > Aurélien Gâteau > >
