#3425 closed enhancement (fixed)
WebAdmin should be more conservative with trac.ini
Reported by: | Emmanuel Blot | Owned by: | Christian Boos |
---|---|---|---|
Priority: | normal | Milestone: | 0.10 |
Component: | admin/web | Version: | devel |
Severity: | minor | Keywords: | |
Cc: | Branch: | ||
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
WebAdmin changes the contents of the trac.ini
config file to reflect the configuration changes made by the user.
However, it does not preserve the original line ordering (nor the section ordering).
This makes the file hard to read for sections with a lot of options, especially with options which are tied to the same feature (e.g. login
and password
) and it makes the comparison of config files of different projects hosted on the same server a nightmare.
WebAdmin should ideally preserve the original ordering, or at least sort the options in lexical order.
Attachments (2)
Change History (19)
comment:1 by , 18 years ago
comment:2 by , 18 years ago
I was thinking about augmenting the ConfigParser class, so that it preserves the original order of trac.ini
file
For example, the ordered dictionary (BSD license) could be used to store the sections and options rather than in the default Python dictionary used by the ConfigParser.
I'm attaching a simple proof-of-concept patch for the current trunk. I made a couple of tests with WebAdmin and the trac.ini
looked ok. I also ran the non-regression test suite. Basically, the patch:
- adds the
odict.py
module - overrides the
__init__
,add_section
and_read
methods of the ConfigParser - replaces the ConfigParser used in the
config.py
file with an OrderedConfigParser - changes the way the
trac.ini
file is built (filter out options from the existing parser rather than building a new one)
I did not find another way but overrides the _read
method of the ConfigParser for a single-line change ;-(
The patch is a bit overkill for a "simple" add-on like this on.
by , 18 years ago
Attachment: | orderedconfig.patch added |
---|
Ordered config parser (w/o the odict.py), for [3542]
comment:3 by , 18 years ago
BTW, the odict.py
file can probably skimmed off, to keep the relevant methods used by ConfigParser and discard the useless ones.
comment:4 by , 18 years ago
Maybe the approach is naive, but wouldn't it be simpler to just
write our own "save" method?
We sort the config sections and entries and write a few key = value
lines, and voila ;)
Is there something I don't see?
comment:5 by , 18 years ago
(rah, the reply feature is really in trouble w/ Safari). Anyway:
wouldn't it be simpler to just write our own "save" method?
A custom 'save' method would allow to define the order of the lines in the config file, but you first need to know the original order: the user may reorganise the file for his needs, there are several plugin that are now using the configuration file and a "save" method would definitely change the order of the plugin sections - which makes the comparison of projects more difficult.
Or, I missing an important point. Anyway, if there is a simpler method, I'm +1.
comment:6 by , 18 years ago
WebAdmin should ideally preserve the original ordering, or at least sort the options in lexical order.
I was targeting the "at least" part and you the "ideally" part ;)
I think that if the order remains consistent, that's good enough. If we go the "ideal" way, then I would really like that we also preserve comments.
comment:7 by , 18 years ago
Milestone: | → 0.10 |
---|---|
Owner: | changed from | to
To illustrate what I was saying above: attachment:config_save_in_lexical_ordering-r3538.patch
This makes it easy to compare trac.ini files saved that way. Note also that new configuration files will be created that way too, so the loss of the "original" order will only affect existing installations.
by , 18 years ago
Attachment: | config_save_in_lexical_ordering-r3538.patch added |
---|
Use simply the lexical ordering of sections and options within each sections, instead of trying to preserve the original ordering
comment:8 by , 18 years ago
Keywords: | review added |
---|
follow-up: 10 comment:9 by , 18 years ago
It seems ok, although it makes me think about a related question:
How do the configuration code cope with unicode values (for characters not in ASCII set)?
It seems the ConfigParser does not make any character set translation at all. So when we read UTF-8 from the ini file, the configuration parser uses utf-8 values. What about if we place unicode characters in the configuration parser and call the save()
method? It seems that neither the original parser method or the patch method can cope with Unicode. Am I wrong?
I don't see where unicode-to-utf8 translation is made for the configuration file.
BTW: it there any reason why the 'top level' scripts in /scripts
directory are kept encoded using iso-8859-1
rather than utf-8
as the rest of the code? ex:
file -i scripts/trac-admin scripts/trac-admin: text/plain; charset=iso-8859-1
follow-up: 11 comment:10 by , 18 years ago
Keywords: | review removed |
---|---|
Owner: | changed from | to
Priority: | lowest → normal |
Status: | new → assigned |
Replying to eblot:
It seems the ConfigParser does not make any character set translation at all. So when we read UTF-8 from the ini file, the configuration parser uses utf-8 values. What about if we place unicode characters in the configuration parser and call the
save()
method? It seems that neither the original parser method or the patch method can cope with Unicode. Am I wrong?
Partly: it works fine with Unicode values, but the current way to do the save()
is not optimal: I first did the correct thing to do, i.e. to_unicode(value).encode('utf-8')
, but then overwrote the patch with a simpler one, which just used value
. That second solution ensured that the original charset used for the trac.ini
file would be preserved. But OTOH, when a config value is programmatically changed (as with WebAdmin), the encoding used will be "utf-8"
(see Section.set()
) and therefore we would eventually end with a mix of charsets, which is not good ;)
So I think it's not that important if the original charset is lost, and "utf-8" is used instead. I'll add a # -*- coding: utf-8 -*-
first line to the file, to make that clear.
Also, there's no support yet for Unicode keys and section names, as I think this would have been overkill.
BTW: it there any reason why the 'top level' scripts in
/scripts
directory are kept encoded usingiso-8859-1
rather thanutf-8
as the rest of the code? ex:
Oversight, probably…
follow-up: 12 comment:11 by , 18 years ago
Owner: | changed from | to
---|---|
Status: | assigned → new |
Replying to anonymous:
Is anonymous eq cboos ?
Partly: it works fine with Unicode values,
but "%s" % (unicode_value) generates a UnicodeEncodeError, doesn't it?
the encoding used will be "utf-8"
I fully agree.
I'll add a
# -*- coding: utf-8 -*-
first line to the file, to make that clear.
You mean in the trac.ini file? Make it less pythonic (users/admin don't have to care about the language and naming conventions used for Trac), something like
; charset: UTF-8
Also, there's no support yet for Unicode keys and section names, as I think this would have been overkill.
Plugins may need non-ascii values. A typical case is the th:TracFrenchTranslation which stores the translated term in the ini file. The configuration file should support UTF-8 natively (but no other encoding scheme).
comment:12 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Sorry, I applied attachment:config_save_in_lexical_ordering-r3538.patch (improved the way described in comment:10) as r3556, before seeing your reply…
Replying to eblot:
Replying to anonymous: Is anonymous eq cboos ?
Sure ;)
but "%s" % (unicode_value) generates a UnicodeEncodeError, doesn't it?
no, it gives you back unicode_value
(i.e. is equivalent to u"%s"
)
I'll add a
# -*- coding: utf-8 -*-
first line to the file, to make that clear.You mean in the trac.ini file? Make it less pythonic (users/admin don't have to care about the language and naming conventions used for Trac), something like
It's not (only) a Trac or Python convention, it's an Emacs convention and I believe Emacs is widely used.
Also, there's no support yet for Unicode keys and section names, as I think this would have been overkill.
Plugins may need non-ascii values. A typical case is the th:TracFrenchTranslation which stores the translated term in the ini file. The configuration file should support UTF-8 natively (but no other encoding scheme).
Ok, I'll think about adding this later.
comment:13 by , 18 years ago
It's not (only) a Trac or Python convention, it's an Emacs convention and I believe Emacs is widely used.
Administrators from the Windows world are probably using other kinds of editors. I work/develop on Win/Linux/Mac, I never used it. Btw, ';' is more common in ini files than '#'. Not a big deal though ;-)
comment:14 by , 18 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
The unit test fails on Mac OS X (10.4.7/Intel), with both Python 2.3.5 and Python 2.4.3. It fails for Latin-1 strings:
amazas:~/Sources/Svn/edgewall.com/trunk eblot$ python trac/tests/config.py ............F ====================================================================== FAIL: test_set_and_save (__main__.ConfigurationTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "trac/tests/config.py", line 123, in test_set_and_save self.assertEquals(u"Voilà l'été", config.get('a', 'option3')) AssertionError: u"Voil\xe0 l'\xe9t\xe9" != u"Voil\u2021 l'\xc8t\xc8" ---------------------------------------------------------------------- Ran 13 tests in 1.022s FAILED (failures=1)
Do we need to support Latin-1 encoding?
comment:15 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Ok, that's fixed now in r3560.
follow-up: 17 comment:16 by , 18 years ago
hi i was recently looking to solve the same problem for my own development, and i came across "configobj", which is basically like configparser, but better in many respects, including the preservation of original file structure, and even comments. check it out, here: http://www.voidspace.org.uk/python/configobj.html
so you may want to try using it instead of the default configparser module, so that you can build on the shoulders of others, rather than hacking together your own solution. :)
comment:17 by , 18 years ago
Replying to nanotube:
hi i was recently looking to solve the same problem for my own development, and i came across "configobj", which is basically like configparser, but better in many respects, including the preservation of original file structure, and even comments. check it out, here: http://www.voidspace.org.uk/python/configobj.html
so you may want to try using it instead of the default configparser module, so that you can build on the shoulders of others, rather than hacking together your own solution. :)
ConfigObj saves in order and has full unicode support.
The writing behavior is how the Python
ConfigParser
module operates. I don't know if there are any open requests to fix this in the Python library, but I suppose it wouldn't be available soon.ConfigObj supports a super-set of the ini-style format currently used, though I don't know if it has any better saving behavior.