instead of member per entry, use key/value with QVariant
instead of manual local/global config, implement this via parent pointer
optional validator via std::function
one example use: setTabWidth
dhaumann | |
loh.tar |
KTextEditor |
instead of member per entry, use key/value with QVariant
instead of manual local/global config, implement this via parent pointer
optional validator via std::function
one example use: setTabWidth
make && make test
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
Other approach to generic config.
Takes the ideas of D18894 with some tweaks.
Uses enums for the keys (thought ATM puts that into an int in the interface, one might alter that later with some templatizing of the KateConfig.
Currently, one cannot 'unset' a value, but I guess that is ok, since we did not have that before
In general this approach is OK. Some thoughts on this:
As you can see, I am trying to push this concept a bit further...
:=) No idea, for each setting you now dow a hash lookup.
We could optimize that to use a vector with the enum as index, given we enumerate them from 0...
But I think we can only judge that after one has converted close to all things.
- will this allow easy access through the KTextEditor::Config interface? This is a Document and View extension interface.
I think we can just add a second key for that to the ConfigEntry specifying its name there
- can we allow modification of the config values through a generic KTextEditor::Command? I.e. set <key> <value>? Maybe also reset <key>?
We can use the same key there and we have with the defaultValue QVariant even the type of the config entry.
Reset is easy, too, as we just can clear it from the hash (for the local configs), for the global config I don't think it makes sense (we could go to the hard-coded default, thought)
- Can we make this accessible through the upcoming variable interface, e.g. as ${CurrentDocument:Config:<key>}?
Same as above, either extra key or "CamlCase" the key we use for the command-line/kate: var line
As you can see, I am trying to push this concept a bit further...
:=) Yes, and that is clever.
I think given we now encode each config entry as "data", it should be easy to add all these things.
either extra key or "CamlCase" the key we use for the command-line/kate: var line
I would avoid any kind of extra key. Just one string used for the config file and these document variables. I guess "Tab Width" is simpler to convert to "set-tab-width" as "TabWidth"
I think some extra keys are not avoidable to be compatible with the old stuff.
See e.g.
// BEGIN ConfigInterface stff
QStringList KTextEditor::DocumentPrivate::configKeys() const
{
static const QStringList keys = { QLatin1String("backup-on-save-local"), QLatin1String("backup-on-save-suffix"), QLatin1String("backup-on-save-prefix"), QLatin1String("replace-tabs"), QLatin1String("indent-pasted-text"), QLatin1String("tab-width"), QLatin1String("indent-width"), QLatin1String("on-the-fly-spellcheck"), }; return keys;
}
If we have these as extra keys, we can just auto-generate that stringlist (and have all settings exposed for free).
The configValue() + setConfigValue() can then just call directly the setValue()... things, too, after looking up the enum from a hash.
Well, when it is important to be compatible, then may that needed. OTH would it be so terrible to break it?
Perhaps it may more important to keep the doc variables untouched than the config file keys. So I would adjust the config file keys to fit the doc variables. This way you can every kind of setting adjust by such doc variable.
I think breaking the config files is just as bad ;=)
After we have converted all things, we might even support exporting/importing the complete document/view/render global config to from other formats like json using the "nice" keys, as we can just write a small load/store routine for that.
I think such a thing was wanted long ago, too, to transfer the settings somewhere (or store per project) without ini file hassle.
Just one extra key per entry is really not hard, you do that once per setting.
I would not introduce a second one for the CamelCaseStyle variable thought.
If we need that, I would just CamelCase the tab-width style name. That is easily automated in code.
I would like to get this in now.
The next step would be converting all existing settings to the new style like tabWidth but keeping the accessors as is, to have a minimal large change.
If that then is done, one can start to think about using the generic interface and removing the specific accessors (after adding some convenience functions for bool/int/... values perhaps).
Next step then would be to use the command stuff in the ConfigInterface, this would directly allow to expose all settings without any extra code.
Looks good to me.
I see two possible performance issues:
But that needs profiling later.
Please push.
src/utils/kateconfig.cpp | ||
---|---|---|
124 | Possible performance issue? We'll see later. |
Performance must later be profiled after it is used a lot.
Then we can e.g. still switch to a vector indexed with the enum value, that should make lookup fast.