Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
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 ourterms of service andprivacy 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
I've changed up the regex for finding section headers.I don't know if this covers every case but it coversall the cases I could come up with. The docs don't reallycall out if ] should be allowed in section headers butI can't think of any reason it shouldn't be.This change keeps the existing failure if you try anddefine a section with no name, like [].
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 intest_configparser.py?
@brandtbucher adding some tests sounds like fun. I'm totally open to opinions on how to best handle it. |
mark99i commentedNov 13, 2019
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 becausethe original issue mentioned that it might not be working.
…com/jsnklln/cpython into configparser_handle_brace_in_header
@brandtbucher tests have been pushed. |
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.
@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 |
bedevere-bot commentedJul 13, 2021
GH-27110 is a backport of this pull request to the3.10 branch. |
Sorry for taking so long to look at this 🤦🏻♂️ |
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 [].
https://bugs.python.org/issue38741