On Mon, Dec 30, 2002 at 08:43:55PM -0500, Eric S. Raymond wrote:

I've got another fix and some enhancements in the works for ConfigParser. Passes all regression tests, but I'm field-testing before I commit. Just because I'm nervous about patch collisions, what code did your patch affect?

I added class InterpolationSyntaxError(Error). Fred quickly fixed it :-), added a bunch of docstrings, fixed the constructor call I noted, and added some tests. Neal

Neal Norwitz :

...

On Mon, Dec 30, 2002 at 08:43:55PM -0500, Eric S. Raymond wrote:

...

I've got another fix and some enhancements in the works for ConfigParser. Passes all regression tests, but I'm field-testing before I commit. Just because I'm nervous about patch collisions, what code did your patch affect?

I added class InterpolationSyntaxError(Error). Fred quickly fixed it :-), added a bunch of docstrings, fixed the constructor call I noted, and added some tests.

Doesn't sound like there should be a problem. Most of my changes are to the _read logic -- I also added a bunch of regression tests for the new features, but I did it in such a way that collision seems unlikely. I'll check them in today or tomorrow and make sure everything is copacetic.

Eric, can you put off your checkin until after I've created the 2.3a1 release branch? --Guido van Rossum (home page: http://www.python.org/~guido/)

Guido van Rossum :

...

Eric, can you put off your checkin until after I've created the 2.3a1 release branch?

Ouch. Our mails crossed. I just checked them in. You can back them out if you're feeling really paranoid -- but all the unit tests pass.

I find it hard to believe that our mails crossed. The first time I asked you to wait until after the release was last night: | Subject: Re: [Python-Dev] bugs in stdlib | From: Guido van Rossum | To: esr@thyrsus.com | Date: Mon, 30 Dec 2002 20:58:25 -0500 | | > I've got another fix and some enhancements in the works for | > ConfigParser. Passes all regression tests, but I'm field-testing | > before I commit. Just because I'm nervous about patch collisions, | > what code did your patch affect? | | CVS is your friend, Eric. :-) | | Can you put off checking in your changes until after the 2.3a1 | release tomorrow? I'm a bit nervous about last-minute changes to | ConfigParser, based on the history of that module. | | --Guido van Rossum (home page: http://www.python.org/~guido/) If you saw that and chose to ignore my advice, I would've appreciated a reply at least. I'll leave the decision to revert your checkin to Fred, who knows the code (and has pending changes to it of his own, which now cause conflicts). --Guido van Rossum (home page: http://www.python.org/~guido/)

Eric S. Raymond :

Guido van Rossum :

...

If you saw that and chose to ignore my advice, I would've appreciated a reply at least.

I did not. I would have heeded that advice.

On reflection, more explanation seems called for... I've been flooded in a particularly repellent way recently. Somebody is using my From address for spam mail about quack remedies for prostate trouble, somebody so illiterate that the Subject line read "urnination flow". As a result, I've been getting several *thousand* bounce messages a day from sites rightly rejecting this crud, and hitting the 'd' key in rapid-fire mode. I may have inadvertently flushed your mail, dammit, which is not something I would ever do deliberately. I subjunctively apologize. You may not be alone, I fear. I almost blew past mail from Dennis Ritchie last night, just barely caught myself in time. I'm wondering now what other important bits I lost... :-( :-( :-( I do not regard the ConfigParser changes as at all urgent -- the bug is a minor one. So I won't be offended if Fred chooses to back them out. -- <a href="http://www.tuxedo.org/~esr/">Eric S. Raymond</a>

Eric> I've been flooded in a particularly repellent way recently. Eric> Somebody is using my From address for spam mail about quack Eric> remedies for prostate trouble, somebody so illiterate that the Eric> Subject line read "urnination flow". As a result, I've been Eric> getting several *thousand* bounce messages a day from sites Eric> rightly rejecting this crud, and hitting the 'd' key in rapid-fire Eric> mode. Sounds like you need to install spambayes or bogofilter. ;-) Skip

Fred L. Drake, Jr. :

Regarding Eric's changes, I'd like to see them discussed before being made part of the library; the interface seems quite ad hoc, and it's not clear that they are the right way to go about adding sequence support.

Do you have the same reaction to the multiline string support? And do you want the line-continuation bugfix?

