| msg10367 - (view) |
Author: Gustavo Niemeyer (niemeyer) *  |
Date: 2002-04-17 10:24 |
| The first patch fixes a bug, implements some speed improvements, some memory consumption improvements, enforces the usage of the already available global variables, and extends the allowed chars in option names to be very permissive. The second one, if used, is supposed to be applied over the first one, and implements a walk() generator method for walking trough the options of a section. |
|
|
| msg10368 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2002-04-18 06:04 |
| Logged In: YES user_id=21627 I'd like to see this split into even more parts: a patch that supposedly has *no* semantic change (ie. the speed improvements, memory consumption improvements, use of global variables); a patch that changes behavior (please explain in which ways); and the patch that implements walk (which appears to be missing currently). |
|
|
| msg10369 - (view) |
Author: Gustavo Niemeyer (niemeyer) * |
Date: 2002-04-18 17:07 |
| Logged In: YES user_id=7887 I'd rather explain here the patch that changes behavior, since it's pretty small. This line in the regular expression OPTCRE: r'(?P |
|
|
| msg10370 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2002-04-22 17:55 |
| Logged In: YES user_id=6380 I'm assigning this to Fred Drake, who has some opinions on ConfigParser. |
|
|
| msg10371 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-05-30 16:06 |
| Logged In: YES user_id=3066 Two comments: - "section in self.__sections.keys()" can be written (still more efficiently) as "section in self.__sections", *but* the use of the sections() method is done to be subclass-friendly. I'm not sure how important this is since I can't imagine anyone actually subclassing from such a mass of messy code. Perhaps these could be modified to use the has_section() method, which is relatively new. - MAX_INTERPOLATION_DEPTH was not "honored" in the code since it's really a safety net; there's no intention that client code be able to change it. (The sanity of the value is not checked before use.) I'm inclined to retain the use of the constant in the interpolation code, and add a comment above the constant the it exists for informational purposes only. Otherwise, I'm fine with the patch. ;-) Sorry for the delay in responding to this. |
|
|
| msg10372 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-05-30 16:11 |
| Logged In: YES user_id=3066 I should clarify: By "retain the use of the constant" I mean the constant numeric literal, not MAX_INTERPOLATION_DEPTH. |
|
|
| msg10373 - (view) |
Author: Gustavo Niemeyer (niemeyer) * |
Date: 2002-06-03 21:45 |
| Logged In: YES user_id=7887 Hello Fred! - About has_section(), I thought about the possibility of subclassing it. But, besides agreeing with you on the messy code matter, other functions weren't (and still aren't) considering this possibility, so I doubt some one will be able to subclass it anyways. Anyway, I may change the patch to use whatever you suggest. - About MAX_INTERPOLATION_DEPTH, I understand your position of wanting "a safety net", but my intention of using it in the code was not to let the user change it, but to clarify and give consistence to the code. I think using literal constants in the code is not a good measure at all. If this was adopted, many modules would have to be changed (in a bad way) for consistence. With a quick look at the library, I was able to find smtpd.py, base64.py, binhex.py, cgi.py, cmd.py, and many others using similar purpose constants. You'll probably agree with me if you think about this more extensively. |
|
|
| msg10374 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-06-13 01:42 |
| Logged In: YES user_id=3066 Ken, can you recall our thinking on the use of the constant for MAX_INTERPOLATION_DEPTH when this was first written? I think you were more involved in this than the rest of us at the time. |
|
|
| msg10375 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-06-21 16:20 |
| Logged In: YES user_id=3066 Ken appears non-responsive, re-assigning to me. |
|
|
| msg10376 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-06-21 17:22 |
| Logged In: YES user_id=3066 I'd like to accept a variation on this patch. Specifically, I'd like "section in self.sections()" tests to become "sections in self.__sections" consistently; your proposed patch is better than the original, but as long as we're not supporting subclassing in this way, let's go ahead and use the better-performing solution. I'll accept the use of MAX_INTERPOLATION_DEPTH and your changes to the interpolation loop, but the loop itself will probably be re-written later, in response to SF bug #511737. The walk() method will need documentation. Can you prepare a new patch for this? Thanks! |
|
|
| msg10377 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-06-22 01:38 |
| Logged In: YES user_id=3066 Changed to a "bug" report even though it's a patch, so that all the ConfigParser reports can show up together. |
|
|
| msg10378 - (view) |
Author: Gustavo Niemeyer (niemeyer) * |
Date: 2002-06-22 02:14 |
| Logged In: YES user_id=7887 Thanks for your review Fred! I'll fix the patch as suggested, and work on a solution for the referred bug as well. |
|
|
| msg10379 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-06-22 02:16 |
| Logged In: YES user_id=3066 Thanks! I can take care of the referred bug; I already know how I want to solve that one. Thanks! |
|
|
| msg10380 - (view) |
Author: Gustavo Niemeyer (niemeyer) * |
Date: 2002-06-22 04:09 |
| Logged In: YES user_id=7887 Deal!! ;-) Here is the updated patch. Thanks! |
|
|
| msg10381 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-07-03 04:08 |
| Logged In: YES user_id=3066 Any objections to my renaming "walk" to "items" (or even "section_items")? "walk" makes me think of os.path.walk(), which uses a callback; what the function returns is a list of name-value pairs, though, which makes me think "items". Other than this, I'm ready to commit. |
|
|
| msg10382 - (view) |
Author: Gustavo Niemeyer (niemeyer) * |
Date: 2002-07-03 19:45 |
| Logged In: YES user_id=7887 No problems at all!! Thank you! |
|
|
| msg10383 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-09-27 15:50 |
| Logged In: YES user_id=3066 Added code cleanup and bugfix code in Lib/ConfigParser.py revisions 1.45 and 1.38.10.3. The new items() method was checked in as Lib/ConfigParser.py 1.46 and Doc/lib/libcfgparser.tex 1.23. I still need to write unit tests for this method, so this tracker item will remain open for now. Updated summary. |
|
|
| msg10384 - (view) |
Author: Fred Drake (fdrake) |
Date: 2002-10-25 20:44 |
| Logged In: YES user_id=3066 Added tests of .items() in Lib/test/test_cfgparser.py revision 1.17. |
|
|