Skip to content
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

ConfigParser: stripping of comments should be documented #86129

Closed
jugmac00 mannequin opened this issue Oct 7, 2020 · 7 comments
Closed

ConfigParser: stripping of comments should be documented #86129

jugmac00 mannequin opened this issue Oct 7, 2020 · 7 comments

Comments

@jugmac00
Copy link
Mannequin

jugmac00 mannequin commented Oct 7, 2020

BPO 41963
Nosy @ambv, @jugmac00, @miss-islington
PRs
  • bpo-41963: document that ConfigParser strips off comments #26197
  • [3.9] bpo-41963: document that ConfigParser strips off comments (GH-26197) #26213
  • [3.10] bpo-41963: document that ConfigParser strips off comments (GH-26197) #26214
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2020-10-07.07:21:09.885>
    labels = []
    title = 'ConfigParser: stripping of comments should be documented'
    updated_at = <Date 2021-05-18.17:03:17.615>
    user = 'https://github.com/jugmac00'

    bugs.python.org fields:

    activity = <Date 2021-05-18.17:03:17.615>
    actor = 'lukasz.langa'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = []
    creation = <Date 2020-10-07.07:21:09.885>
    creator = 'jugmac00'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 41963
    keywords = ['patch']
    message_count = 6.0
    messages = ['378146', '378147', '378308', '378313', '393885', '393888']
    nosy_count = 3.0
    nosy_names = ['lukasz.langa', 'jugmac00', 'miss-islington']
    pr_nums = ['26197', '26213', '26214']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue41963'
    versions = []

    @jugmac00
    Copy link
    Mannequin Author

    jugmac00 mannequin commented Oct 7, 2020

    While working on tox-ini-fmt, a formatter for - you guessed it - tox.ini files, I noticed ConfigParser strips comments when reading a config file.

    ( tox-dev/tox-ini-fmt#34 )

    While reasonable, this behaviour is surprising, as it is neither documented at https://docs.python.org/3/library/configparser.html nor in the docstrings (read and _read) which I read at first.

    The stripping of comments is only documented with inline comments

    https://github.com/jugmac00/cpython/blob/610a60c601fb4380eee30e15be1cd4dcbdaeec4c/Lib/configparser.py#L1019

    and

    https://github.com/jugmac00/cpython/blob/610a60c601fb4380eee30e15be1cd4dcbdaeec4c/Lib/configparser.py#L1031

    Once I found these comments, I was surprised once again, as in my code the inline comments were not stripped. After some more pdb-ing and reading the source of ConfigParser, I noticed that - while comments have a default value, inline comments do not - and that is why when you read a config file, some comments get removed and others not.

    I'd like to work on a pull request to document this behaviour, both in the official documentation and in the docstrings of the read and the _read methods.

    @jugmac00
    Copy link
    Mannequin Author

    jugmac00 mannequin commented Oct 7, 2020

    This "while comments have a default value, inline comments do not" should read "while comment prefixes have a default value, inline comment prefixes do not"

    @jugmac00
    Copy link
    Mannequin Author

    jugmac00 mannequin commented Oct 9, 2020

    The Python wiki mentions that "writing to an INI file will wipe out all comments".

    https://wiki.python.org/moin/ConfigParserExamples

    So, when updating the docstrings, it may be a good idea to both add a note at the read methods, that comments get ignored and to the write method(s) that on writing, the possible previous existing comments get removed.

    Are there any thoughts on this or should I go forward and create a pull request?

    @jugmac00
    Copy link
    Mannequin Author

    jugmac00 mannequin commented Oct 9, 2020

    pymotw.com shows a big red warning about comments not being preserved:

    https://pymotw.com/3/configparser/

    @ambv
    Copy link
    Contributor

    ambv commented May 18, 2021

    New changeset c17ba23 by Miss Islington (bot) in branch '3.9':
    bpo-41963: document that ConfigParser strips off comments (GH-26197) (GH-26213)
    c17ba23

    @ambv
    Copy link
    Contributor

    ambv commented May 18, 2021

    New changeset 4d17c93 by Łukasz Langa in branch '3.10':
    [3.10] bpo-41963: document that ConfigParser strips off comments (GH-26197) (GH-26214)
    4d17c93

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @arhadthedev
    Copy link
    Member

    Closing as fixed in gh-26197.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants