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.

Sign up for GitHub

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

Jump to bottom

ConfigParser: accept leading whitespace on options+comments #43689

Closed
kenlalonde mannequin opened this issue Jul 18, 2006 · 10 comments
Closed

ConfigParser: accept leading whitespace on options+comments #43689

kenlalonde mannequin opened this issue Jul 18, 2006 · 10 comments
Labels
easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@kenlalonde
Copy link
Mannequin

kenlalonde mannequin commented Jul 18, 2006

BPO 1524825
Nosy @akuchling, @terryjreedy, @tiran, @voidspace, @ambv
Superseder
  • bpo-1682942: ConfigParser support for alt delimiters
    		•
    Files
  • patch.txt: Patch to ConfigParser.py, current SVN
  • cfgparser_comments.patch: Patch against trunk (r61890)
    		•

    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 = <Date 2010-07-26.18:13:24.550>
created_at = <Date 2006-07-18.21:17:05.000>
labels = ['easy', 'type-feature', 'library']
title = 'ConfigParser: accept leading whitespace on options+comments'
updated_at = <Date 2010-07-26.20:17:23.818>
user = 'https://bugs.python.org/kenlalonde'

    bugs.python.org fields:

    activity = <Date 2010-07-26.20:17:23.818>
actor = 'terry.reedy'
assignee = 'none'
closed = True
closed_date = <Date 2010-07-26.18:13:24.550>
closer = 'michael.foord'
components = ['Library (Lib)']
creation = <Date 2006-07-18.21:17:05.000>
creator = 'kenlalonde'
dependencies = []
files = ['7413', '9848']
hgrepos = []
issue_num = 1524825
keywords = ['patch', 'easy']
message_count = 10.0
messages = ['50719', '50720', '62856', '62993', '63047', '64482', '66523', '66583', '109818', '111659']
nosy_count = 13.0
nosy_names = ['akuchling', 'terry.reedy', 'kenlalonde', 'whit537', 'draghuram', 'christian.heimes', 'schmir', 'quentin.gallet-gilles', 'msuchy', 'jonatasoliveira', 'jerith', 'michael.foord', 'lukasz.langa']
pr_nums = []
priority = 'normal'
resolution = 'duplicate'
stage = 'resolved'
status = 'closed'
superseder = '1682942'
type = 'enhancement'
url = 'https://bugs.python.org/issue1524825'
versions = ['Python 3.2']
    @kenlalonde
    Copy link
    Mannequin Author

    kenlalonde mannequin commented Jul 18, 2006

    ConfigParser considers leading white space before
    options and comments to be syntax errors, which is a
    bit harsh, and limits the utility of the module when
    parsing files originally written for other programs,
    such as Samba's smb.conf.

    The attached patch ignores leading white space on
    options, and skips comments with leading w.s.

    @kenlalonde kenlalonde mannequin added stdlib Python modules in the Lib dir labels Jul 18, 2006
    @whit537
    Copy link
    Mannequin

    whit537 mannequin commented Jul 31, 2006

    Logged In: YES
    user_id=340931

    Thanks for the patch Ken! This change sounds reasonable and
    low-risk. However, some patch suggestions:

    1. Diff against HEAD of trunk/ from svn, not your platform's
      python installation

    2. Add some tests in Lib/test/test_cfgparser.py (this will
      be in a source distribution from svn, but not in your
      platform's binary distribution)

    3. Update the doc in Doc/lib/libcfgparser.tex (again, this
      will be in a source dist)

    FWIW, these are mentioned in the (admittedly somewhat
    obtuse) patch guidelines:

    http://www.python.org/dev/patches/

    @birkenfeld birkenfeld added type-feature A feature request or enhancement labels Feb 23, 2008
    @quentingallet-gilles
    Copy link
    Mannequin

    quentingallet-gilles mannequin commented Feb 24, 2008

    I tried to come up with a patch, but the issue isn't as easy as it
    seems, the proposed change is too simplistic. Leading spaces in a line
    is already used for one purpose : continuations of a previous line. For
    instance :
    option = value
    continued here
    gives "option" as key and "value\n continued here" as value.

    The proposal conflicts with this behavior, since having :
    option1 = value1
    option2 = value2
    creates only one key-value pair "option1":"value1\n option2=value2"

    Adding blank lines doesn't solve the issue. The only case when it's
    treated correctly is when the options is the first one of a
    section/file: there's no continuation since there's no previous option
    defined.

    One solution could be to check for the presence of a delimiter (colon or
    equals sign) and not treat it as a continuation if that's the case, but
    that could potentially break existing configurations files, since
    nothing forbids you from using delimiters in the values.

    Any opinion ?

    (By the way, the leading whitespaces for comments isn't affected by all
    this, the implementation is simple)

    @kenlalonde
    Copy link
    Mannequin Author

    kenlalonde mannequin commented Feb 25, 2008

    Quentin: I didn't appreciate the line-continuation issue.
    Breaking existing code is out of the question, so I'd like to
    retract that part of the patch.

    @quentingallet-gilles
    Copy link
    Mannequin

    quentingallet-gilles mannequin commented Feb 26, 2008

    Okay, I'll upload a patch with unit tests and doc changes for the
    comment part in the next few days.

    @quentingallet-gilles
    Copy link
    Mannequin

    quentingallet-gilles mannequin commented Mar 25, 2008

    Didn't think "a few days" would translate into a month. My bad!

    Anyway, here's the promised patch.

    @jerith
    Copy link
    Mannequin

    jerith mannequin commented May 10, 2008

    This looks very much like a duplicate of bpo-1714. Perhaps the two
    should be merged?

    @jonatasoliveira
    Copy link
    Mannequin

    jonatasoliveira mannequin commented May 10, 2008

    The patch cfgparser_comments.patch works for me.

    I agree with Jeremy Thurgood about the bpo-1714.

    @terryjreedy
    Copy link
    Member

    I closed bpo-1714 as a duplicate of this. It also has a patch attached.

    @ambv
    Copy link
    Contributor

    ambv commented Jul 26, 2010

    Implemented as part of bpo-1682942 since it touches the same code. Moreover, this issue mentions Samba config parsing in the original comment (msg50719) which was not doable without the changes introduced by bpo-1682942.

    So I would supersede this issue with bpo-1682942. One way or the other, they both fly or both go.

    Michael, can you close this as superseded?

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @ambv @voidspace @birkenfeld @facundobatista @terryjreedy