Review Request: port Sonnet to QSettings

[Kevin Krammer]

> On Dec. 27, 2012, 10:33 a.m., Kevin Krammer wrote:
> > I am a bit puzzled by the usage of QSettings for file I/O.
> > 
> > My, rather limited, understanding of QSettings in Qt5 context is that is mostly the same as in Qt4 and Qt4's version is AFAIK neither capable of doing hierachical files nor immutable settings nor environment/tool-output dependent values.
> > All of which KConfig can do and which could have been deployed (especially hierachy and immutability). 
> > 
> > Or maybe I am misunderstanding the purpose of this endeavour, i.e. is this just exploring options and not idended to be ever merged into KF5?
> 
> David Faure wrote:
>     Right, QSettings doesn't do these things (well, it does hierarchical, but only two levels -- this is the most common use case, though).
>     But this is about simple spell-checking... why would an administrator need immutable settings, or environment variables / tool output in config keys? This is typically user preferences.
>     
>     Any Qt-only library would do exactly this (use QSettings internally, waiting for a better solution in Qt).
>     
>     Yes, someone should really work on getting a better configuration framework into Qt (e.g. splitting out windows registry stuff out of QSettings, to make QSettings INI-only, add a KConfigGroup equivalent to QSettings, and make it support multiple levels of hierarchy via QStandardPaths).

Two levels are most likley the most common case because most systems do not have any system administrator. Once you have admin customization you have at least three (package, admin, user). I don't have any evidence for nor against usage of Kiosk features in regard to spell checking, I am just pointing out that the solution proposed here does have none of those.

I also don't think it matters that a Qt-only library would do exactly this, a Qt-only library would not have been part of a solution that offered those option in the first place.

The reuse of the filename "sonnetrc" suggest to me that the intention is to use the same file. A KConfig based application and a QSettings based application will behave inconsitently if using the same file. Or is this a per-application stored file?

Do we assume that KDE application developers who's programs are being used in an "Enterprise" setup will fork the library and reimplement the config with KConfig or do we want them to use the KF5 version? The later will either require that the library does not handle config itself or at least allows applications to intercept config access or provide config that takes precendence over stored config.

IMHO the most sensible case is to let the application handle config I/O, allowing it to store the config in a way that is most approriate for its usage. If that is a KConfig INI file, a simple QSetting INI file or a JSON file shouldn't really matter to an engine which only is interested in the values.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107791/#review24025
-----------------------------------------------------------


On Dec. 27, 2012, 12:52 a.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. 27, 2012, 12:52 a.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/sonnet/tests/test_configdialog.cpp 4c4fd21 
>   kdeui/widgets/ktextedit.h d0c1c4d 
>   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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121227/2c71257d/attachment.htm>