Review Request: port Sonnet to QSettings

[Martin Tobias Holmedahl Sandsmark]

On Thu, Dec 27, 2012 at 03:13:52PM +0100, Kevin Krammer wrote:
> Indeed, it should be doing any form of config IO. It can't know whether any of 
> its options need to be persisted, or are supplied hardcoded by the using code 
> or offered to the user but volatile or offered the users and stored 
> persistently or confgured by the system administrator or configured by the 
> system vendor.
> 
> Due to the old code we can deduce that the option values were potentially 
> stored persistently, allowing user, administrator and system vendor to 
> provide, override and lock-down each of them.
> 
> But as you said yourself: it does spell checking, it does not have to care 
> about settings persistence at all.

So, we could just remove all usage of KConfig/QSettings? We're able to get
the default locale for the application (and automatically detect the language
once I merge that work), with the changeLanguage() function I think we're pretty
well-covered.


> No idea, maybe they all just hard-code values. The old code's usage of KConfig 
> and the config access in ktextedit.cpp suggests that some applications wanted 
> to settings to stay configurable.
> In either case nothing the spell checker needs to care about. It uses the 
> values, it has no interest what so ever in how it got them in the first place.

Yeah, I couldn't really find any usage of the config storage stuff.


> My problem with understanding this is: why does the spell checker need the 
> capability to secretly access persistent config?
> Removing its internal config access will still have it "Just Works" by default 
> (again assuming that those variables are at least initialized properly).

Well, it abused it for one thing in particular; words to ignore. But this
should really be handled by whatever spellchecking backend we use (aspell at
least has this functionality built in, with proper system/package/user
separation etc.).


> So I looked at Sonnet::Loader which seemed to be referred to a lot regarding 
> settings. Finally, a way to access the settings! Weird thing is that it is 
> declared in a header with _p.h suffix, usually indicating that this is not 
> public API. Even weirder, the same seems to be true for Sonnet::Settings!

Yes, Sonnet::Settings isn't supposed to be accessed by applications. From
looking at it, it doesn't really contain stuff that should be
application-specific (skipping run-together words etc.).
Sonnet::BackgroundChecker however, for example, already exposes a method to
change language, which is something I think applications might want to change
themselves. We could just remove the code that persist this, though?

-- 
Martin Sandsmark