Opened 15 years ago
Closed 8 years ago
#6551 closed defect (fixed)
IntOption and BoolOption values saved even when not differing from default
|Reported by:||Owned by:||Ryan J Ollos|
Fixed incorrect highlighting of unmodified values in the Configuration section of the About Trac page.
Configuration.save method fails at checking whether IntOptions and BoolOptions differ from their default values. So when calling
Configuration.save, if a value has been set on an IntOption or BoolOption, that value is saved to the file even if it is the same as the default for that option, which is not the desired behavior I don't think.
Change History (20)
comment:1 by , 15 years ago
comment:2 by , 15 years ago
I also noticed that there is some inconsistency with the default values from
BoolOptions. For most of them, the defaults are set to
'false' as it would appear in the file.
color_scale's default in
True., so with my suggested fix this would be returned as
u'True', which is still not good.
comment:3 by , 15 years ago
Suggesting a milestone for this since it seems to have gotten lost in the pile.
comment:4 by , 15 years ago
I'll put it on my to-do.
by , 15 years ago
I'm not entirely sure how I feel about this patch, but it does fix the problem as described. Ensures that
Section.get() always returns a unicode string, and currectly returns true/false for BoolOptions and returns an integer string for IntOptions (even if the value is 0).
by , 15 years ago
A new version of the patch. I found that if an option's default value is
Option.__get__ would return
u'None' which is certainly no good. I'm still not sure I like this fix, but it gets the job done, and always returns the correct value now.
comment:5 by , 14 years ago
|Milestone:||0.11-retriage → 0.11.4|
comment:6 by , 14 years ago
comment:7 by , 14 years ago
|Priority:||low → normal|
|Severity:||minor → normal|
comment:8 by , 9 years ago
|Milestone:||next-minor-0.12.x → next-stable-1.0.x|
comment:9 by , 8 years ago
|Milestone:||next-stable-1.0.x → 1.1.5|
|Status:||new → assigned|
by , 8 years ago
|Attachment:||Screen Shot 2015-04-14 at 8.44.29 PM.png added|
comment:10 by , 8 years ago
Related issue. Several
BoolOptions are shown as modified on the About page though they still have the default value:
comment:11 by , 8 years ago
Section.get states that it will return a string, so it seems clear that is a defect we should fix. We could use
Option.dumps to convert the default option value to a string in
Section.get. I'm debating whether
Section.defaults should also return a dictionary of strings. Alternatively,
Option.dumps could be called in
The proposed changes are captured in log:rjollos.git:t6551. I'm still working to understand the steps required to produce the problem reported in comment:description.
comment:12 by , 8 years ago
I'm considering moving Environment.get_configinfo to a function of the
config module since it's not generally useful as a method of the
Environment class. Locating the code in the
config module could also make maintenance easier.
comment:13 by , 8 years ago
Option.dumps() with True and False serializes
BoolOption which has
'true' as default is set, Trac determines the value is different from the default.
$ ~/venv/trac/1.1.4/bin/python Python 2.6.9 (default, Oct 22 2014, 19:47:46) [GCC 4.6.3] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> from trac.test import EnvironmentStub >>> from trac.loader import load_components >>> from trac.config import Option, BoolOption >>> env = EnvironmentStub() >>> load_components(env) >>> set(o.default for o in Option.get_registry().itervalues() if isinstance(o, BoolOption)) set([False, 'true', 'false', True])
It would be good to add
'enabled'. Additional proposed changes in email@example.com.
comment:14 by , 8 years ago
Thank you for the additional changes. I'll rebase the changes on the trunk now that #11982 is committed.
comment:15 by , 8 years ago
Related to the comment in #11437 about having a
trac-admin $ENV config clean command, it could be useful to highlight the rows on the About page differently when the option is not defined in any enabled Component. We'd need special handling for options that only have a
ConfigSection, such as
ticket-workflow, or somehow indicate that arbitrarily defined options are allowed for a given section.
comment:16 by , 8 years ago
Minor refactoring of
config module committed in [14017:14018].
comment:17 by , 8 years ago
|API Changes:||modified (diff)|
|Release Notes:||modified (diff)|
|Status:||assigned → closed|
It looks like the actual problem here is that
Section.getand by extension
Configuration.getdon't always return a string as documented.
The problem is that if the option is not in the trac.ini file, or the global trac.ini file, its default value is retrieved from the Option registry. In the case of an
IntOption, for example, the default value will be an integer. The problem is in how the value is returned. From
This leads to all kinds of inconsistencies. For example, if the default value for the option is
0, this returns
u'0'as one would expect. Or if the default value is
100000it will return
100000as an int.
It should probably be something more like this: