Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-38741: Definition of multiple ']' in header configparser#17129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
jsnklln commented Nov 12, 2019 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
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 [].
brandtbucher commented Nov 12, 2019
Thanks @jsnklln! |
brandtbucher left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
jsnklln commented Nov 12, 2019
@brandtbucher adding some tests sounds like fun. I'm totally open to opinions on how to best handle it. |
mark99i commented Nov 13, 2019
its is a my commit! |
jsnklln commented Nov 13, 2019
@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
jsnklln commented Nov 13, 2019
@brandtbucher tests have been pushed. |
brandtbucher left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jsnklln commented Nov 13, 2019
@brandtbucher thanks. @mark99i sorry we got our wires crossed up, I'll gladly split the credit. |
jsnklln commented Dec 4, 2019
@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. |
brandtbucher commented Jul 17, 2020
Closing and reopening to trigger CI. |
brandtbucher commented Jul 17, 2020
@ambv This patch is simple and looks fine to me. Do you mind clearing up whether or not it is correct to allow |
miss-islington commented Jul 13, 2021
bedevere-bot commented Jul 13, 2021
GH-27110 is a backport of this pull request to the 3.10 branch. |
ambv commented Jul 13, 2021
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