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

Add 'surgical editing' to ConfigParser #42811

Open
anadelonbrin mannequin opened this issue Jan 20, 2006 · 15 comments
Open

Add 'surgical editing' to ConfigParser #42811

anadelonbrin mannequin opened this issue Jan 20, 2006 · 15 comments
Assignees
Labels
easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@anadelonbrin
Copy link
Mannequin

anadelonbrin mannequin commented Jan 20, 2006

BPO 1410680
Nosy @pfmoore, @devdanzin, @mcepl, @merwok, @bitdancer, @ambv, @mathieui, @Fipaddict
Files
  • ConfigParser.diff: ConfigParser.py diff with 21/01/06 SVN
  • libcfgparser.diff: libcfgparser.tex diff against Jan 06 SVN
  • test_cfgparser.diff: test_cfgparser.py diff against Jan 06 SVN
  • 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 = 'https://github.com/ambv'
    closed_at = None
    created_at = <Date 2006-01-20.11:12:46.000>
    labels = ['easy', 'type-feature', 'library']
    title = "Add 'surgical editing' to ConfigParser"
    updated_at = <Date 2018-03-05.12:02:23.935>
    user = 'https://bugs.python.org/anadelonbrin'

    bugs.python.org fields:

    activity = <Date 2018-03-05.12:02:23.935>
    actor = 'mcepl'
    assignee = 'lukasz.langa'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2006-01-20.11:12:46.000>
    creator = 'anadelonbrin'
    dependencies = []
    files = ['6961', '6962', '6963']
    hgrepos = []
    issue_num = 1410680
    keywords = ['patch', 'easy']
    message_count = 14.0
    messages = ['49350', '49351', '49352', '49353', '49354', '83891', '110440', '110442', '110443', '113324', '113328', '149139', '220955', '308153']
    nosy_count = 12.0
    nosy_names = ['paul.moore', 'jimjjewett', 'anadelonbrin', 'scott.dial', 'ajaksu2', 'mcepl', 'eric.araujo', 'r.david.murray', 'thijs', 'lukasz.langa', 'mathieui', 'fipaddict']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue1410680'
    versions = ['Python 3.5']

    @anadelonbrin
    Copy link
    Mannequin Author

    anadelonbrin mannequin commented Jan 20, 2006

    See also:

    [ 1399309 ] ConfigParser to save with order
    [ 1371075 ] ConfigParser to accept a custom dict to
    allow ordering

    The attached patch adds a new method, update_file, to
    RawConfigParser. This method is similar to the
    existing write method, but preserves blank lines,
    comments, and ordering, as requested many times on
    python-dev and python-list. These are the three
    requirements that Guido specified in January 2005.

    IMO the attached patch is better than 1399309 because
    it includes all the functionality (passing a pointer to
    an empty file results in a sorted write() output), but
    is completely backwards compatible as write() is
    unchanged. The addition of preserving comments is
    also essential for many applications.

    IMO the attached patch is better than 1371075 because
    the latter really requires a custom class (e.g. the
    third party one suggested) in order to be useful. It
    also doesn't address the issue of preserving comments.

    The attached patch is essentially a tidied up version
    of the one included with SpamBayes (in the
    OptionClass.py module), which is used extensively
    within SpamBayes (and has been for several years).

    Also attached are additional unittests for the new
    method, and a documentation patch.

    Please let me know if there are recommended changes.

    This patch does not address any of the additional
    ConfigParser improvements that have been suggested at
    various times.

    @anadelonbrin anadelonbrin mannequin added stdlib Python modules in the Lib dir labels Jan 20, 2006
    @jimjjewett
    Copy link
    Mannequin

    jimjjewett mannequin commented Jan 20, 2006

    Logged In: YES
    user_id=764593

    Wanting to keep the whole thing (except defaultsection) in
    sorted order will probably be a common use case.

    It sounds like you can do this by "updating" to an empty
    file, but it isn't the obvious solution. Could you either
    add an option to sort the whole thing (so inserts may not be
    at the end) or an explicit mention in the docstring of how
    to get this?

    @scottdial
    Copy link
    Mannequin

    scottdial mannequin commented Jun 8, 2006

    Logged In: YES
    user_id=383208

    Before I lose track of the updated patches I made.. They can
    be found here:

    http://scottdial.com/python-dev/ConfigParser.diff
    http://scottdial.com/python-dev/test_cfgparser.diff

    The notes I emailed Tony were:

    • I have modified the OPTCRE matching to not just throw away
      the whitespace after a [=:]. Second, I have modified the
      opt.rstrip(), again so that the whitespace isn't just thrown
      away. A new variable padded_vi then appears which is a
      formatting-preserved version of vi.

    • I have removed at least one erroneous write('\n'), and
      changed the pattern for adding newlines to missing sections,
      such that the newlines are added /before/ the section
      heading (and only if there are lines appearing before).
      These together solve the growing blank lines phenomena.

    • I have modified the add_missing section to deal with
      DEFAULTSECT appropriately. Which solves the
      appearing/disappearing act that I had mentioned.

    • I have updated the test's to match the new expected output.

    ---

    And in reponse to Jim's comment, such a feature is not in
    the scope of the patch. The patch is to explicitly leave
    things the way they are, so any sort of sorting would make
    no sense.

    @pfmoore
    Copy link
    Member

    pfmoore commented Mar 7, 2007

    I've looked at the patches attached here. They look reasonable, and in isolation I would be happy for them to be accepted, although I have no personal use for the functionality.

    However, patch 131075 (ConfigParser to accept a custom dict to allow ordering) has since been accepted, and in the light of that, this patch may no longer be appropriate. From the descriptions, and a review of the code, I am not clear how the two are related. The justification should be updated to reflect the fact that patch 131075 has been accepted - or if there is no longer sufficient justification for what may well be a second way to do the same thing, then this patch should be withdrawn.

    I have looked at Scott Dial's updated patches, but I am not clear on what they add. There has obviously been some discussion which happened off the tracker - as a result I can't comment on the DEFAULTSECT issue. The whitespace stripping issue needs a clearer description. I don't see what is happening here. One or other of the patches is presumably mishandling leading or trailing whitespace in options, but I can't tell which. The "growing blank lines" fix sounds sensible.

    Recommendation:

    1. The OP should review the justification in the light of the acceptance of patch 1371075.
    2. Scott should clarify the issues around the 2 areas I mentioned.
    3. An updated patch should be submitted, either by the OP against this tracker, or by someone else (Scott, perhaps) under a new tracker item, pointing back to this one.

    Otherwise, I would recommend rejection on the grounds that the functionality now appears to be available via the acceptance of 1371075 (albeit in a form that this patch claims is inferior).

    Paul Moore

    @anadelonbrin
    Copy link
    Mannequin Author

    anadelonbrin mannequin commented Mar 18, 2007

    I assume that Paul meant 1371075 and not 131075 was accepted.

    1371075 didn't do what Guido wanted at the time this patch was opened (or have documentation or unit tests), but I guess opinion has changed over time.

    There is incomplete overlap between that patch and this. This patch is really about being able to modify a configuration file 'in place', without losing the ordering or (importantly) comments. 1371075 provides the first (if you write/find an appropriate ordered dict), but not the second.

    However, it seems unlikely that merely preserving comments is enough to make this change worthwhile. I have no problem with it being rejected or being subsumed into some other patch.

    @devdanzin
    Copy link
    Mannequin

    devdanzin mannequin commented Mar 20, 2009

    Anyone interested in updating this?

    @devdanzin devdanzin mannequin added type-feature A feature request or enhancement labels Mar 20, 2009
    @devdanzin devdanzin mannequin added easy labels Apr 22, 2009
    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 16, 2010

    As noone has shown any interest in this for over three years I suggest it is closed as rejected. Note also that bpo-1371075 has been accepted and the OP's comments on msg49354.

    @merwok
    Copy link
    Member

    merwok commented Jul 16, 2010

    I want to review and update the patches. I’ll probably have the time to do it in August/September.

    @merwok
    Copy link
    Member

    merwok commented Jul 16, 2010

    Thanks Brian, I’d forgotten I could assign to me. I feel bound now <wink>

    @ambv
    Copy link
    Contributor

    ambv commented Aug 8, 2010

    Éric, as this is one of the major features I'm implementing at the moment for the great configparser overhaul for Python 3.2, I hope you won't mind me taking this over :)

    @ambv ambv assigned ambv and unassigned merwok Aug 8, 2010
    @merwok
    Copy link
    Member

    merwok commented Aug 8, 2010

    Be my guest :) Thanks for your work. FYI, note that it seems more usual to ask first before unassigning someone.

    @mathieui
    Copy link
    Mannequin

    mathieui mannequin commented Dec 10, 2011

    What is the state of that feature, as of today?

    @bitdancer
    Copy link
    Member

    According to a review done at the PyCon 2014 sprints, comment and blank line preservation has not yet been implemented.

    @fipaddict
    Copy link
    Mannequin

    fipaddict mannequin commented Dec 12, 2017

    I would have liked for configparser to keep comments when my program re-write configuration file too.
    Thanks

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

    chouetz commented Sep 7, 2022

    Hello, I intend to do a proposal for this topic, and I have some questions:

    • the initial proposal was to create an update_file method. Do you consider it's better than adding an option in the write_file to preserve formatting?
    • should we consider separately the possibility to preserve comments, white lines, etc? Or a single option (either in write_file or in an update_file) is enough?
    • do we also want to preserve the number of white spaces when there is indentation?
    • do we also want to preserve the different delimiters?

    I will try to see if it remains readable to put an option in write_file, otherwise I will do a new method

    try to ping @ezio-melotti

    Thanks in advance

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    5 participants