Setting registry

[3nids]

QGIS Enhancement: Setting Registry

Date 2018/05

Author Denis Rouzaud #3nids

Contact denis@opengis.ch

maintainer @3nids

Version QGIS 3.4

Summary

QGIS application settings are spread over the source code and are prone to many errors (typo, different default values, type change (e.g. int to enum, double to int), etc.).
We propose here to introduce a registry for settings which will allow preventing such errors.
As a note, the registration of settings will be mandatory, and current class/methods (QgsSettings) will remain. This means that any code related to settings in a plugin will continue to be valid after this development.
This registry will also provide some extra information like description or validity domain (min, max, etc).
As a first step, these informations will allow to greatly improve the advanced settings editor by providing appropriate widget with a text hint (instead of a simple line edit). Even more, for enum based settings, it will provide a combo box filled with the enum names instead of meaningless integer values.
Second, it will offer an API to associate widget to settings. It can then automatically set the widget from the setting value and later set the setting from the widget value (on value change or dialog acceptance). These features are currently available as a Python library (aka qgis setting manager) and has proven to be very useful for plugin authors.
We believe this is a great addition both for the correctness and completeness of QGIS core but also as a time saving tool for plugin authors.

Problematic

Currently, application settings are read and written from the source code using an in-house derivative of QSettings.
These are the form of:

QgsSettings().setValue( "category/my_setting", value, QgsSettings::Section::Gui);
QVariant value = QgsSettings().value( "category/my_setting", defaultValue, QgsSettings::Section::Gui);

This approach presents a few weakness:

• typo errors are fairly possible and difficult to track down (leading to a badly read or unset value)
• default values are repeated at each call making it difficult to modify and fairly possible to have different default values spread in the source code
• there is no validity checking (value range, string length, etc)
• no meta information is provided with the settings to explicitly define its validity or to provide description about the setting
• QgsSettings::value() returns a QVariant which very frequently needs to be cast later on.
• in the advanced settings editor:
  • settings are sometimes obscure since there is no description text
  • settings associated to enum are visible as integers, which is meaningless without looking in the source code
  • editing setting is risky as there is no validation/control on the entered value

Proposed solution

We are proposing to introduce a registry for settings. This registry QgsSettingRegistry would belong to QgsApplication similarly to current existing registries.

Registering settings

It would offer the possibility to register the settings with the following generic method:

template <class T>
void register( const QString &settingName,
               QVariant::Type type, 
               const T &defaultValue = T(),
               const QString &description = QString() );

Some specific methods would allow defining validity checking for a set of types:

enum NumberType {
  Integer,
  Double
};

template <class T>
void registerNumber( const QString &settingName, 
                     const QString &description = QString(),
                     NumberType type = Integer,
                     const T &defaultValue = T(),
                     double minValue = -DBL_MAX + 1,
                     double maxValue = DBL_MAX );

void registerString( const QString &settingName,
                     const QString &description = QString(),
                     const QString &defaultValue = QString(),
                     int minLength = 0,
                     int maxLength = 1 << 30 );                    

This list is not exhaustive and would be easily completed later on.

The special case of enums

For the case of settings based on enums, such as for snapping mode ( i.e. ActiveLayer, AllLayers, AdvancedConfiguration), a specific method will be proposed to register them:

template <class T>
void registerEnum( const QString &settingName,
                   const T &defaultValue,
                   const QString &description = QString() );

The enum type is deduced using QMetaEnum.
This approach has been already testing in QgsSettings::enumValue() (see 1198dbad) and in open pull request for QgsSettings::setEnumValue().

unregistering settings

Unregistering the settings will be possible via

template <class T>
void unregister( const QString &settingName );

From where registering the settings ?

The setting registration is done at run-time (and not compile-time). This has to be done once and before any call to value/setValue methods.
This will be achieved by having the registration done in one class per module (core, gui, app, server, analysis). This class will just hold the registration calls and be ensured to be run before application startup or library loading (see reply on stackoverflow)

Reading and writing settings

The registry would offer similar function than QgsSettings.

