New way of defining/using setting (QGIS-Enhancement-Proposals #124)

[domi4484]

As of Enhancement proposal #124 the goals of this pull request are to make the usage of settings less prone to errors.
Currently literals are used when accessing settings which is prone to typos errors.
Another source of error is that default value must be provided when getting the value of a settings and could potentially be different at different place in the code. It is also problematic to change the default value.
The idea is also to bring meta-information to the settings to provide more information and automatically configured editing widgets for the settings whenever required (such as in the advanced settings editor)

In this proposed solution settings are organized in settings groups and settings entries accessible from more central locations (QgsSettingsRegistryCore for section core, QgsSettingsRegistryGui for section gui, ...).
Settings entries represents one settings path like for example "measure/planimetric" and provides methods to set/read values. Settings are accessed by instance so errors are detected at compile time.

Setting entries have meta-information like description, min/max values or other information to get better displayed in Gui. For example for a double settings an information about how many decimals make sense to display.

This approach has some pro and cons:

Pros

• No more literals in code -> less typo errors
• Default values centralized
• Meta information (description, gui, ...)
• Auto completion
• Hierarchic way of declaring/accessing settings
• Programmer is forced to think which category/group suites better for new settings
• Functions to reset to default instead of delete

Cons

• Settings must be declared (more coding)
• Less flexible?

[elpaso]

Have you considered using the QgsSettings::Section and a single registry instead of having separate classes for core, gui etc.?

[nyalldawson]

The approach used here is considerably different to the approach described by the qep. Can you explain why this is?

[domi4484]

Have you considered using the QgsSettings::Section and a single registry instead of having separate classes for core, gui etc.?

@elpaso Yes but the problem is that if we distribute the settings structures in the appertaining classes, the single "global" register would have to include headers from different sections making it difficult to build for example only core.
Although it would be possible to implement a settings register global in which settings register sections could register themself.

The approach used here is considerably different to the approach described by the qep. Can you explain why this is?

@nyalldawson During implementation I noticed that with this approach we could reduce the use of literals to the declaration of the settings. With advantages like reduction of typos and easier refactoring. It is somewhat like @sbrunner propose in his comment.

Moreover I have seen that a lot of setting path are built dynamically, and we solved the problem of declaring such settings with a map like settings group QgsSettingsGroupMap. With the qep proposed approach we would probably had to use some special placeholder when registering the settings. Also possible but a little more complicated.

Another advantage that I didn't mention before is that the programmer is implicitly forced to group settings which belong together resulting in paths like:
locator_filters/FILTERNAME1/prefix
locator_filters/FILTERNAME1/enable
locator_filters/FILTERNAME2/prefix
locator_filters/FILTERNAME2/enable
which is in my opinion preferable than:
locator_filters/prefix_FILTERNAME1
locator_filters/enable_FILTERNAME1
locator_filters/prefix_FILTERNAME2
locator_filters/enable_FILTERNAME2

[elpaso]

Have you considered using the QgsSettings::Section and a single registry instead of having separate classes for core, gui etc.?

@elpaso Yes but the problem is that if we distribute the settings structures in the appertaining classes, the single "global" register would have to include headers from different sections making it difficult to build for example only core.
Although it would be possible to implement a settings register global in which settings register sections could register themself.

I don't follow: isn't the other way round?

A GUI class would call the global settings register and register the setting in a particular section (Gui in this case).
It would be the GUI class that needs to include the global settings header, not the opposite.

I suggest that we close this PR and go back to the QEP if the implementation you are proposing here is substantially different.

[3nids]

With this approach, the settings are members of the registry, declared at compile time. This means you cannot dynamically register the settings.

I don't follow: isn't the other way round?
A GUI class would call the global settings register and register the setting in a particular section (Gui in this case).
It would be the GUI class that needs to include the global settings header, not the opposite.

The settings (QgsSettingEntry and groups) are defined where they belong (be it the locator filters, the layout, map canvas, map tools, etc.). To be added to the registry, the reigstry needs to include their definition.
If we want to have a single registry, this means we have to #ifdef GUI, DESKTOP, etc. I have no preference at the moment between the 2 approaches.

I suggest that we close this PR and go back to the QEP if the implementation you are proposing here is substantially different.

I think that @domi4484 brought a very interesting approach here and it deserves a proper discussion.
It has pros and cons, and I'd like to get opinions before reverting to the original idea.
We can move the discussion to the QEP, but I think it makes sense to discuss it here along the small code examples.

[elpaso]

With this approach, the settings are members of the registry, declared at compile time. This means you cannot dynamically register the settings.

Ok, thanks for the clarification, I was totally missing this point. It seems a pretty big limitation to me but whatever it takes to bring some order to the current anarchy is deeply welcome.

So, if basically the whole register is "static" and generated at compile time, how would you solve the problem with dynamically loaded modules such as the providers, the server modules and all the plugins (C++ and python)? They are not known at compile time. You might argue that they do not need to use settings at all (providers) or they can use Q(gs)Settings directly but this would bring it back the anarchy problem.

I don't follow: isn't the other way round?
A GUI class would call the global settings register and register the setting in a particular section (Gui in this case).
It would be the GUI class that needs to include the global settings header, not the opposite.

The settings (QgsSettingEntry and groups) are defined where they belong (be it the locator filters, the layout, map canvas, map tools, etc.). To be added to the registry, the reigstry needs to include their definition.
If we want to have a single registry, this means we have to #ifdef GUI, DESKTOP, etc. I have no preference at the moment between the 2 approaches.

I suggest that we close this PR and go back to the QEP if the implementation you are proposing here is substantially different.

I think that @domi4484 brought a very interesting approach here and it deserves a proper discussion.
It has pros and cons, and I'd like to get opinions before reverting to the original idea.

We can move the discussion to the QEP, but I think it makes sense to discuss it here along the small code examples.

I'm not talking about reverting, just that I find a QEP much more readable than a PR but maybe it's just me.

[domi4484]

With this approach, the settings are members of the registry, declared at compile time. This means you cannot dynamically register the settings.

Ok, thanks for the clarification, I was totally missing this point. It seems a pretty big limitation to me but whatever it takes to bring some order to the current anarchy is deeply welcome.

I agree that it seems a limitation at first impact, but it is really needed to create new settings at runtime? Can you show me some example where it is done? Maybe there is also a solution for such cases.

So, if basically the whole register is "static" and generated at compile time, how would you solve the problem with dynamically loaded modules such as the providers, the server modules and all the plugins (C++ and python)? They are not known at compile time. You might argue that they do not need to use settings at all (providers) or they can use Q(gs)Settings directly but this would bring it back the anarchy problem.

The whole register is accessed on a static way but it is instantiated at the beginning of QgsApplication::ApplicationMembers::ApplicationMembers().

Other modules and plugins would instantiate his own settings register on load. They could get an QgsApplication::settingsRegistryGlobal() instance as parent. The global register would have all other registers like Core, Gui, Plugin1, Plugin2, ... as children and act as a single point to generate lists of all settings or ini files.

[3nids]

Other modules and plugins would instantiate his own settings register on load. They could get an QgsApplication::settingsRegistryGlobal() instance as parent. The global register would have all other registers like Core, Gui, Plugin1, Plugin2, ... as children and act as a single point to generate lists of all settings or ini files.

I'd go a bit further and suggest that all modules could be registered in the main one. Therefore, we would get introspection for all settings, such things as providers are not an issue, as they register their registries themselves to the main one.
For such registration, we could easily reuse the QgsSettings::Section enum.

Plugins would also declare their own group of settings which would also be registered to the main group.

[nyalldawson]

I have quite strong reservations about the compiled-in-approach. I can see the motivation here, but I just think the overhead and maintainance burden will be too great, and we could get similar levels of safety through an assert instead.

I also want to see a guarantee that no setting keys will change as a result of this work, so that any plugins or startup scripts which set the existing key values will continue to work without issue. A huge number of plugins directly set core/gui/app qgssettings values in order to control the behaviour of QGIS, and organisations have invested a lot of resources in configuring their internal startup scripts with code which sets settings value for the organisational needs. We can't break this, so need to ensure that no actual qsettings keys are changed in the process of this work.

[elpaso]

With this approach, the settings are members of the registry, declared at compile time. This means you cannot dynamically register the settings.

Ok, thanks for the clarification, I was totally missing this point. It seems a pretty big limitation to me but whatever it takes to bring some order to the current anarchy is deeply welcome.

I agree that it seems a limitation at first impact, but it is really needed to create new settings at runtime? Can you show me some example where it is done? Maybe there is also a solution for such cases.

A good example of settings that are dynamically created/destroyed at runtime are all the connections (postgis etc.).

The connections API should in theory take care of the connections (un)mashalling but in practice most of data providers do that in their own way by calling QgsSettings directly but this doesn't change a bit in this context because the connections API would use QgsSettings anyway.

In any event, I think it's better if we discuss about architectural changes in a QEP instead of a PR.

[domi4484]

Thank you all for the comments and the discussions by side.
The concerns raised are very valid (mainly compilation overhead) and have proven this approach to be too costly.
We will close this PR and propose a new alternative on the QEP closer to the original approach.