The value of having __str__() return a parsable INI file is highly questionable; the motivation should be explained and discussed.

Cleanliness. I believe in invertible representations. Whenever I write a class, I like to have its str() or repr() emit something parseable. If nothing else, this is useful for debugging. This is just general principles, I don't (unlike my other changes) have a use case for it.

The issue of ordering sections and options in the same way as the input seems unimportant as well; comments are lost anyway. Being able to surgically edit a config file using the ConfigParser module is simply not a supported use case.

It is now. That was the point of the enhancements.

I'll be reverting the change shortly. Eric, I would like to encourage you to pursue this functionality via discussion on python-dev with the patch submitted via SourceForge. The functional extensions are not unwelcome by any means.

OK. Then let the discussion begin. Here are the issues: 1. There is a minor bug in the way continuations are handled that, in some circumstances, can mean the write() representation is not invertible. 2. Currently ConfigParser cannot support multiline strings -- that is, values that are multiline and have embedded whitespace. I need this capability. I added support for this through a class member that lets you declare string quotes. Is there some objection to supporting this value type, or just to the magic-class-member interface? 3. Currently ConfigParser cannot support structured values. Again, I need this capability (for association lists of coordinates and names, as it happens). The syntax I have implemented is a configurable list bracket character surrounding comma-separated lists. The alternative Fred suggests is, I think, extremely ugly. If anyone has a more natural suggestion than my proposal, I'm willing to hear it. 4. I *do* in fact want to support `surgical' alteration of .INI files. I have a use case for this in a configuration editor I'm writing for FreeCiv. More generally, when writing code to support invertible representations, I think it is good citizenship to perturb the original data as little as possible. Thus I regard the fact that the present code permutes options and sections into an arbitrary order dependent on the hash table implementation as a design bug, and actually a fairly serious sort of thoughtlessness. -- <a href="http://www.tuxedo.org/~esr/">Eric S. Raymond</a>

Guido van Rossum :

I don't like classes whose str() or repr() can be many lines in a typical case. Since this is read from a file, there should be (and is) a method to write it to a file; if there's no use case for converting the whole shebang to a string, don't provide that, and especially not as str() or repr().

Noted for the futuee, thanks. -- <a href="http://www.tuxedo.org/~esr/">Eric S. Raymond</a>

Fred L. Drake, Jr. :

That would be nice to have; I should have responded to that point separately. Feel free to commit a minimal patch to fix that, and add a regression test to Lib/test/test_cfgparser.py.

On my to-do list.

I don't think it's clear that using __str__() for this is valuable. Creating a potentially large string in memory may not be the best approach; I actually like the write() method better; the caller can decide to use a StringIO if that's what makes sense, or toss things to a real file if needed.

Guido has already made this point. As a matter of taste, I like the serialization operation to be separate from I/O. I think that's cleaner, and in this case the in-memory string is unlikely to get large. But I don't actually need this change, so I won't argue for it against opposition.

Perhaps I missed it; did you preserve comments as well?

No. But comments are fair game to be discarded, since they're not part of the data content. In my use case they're not important.

I think the interface should be discussed; your implementation may be sufficient, but after working with pyexpat as much as I have, I've developed a strong dislike for attributes that have substantial side effects.

A fair point. I would be happy to change the interface away from a magic member to a real method. Likewise for the structured-value stuff.

If we're going to add a special syntax for multi-line values, we may need to think about encoding special values, text encoding, and Unicode. And that's never easy to get concensus on. ;-)

Then we shouldn't try. Let's not let the ideal be the enemy of the good here; the multiline-literal syntax is othorgonal to how we handle string encoding, and internationalization can be layered on it later.

...

3. Currently ConfigParser cannot support structured values. Again, I need this capability (for association lists of coordinates and names, as it happens). The syntax I have implemented is a configurable list bracket character surrounding comma-separated lists. The alternative Fred suggests is, I think, extremely ugly. If anyone has a more natural suggestion than my proposal, I'm willing to hear it.

I think "suggests" is too strong a word; I was only citing precedence, not advocating that approach. I think it's pretty ugly as well.

I'm taking suggestions on how to do it right. I don't think there's any obviously better way than what I've implemented, except maybe for making the trigger a real method rather than a magic member.

...