template <class T>
bool setValue( const QString &key, 
                   const T &value, 
                   const QgsSettings::Section section = QgsSettings::NoSection );

template <class T>
T value( const QString &key,
         const Section section = NoSection ) const;

The main difference being that the default value does not need to be entered here.

Some assertion will ensure that the settings are properly set and read. These assertions are:

• the setting exist
• the type provided in setValue() or expected as return type in value() matches the one of the setting
• settings category follows module dependency (a gui setting cannot be called from core)

For instance, this would crash the application (only in Debug build type!):

int v = QgsApplication::instance().settingRegistry().value<int>('myStringSetting');

Python bindings

Registering settings from Python code (and hence from plugins) will be possible using the same method, without the section argument which will be forced to be QgsSettings.Plugins:

settings = QgsApplication.instance().settingRegistry()
settings.register( settingName, type, default, description )

For reading and writing values, the problematic of returned type in c++ does not occur for Python. Some SIP code will allow to return the proper type of variable using the unique value() function:

settings = QgsApplication.instance().settingRegistry()
s = settings.value('myStringSetting')
type(s)
>> str
i = settings.value('myIntegerSetting')
type(i)
>> int

Populating the global settings ini file

The global settings ini file can now be automatically populated.
A tag when registering the setting would define if the setting is located on top of the ini file, or at the bottom as an advanced setting.
An advantage is that settings of plugins could also be modified here.

API break or plugins issues

Registering settings is not mandatory, the current approach of QGIS core and settings would still be available.

There is no API break involved and plugins will strictly continue to work the same way.
There will just be a new opportunity for them to register the settings.

At a later stage, QgsSettings::value() and setValue() might be deprecated, but it is not aimed to be done here. If doing so, the value() and setValue() methods from the registry would be extended to allow accessing unregistered settings (similarly to what is done currently in QgsSettings).

Advanced settings editor

Having more information about the settings, it will be possible to provide improved readability and better editing capabilities in the advanced settings editor.
For example, the line related to default snapping tolerance unit doesn't provide much information about it, having 2 being shown:

When reading, it will now show Pixels instead of 2.
And for editing, a combobox will be shown:

Reading and setting to widgets

Using the registry, it is possible to provide helpers to automatically assign some widget to a setting to:

• set its text hint to the description of the settings
• sets it value using the setting
• read the value and write the settings (either directly on change or when accepting the dialog).

All these functionalities are already available for plugins as a Python library qgissettingmanager.

This would become very handy both for plugin authors and core developp whenever building a UX associated to settings.

Basically, there would be compatibility map between settings type (int, QString, QImage, QStringList, whatever) and some widget type (combo box, slider, spin box, etc.).

Only will be implemented the ones requested for the advanced settings editor.
But the list would be easily completed in the future.

Potential improvements

• the registry could handle renaming of settings (e.g. when creating a new sub-category, of fixing typo) so that older settings are recognized

Open questions

1. Shall the python bindings still offer the possibility to read/write settings from non plugin settings (i.e. core settings)? I think we should in the long term remove this possibility and let the plugin developer use the API rather than writing settings directly. Since QgsSettings() is still accessible, there will be no break of existing plugins. But I don't have a strong opinion here.

Unit tests

This work will be covered by unit tests.

Changelog

13.5.2018 initial version
14.5.2018 added unregister settings
12.5.2019 added population of ini file and assertion on categories

[nirvn]

Big +1 over here.

[nyalldawson]

Another big +1

A benefit not listed here is that we could automatically populate https://github.com/qgis/QGIS/blob/master/resources/qgis_global_settings.ini from the registry. This is currently (another!) place where settings must be redefined, including their default values, and is far from complete. Having this ini file as a complete representation of the available settings (including the nice descriptions) will make deploying customized QGIS installs much easier for admins.

Looking at the API, the settings definitions share a lot in common with processing parameters. It's a shame we can't reuse them (but I don't think that's possible -- processing parameters have too much processing specific logic, and refactoring these out to a common base class for use with settings would be a boring PITA). It's also a shame we can't re-use Inasafe parameters (ping @timlinux ) https://github.com/inasafe/parameters - but unfortunately that's all python, which is a blocker.

