> On Dec. 17, 2012, 10:38 p.m., Kevin Krammer wrote: > > IMHO this is wrong. > > Not code wise but conceptual. As far as I understand QSettings is basically > > deprecated, it is just not official marked as such because there is no > > replacement. This would be porting away from a fully functional, way more > > advanced and maintained settings. > > > > If the sole goal of this endavor is to remove the KConfig dependency than > > this ought to be done by either having an interface that can be implemented > > through KConfig or by passing values as QVariant maps or hashes. > > > > Oswald Buddenhagen wrote: > and where exactly do you see that kconfig maintainer? ;) > > it's unfortunate that the chosen config class is part of the API. > judging by uses, would it be reasonable to just disable that part of the > API indefinitely? > less drastically, would it be acceptable to pass a file name instead of a > config object? that would of course incur some overhead (assuming we are > passing the application's main config file). > > Kevin Krammer wrote: > "it's unfortunate that the chosen config class is part of the API." > > Indeed. Most likely out of convenience. Hence the idea to just replace it > with a generic key/value object that doesn't do any specific form of I/O and > can allowing the using application to decide on persistant storage. But if I > understand you correctly you would rather go for the full solution and make > those properties directly read-/writable, right? > > Oswald Buddenhagen wrote: > the idea with the file name has the advantage that it is most natural, > but sort of slow, and it may actually not work - if the app uses kconfig, but > sonnet uses qsettings on the same file, you may actually get garbage out of > it. > > i'd strongly recommend not using a variant map, etc., as using it would > require lots of boilerplate code. > > if you make it so that instantiating is nothing else than > new Sonnet::ConfigDialog(new KConfigWrapper(new > KConfigGroup(KGlobal::config(), "Spellchecking"))); > it may be ok. still a bit ... weird. > you could make kconfiggroup directly implement the interface, but then > you'd get an asymmetry with qsettings. > also, where would KMapInterface be defined? where would the qsettings > wrapper live? > or maybe upstream QMapInterface and make QSettings implement it, too? > would it even fit the API? >
What about not exposing any of the config storage details at all? We have the application name, that should be enough to store application specific settings. - Martin Tobias Holmedahl ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107791/#review23643 ----------------------------------------------------------- On Dec. 17, 2012, 9:15 p.m., Martin Tobias Holmedahl Sandsmark wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107791/ > ----------------------------------------------------------- > > (Updated Dec. 17, 2012, 9:15 p.m.) > > > Review request for KDE Frameworks, kdelibs and David Faure. > > > Description > ------- > > Ported everything away from KConfig to QSettings. > > I couldn't really find any users of the ::save function, so I think the > source incompatible change would be worth it. Alternatively we could add a > deprecated dummy function that takes in a KConfig object and just ignores it, > and uses QSettings. > > > Diffs > ----- > > kdeui/sonnet/configdialog.h 7c4993b > kdeui/sonnet/configdialog.cpp 625441b > kdeui/sonnet/configwidget.h 023b659 > kdeui/sonnet/configwidget.cpp 549d5af > kdeui/sonnet/highlighter.cpp 6cbb14c > kdeui/widgets/ktextedit.cpp 71d2a9f > staging/sonnet/src/core/backgroundchecker.h f0da3a3 > staging/sonnet/src/core/backgroundchecker.cpp dc05b94 > staging/sonnet/src/core/globals.cpp bf4f504 > staging/sonnet/src/core/loader.cpp 887aee5 > staging/sonnet/src/core/settings.cpp 59cb593 > staging/sonnet/src/core/settings_p.h e14bad7 > staging/sonnet/src/core/speller.h 37dd82f > staging/sonnet/src/core/speller.cpp f831f55 > > Diff: http://git.reviewboard.kde.org/r/107791/diff/ > > > Testing > ------- > > it builds. > > > Thanks, > > Martin Tobias Holmedahl Sandsmark > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