4. I *do* in fact want to support `surgical' alteration of .INI files. I have a use case for this in a configuration editor I'm writing for FreeCiv. More generally, when writing code to support invertible representations, I think it is good citizenship to perturb the original data as little as possible. Thus I regard the fact that

I think there's a distinction between reading a configuration and editing it: The original code never wrote a file back out. That was something you added in revision 1.19.

Right, because I needed it.

Seriously, I have no objection to supporting surgical editing. I do think that separate classes should be used for read-only uses and read-write uses; a read-only ConfigParser-thingie should remain very lightweight, and surgical editing just isn't that.

I had this conversation with Guido a couple years back. I too would have been happier with two classes, one read-only and one that supports writing back out. Here is whatt Guido said and I replied: --------------------------------------------------------------------------- Guido van Rossum :

One possible suggestion (just for your contemplation): if you're only adding methods, would it make sense to make a subclass, so that from the class one uses it is clear whether one intends to modify the options or just to read them? That would enable future optimizations.

Yes, and I even know what I'd call the class: ConfigEditor. Unfortunately this plan it trips on a historical fact; one (1) of the write methods, add_section(), already existed in ConfigParser. Moving it to a ConfigEditor subclass would break backward compatibility. And I think it would be just too ugly to move all but one of the write methods into ConfigEditor and leave add_section() in the base class. If not for this problem I would be more than happy to do as you suggest. --------------------------------------------------------------------------- Guido dropped the thread, which I took to mean that he saw the force of the objection. But I'd *still* like to do as he suggested. If the consensus is that its OK to deprecate add_section() and write() in the base class, I'll write ConfigEditor and move all my surgery stuff over there in a heartbeat. -- <a href="http://www.tuxedo.org/~esr/">Eric S. Raymond</a>

Eric S. Raymond writes:

No. But comments are fair game to be discarded, since they're not part of the data content. In my use case they're not important.

I don't think they're fair game if we want a surgical implementation. Neither are blank lines.

...

If we're going to add a special syntax for multi-line values, we may need to think about encoding special values, text encoding, and Unicode. And that's never easy to get concensus on. ;-)

Then we shouldn't try. Let's not let the ideal be the enemy of the good here; the multiline-literal syntax is othorgonal to how we handle string encoding, and internationalization can be layered on it later.

The question of exactly where to draw the line has bit us so many times with ConfigParser that I'm quite wary. I'll leave it for others to determine whether I'm too much so.

...

I think "suggests" is too strong a word; I was only citing precedence, not advocating that approach. I think it's pretty ugly as well.

I'm taking suggestions on how to do it right. I don't think there's any obviously better way than what I've implemented, except maybe for making the trigger a real method rather than a magic member.

I'm not suggesting that there's a better way, only a precedent.

I had this conversation with Guido a couple years back. I too would have been happier with two classes, one read-only and one that supports writing back out. Here is whatt Guido said and I replied:

[long quotation elided]

Guido dropped the thread, which I took to mean that he saw the force of the objection. But I'd *still* like to do as he suggested.

If the consensus is that its OK to deprecate add_section() and write() in the base class, I'll write ConfigEditor and move all my surgery stuff over there in a heartbeat.

I think add_section() is useful for modifying an in-memory configuration (perhaps loaded from multiple sources, other than INI files). The implementation would be different than for your proposed ConfigEditor, but that's OK. write() may also be useful for other reasons, and probably isn't worth deprecating. Maintenance of those methods is not a good reason to create a separate ConfigEditor class. -Fred -- Fred L. Drake, Jr. <fdrake at acm.org> PythonLabs at Zope Corporation

Fred L. Drake, Jr. :

...

I don't think they're fair game if we want a surgical implementation. Neither are blank lines.

Eric S. Raymond writes:

That depends on whether the surgery is at the data level or the representation level. The former covers my use case.

-Fred -- Fred L. Drake, Jr. <fdrake at acm.org> PythonLabs at Zope Corporation

Fred L. Drake, Jr. :

Eric S. Raymond writes:

...

That depends on whether the surgery is at the data level or the representation level. The former covers my use case.

At the abstract data level, the order is fair game as well.

My use case calls that assummption into some question. There are attribute groups within sections where I think the meaning is order-dependent. -- <a href="http://www.tuxedo.org/~esr/">Eric S. Raymond</a>