WebAdmin should be more conservative with trac.ini

[eblot]

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.

[mgood]

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.

[eblot]

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 _readmethod of the ConfigParser for a single-line change ;-(

The patch is a bit overkill for a "simple" add-on like this on.

[eblot]

Ordered config parser (w/o the odict.py), for [3542]

[eblot]

BTW, the odict.py file can probably skimmed off, to keep the relevant methods used by ConfigParser and discard the useless ones.

[cboos]

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?

[eblot]

(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.

[cboos]

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.

[cboos]

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.

[cboos]

Use simply the lexical ordering of sections and options within each sections, instead of trying to preserve the original ordering

[eblot]

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

[anonymous]

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 using iso-8859-1 rather than utf-8 as the rest of the code? ex:

Oversight, probably...

[eblot]

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).

[cboos]

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.

[eblot]

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 ;-)

[eblot]

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?

[cboos]

Ok, that's fixed now in r3560.

[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. :)

[anonymous]

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.

Fuzzyman http://www.voidspace.org.uk/python/configobj.html