Hide configuration options for disabled components

[Remy Blank]

Configuration options (subclasses of Option) are registered in a global registry (Option.registry) so that the default values can be retrieved, and also for the TracIniMacro to display the documentation.

The registry lists options for enabled and disabled components alike. Hence, the TracIniMacro also shows documentation for disabled components. This is undesirable, especially if we have components in tracopt that are disabled by default.

The idea is to modify the TracIniMacro so that it only lists options for enabled components. Moreover, the defaults written to trac.ini on environment creation and trac.ini.sample on upgrade should also only contain options for enabled components.

[osimons]

Fully agree with fixing both issues as suggested.

[Remy Blank]

Hide config options for disabled components.

[Remy Blank]

The patch above hides configuration options for disabled components:

• in the output of the [[TracIni]] macro

• in the configuration displayed on /about

• in trac.ini for a newly-created environment, and in trac.ini.sample for an upgraded environment.

The change was pretty heavy, as configuration objects have no notion of the environment. The basic idea is to add an optional compmgr argument (a ComponentManager, usually the environment object) to all iteration methods, and to filter out options belonging to disabled components if it is specified.

Thoughts?

[Christian Boos]

This is a good change, except for one thing, not writing the default settings for disabled components in the trac.ini.sample file.

I think we should keep writing the options there, because at upgrade time, we don't know if the user really wants to keep those components disabled or is going to enable them the moment after. If the user does so, then he should be able to benefit from the presence of the related settings in the sample ini file.

Some comments on the patch itself:

In about.py, as you've touched the sort lines, instead of:

sections.sort(lambda x, y: cmp(x['name'], y['name']))

prefer:

sections = sections.sorted(key=lambda s: s['name'])

In config.py, I think get_registry(compmgr=None) should be better called get_enabled_options(compmgr), it's clearer and you can simplify the docstring and remove the test for compmgr. If we need all the options, we can keep using Option.registry.

[Remy Blank]

Thanks for your comments!

Ok for keeping the options for disabled components in trac.ini.sample.

About the sort lines, I actually only "normalized" some missing whitespace. But you're right, as long as I'm touching the code, I may as well clean it up. I'll have to do something like that, though (I don't think a list has a .sorted() method:

sections = sorted(sections, key=lambda s: s['name'])

About get_registry(), I would rather keep it as-is, as it's called from two locations with an argument where we don't know a-priori if it's None or not. If I removed the argument, I would have to add the check in those two locations, and decide between calling get_enabled_options() or getting Option.registry.

But maybe get_options() would be a better name?

[Christian Boos]

Replying to rblank:

Thanks for your comments!

Ok for keeping the options for disabled components in trac.ini.sample.

Perfect.

I'll have to do something like that, though (I don't think a list has a .sorted() method:

Sorry, I was in mixed Ruby/Python mode ;-)

About get_registry(), I would rather keep it as-is, as it's called from two locations with an argument where we don't know a-priori if it's None or not. If I removed the argument, I would have to add the check in those two locations, and decide between calling get_enabled_options() or getting Option.registry.

Ok, it's not as clear cut as I thought, so we can as well keep get_registry().

[Remy Blank]

Replying to cboos:

Sorry, I was in mixed Ruby/Python mode ;-)

What!?! Traitor! :-)

(I'll have to try Ruby some day, looks pretty interesting.)

[Remy Blank]

Updated patch applied in [8686].