bpo-38741: Definition of multiple ']' in header configparser#17129
bpo-38741: Definition of multiple ']' in header configparser#17129ambv merged 4 commits intopython:mainpython/cpython:mainfrom jsnklln:configparser_handle_brace_in_headerjsnklln/cpython:configparser_handle_brace_in_headerCopy head branch name to clipboard
Conversation
I've changed up the regex for finding section headers. I don't know if this covers every case but it covers all the cases I could come up with. The docs don't really call out if ] should be allowed in section headers but I can't think of any reason it shouldn't be. This change keeps the existing failure if you try and define a section with no name, like [].
|
Thanks @jsnklln! |
There was a problem hiding this comment.
The patch itself looks good, but I don't know enough about this module to decide if this behavior is technically correct or not. @ambv, thoughts?
Also, this probably needs a test or two. Can you include them in test_configparser.py?
|
@brandtbucher adding some tests sounds like fun. I'm totally open to opinions on how to best handle it. |
|
its is a my commit! |
|
@mark99i I didn't realize we were working on the same bug. There's not a PR on the bug tracker and I'm pretty sure I pulled from changes master yesterday but I hay not have. One note. Using .* allows [] to be considered valid where with the previous version it threw an exception. ConfigParser doesn't allow definitions without a section, I'm pretty sure that's still true, so I don't know where those would go if they were allowed. |
read correctly. Included a check for key=value because the original issue mentioned that it might not be working.
…com/jsnklln/cpython into configparser_handle_brace_in_header
|
@brandtbucher tests have been pushed. |
|
@brandtbucher thanks. @mark99i sorry we got our wires crossed up, I'll gladly split the credit. |
|
@brandtbucher is this waiting on something to be merged? Not being pushy just want to make sure I've got all my t's dotted. |
|
Closing and reopening to trigger CI. |
|
@ambv This patch is simple and looks fine to me. Do you mind clearing up whether or not it is correct to allow |
|
GH-27110 is a backport of this pull request to the 3.10 branch. |
|
Sorry for taking so long to look at this 🤦🏻♂️ |
I've changed up the regex for finding section headers.
I don't know if this covers every case but it covers
all the cases I could come up with. The docs don't really
call out if ] should be allowed in section headers but
I can't think of any reason it shouldn't be.
This change keeps the existing failure if you try and
define a section with no name, like [].
https://bugs.python.org/issue38741