So +1 for the described approach.

[sbrunner]

shouldn't the register method return an object with a get/set to access directly to the settings without specify again the name?

To have code like that:

MY_NAME = QgsSettings.register("my_component/name", ...)

...

name = MY_NAME.get();

...

MY_NAME.set(new_name)

[haubourg]

big +1 too

[elpaso]

+1
... provided that qgis_global_settings.ini remains the recommended way to set (or override) defaults, this is really useful for enterprise deployments.

[3nids]

@sbrunner The point is that settings will be registered from one place but be read/set from another.

@elpaso indeed. I don't think this will change anything regarding the INI file since QSettings is created using it. One improvement regarding INI file is related to PR #6733 since it will possible to write enum based settings as string instead of integer, which is much more meaningful (it will not break existing INI since integer will still be read)

[3nids]

@nyalldawson I completed with the ini file section. I think another advantage here is that one could also modify default value of settings of plugins, pretty handy for enterprise.

[haubourg]

Hi, I hope I'm not off topic here. Enterprise deployment requires to be able to lock some settings, or be able to make sure a list of values is present in case the setting is a list.
Up to now, I suggested external scripting at the OS level to maintain the INI files. Would that make sense to propose a core mechanism to constraint settings ?

[3nids]

I suppose it would be fairly easy to add such mechanism.

[haubourg]

@3nids thanks for the confirmation

[luipir]

I almosto +1 about the general aspect of the PR, but -1 about

"Pyhton bindings
... without the section argument which will be forced to be QgsSettings.Plugins:"...

reasons are

1. not only plugins use qsetting s( it's possibile to create python providers)
2. what would be the rationale to limitate the user changing settings in other sections?... soft security?

[3nids]

My reasoning was that plugins are registering the settings a bit everywhere, making the whole a bit messy.
But it's true, it should still be possible.
What about having another method as registerPluginSetting ?

[luipir]

What about having another method as registerPluginSetting ?

for me could be ok

[ponceta]

It would be really nice to have it in core instead of in every plugin (with a different version)!

[3nids]

The concerns raised in the PR are very valid (mainly compilation overhead) and have proven the alternative approach to be too costly.
We will propose a new solution (much closer to this QEP) in the following days.

[3nids]

Final Report

QEP acceptance

This QEP did not make it during the 2020 votation process.
But thanks to the remaining available funds from QGIS.org and OPENGIS.ch being OK to offer the rest of the development,
we could start the implementation of a subset of the QEP, the core part (leaving away the gui part).

Implementation and changes from the original proposal

Instead of creating the settings at run time, a different approach using static definitions of the settings was proposed. This reduces the use of literals in the code and helps the dev experience by bringing code completion when looking for settings.
The settings are then added to a setting registry. There is a main core registry to which others can be attached such as gui, app, analysis but also providers or plugins one.
This nesting of registries allows us to perform introspection.

What has been achieved

The complete implementation of the core part has been achieved (settings, registry and Python bindings).
All core settings were migrated.

Code changes:

• core part: Settings registry (QEP 124) QGIS#42597
• introspection: Settings registry (QEP 124) part two settings introspection QGIS#42860
• migration of core settings: Settings registry (QEP 124) part three QGIS#42954

What is missing from the complete proposal or could be added as a follow-up

• The rest of the settings must be migrated
• a CI test should be introduced to avoid usage of old API (QgsSettings)
• the gui part (providing auto-populated widgets to edit settings: color picker for color settings, combobox for enum, etc.)
• adapt the advanced settings editor to take advantage of the introspection and gui part
• a tool to automatically populate an ini file out of current value or defaults on request
• a migration tool to migrate settings (rename, move, … such as migrating old R/G/B/A settings to proper color ones.)

[nyalldawson]

This QEP is being archived in order to empty the issue tracker on this repository. Previous discussion and voting on the QEP remains valid and unchanged.