Edgewall Software
Modify

Ticket #2019 (closed defect: fixed)

Opened 6 years ago

Last modified 6 years ago

Configuration section disabled_components uses negative logic

Reported by: trac@… Owned by: cmlenz
Priority: low Milestone: 0.9
Component: general Version: 0.9b1
Severity: minor Keywords: disabled_components trac.ini
Cc: trac@…
Release Notes:
API Changes:

Description

The [disabled_components] section of trac.ini uses the form:

[disabled_components]
trac.versioncontrol.web_ui = yes

It may be cleaner, especially for non-native English speakers, to use disabled, rather than yes, as the value indicating that a component should be disabled. In general, asking negative questions should be avoided, am I wrong? </example>

This should be a trivial change/fix with a small, but tangible benefit.

Attachments

Change History

comment:1 Changed 6 years ago by eblot

+1

In such a case, I think the section title would be better changed as well, e.g.

[core_components]
trac.versioncontrol.web_ui = disabled

comment:2 Changed 6 years ago by cmlenz

The list isn't only about core components, it's about any component, also those provided by plugins.

Say I installed some plugin that provides some feature I don't want, nicely tucked away into a component. I can disable that component to get rid of the unwanted feature, just as I can disable a component that comes with Trac.

Maybe it'd be better if any component listed under [disabled_components] would be treated as disabled, regardless of value. But I think that'd actually be more confusing than the current solution.

comment:3 Changed 6 years ago by eblot

So why simply not:

[components]
trac.versioncontrol.web_ui = disabled
custom.plugin.extension = disabled

disabled or off, no, and any boolean value that evaluate to false as defined in util.py

comment:4 Changed 6 years ago by anonymous

  • Cc trac@… added

comment:5 Changed 6 years ago by trac@…

I'm of the mindset that it is best to include the actual desired state in the value specified. That is, enabled/disabled or on/off should be the only accepted values. This indicates the desire without referencing the stateful intention of the section. Maybe these are minor semantics, but I like to know what is happening on a given line without having to ask whether yes means enabled or disabled.

I'm also a big fan of extending the section to the larger general component scope for the exact reasons you mention.

As a note, especially when setting up and testing, I like to adjust my configs by doing things like this, where switching consists of commenting the alternate line:

[components]
#some.extension.being.tested = enabled
some.extension.being.tested = disabled
some.alternative.extension = enabled
#some.alternative.extension = disabled

comment:6 Changed 6 years ago by cmlenz

  • Owner changed from jonas to cmlenz
  • Status changed from new to assigned

I've implemented the following approach here:

Given a trac.ini containing:

[components]
trac.ticket.web_ui.UpdateDetailsForTimeline = false
trac.versioncontrol.* = false
webadmin.* = true

Would enable all the core components except for the explicitly disabled trac.ticket.web_ui.UpdateDetailsForTimeline and everything in the trac.versioncontrol package. In addition, it would enable every component in the webadmin package.

Note that trac.* = true is implicit in the configuration.

comment:7 Changed 6 years ago by trac@…

I love it. The wildcards should be very handy.

comment:8 Changed 6 years ago by cmlenz

I've missed your previous comment, but I do actually agree on using proper value names. My current implementation interprets both “enabled” and “on” as legal values for enabling a component. Anything else is considers to disable the component.

About the wildcards, they only work at the end of the string. I figured that was the only place they would actually make sense in practice. But by actually requiring the wildcard at the end to do name-prefix matching, the meaning of those options becomes a lot clearer than by just doing the prefix matching implicitly (as it is now).

comment:9 Changed 6 years ago by cmlenz

  • Resolution set to fixed
  • Status changed from assigned to closed

Implemented in [2335].

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
to The owner will be changed from cmlenz. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.