Section comments are bound to the preceding section rather than the following

[intelfx]

Description of your problem

Please provide a minimal, self-contained, and reproducible example.

from configupdater import ConfigUpdater

INPUT = '''\
[section1]
sec1key = SEC1VAL
'''

OUTPUT = '''\
[section1]
sec1key = SEC1VAL

[section2]
sec2key = SEC2VAL

# section3 comment
[section3]
sec3key = SEC3VAL
'''

def test_configupdater():
    conf = ConfigUpdater()
    conf.read_string(INPUT)

    (conf["section1"]
     .add_after
     .comment("section3 comment")
     .section("section3"))
    conf["section3"]["sec3key"] = "SEC3VAL"

    (conf["section3"]
     .add_before
     .section("section2")
     .space(2))
    conf["section2"]["sec2key"] = "SEC2VAL"

    assert str(conf) == OUTPUT

Please provide the full traceback.

$ pytest -rA -vv tests/test_configupdater.py
=================================== test session starts ===================================
platform linux -- Python 3.11.3, pytest-7.4.0, pluggy-1.1.0 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /home/intelfx/devel/proj/build.py
configfile: pyproject.toml
plugins: anyio-3.7.1
collected 1 item

tests/test_configupdater.py::test_configupdater FAILED                              [100%]

======================================== FAILURES =========================================
___________________________________ test_configupdater ____________________________________

    def test_configupdater():
        conf = ConfigUpdater()
        conf.read_string(INPUT)

        (conf["section1"]
         .add_after
         .comment("section3 comment")
         .section("section3"))
        conf["section3"]["sec3key"] = "SEC3VAL"

        (conf["section3"]
         .add_before
         .section("section2")
         .space(2))
        conf["section2"]["sec2key"] = "SEC2VAL"

>       assert str(conf) == OUTPUT
E       AssertionError: assert '[section1]\nsec1key = SEC1VAL\n# section3 comment\n[section2]\nsec2key = SEC2VAL\n\n\n[section3]\nsec3key = SEC3VAL\n' == '[section1]\nsec1key = SEC1VAL\n\n[section2]\nsec2key = SEC2VAL\n\n# section3 comment\n[section3]\nsec3key = SEC3VAL\n'
E           [section1]
E           sec1key = SEC1VAL
E         -
E         + # section3 comment
E           [section2]
E           sec2key = SEC2VAL
E
E         - # section3 comment
E         +
E           [section3]
E           sec3key = SEC3VAL

tests/test_configupdater.py:38: AssertionError
================================= short test summary info =================================
FAILED tests/test_configupdater.py::test_configupdater - AssertionError: assert '[section1]\nsec1key = SEC1VAL\n# section3 comment\n[section2]\...
==================================== 1 failed in 0.06s ====================================

Please provide any additional information below.

This report might be seen as an RFE rather than a bug, since the expected behavior was never guaranteed in the documentation. However, this example implies that ConfigUpdater's authors recognize a comment immediately preceding a section as a comment pertaining to that section. As such, the principle of least surprise is probably being broken here.

Versions and main components

• ConfigUpdater Version: python-configupdater 3.1.1
• Python Version: Python 3.11.3
• Operating system: Arch
• How did you install ConfigUpdater: distribution package

[FlorianWilhelm]

Hi @intelfx, thanks for taking the time to post this issue.

I think I get your point and if you see the comment above a section as one entity then you are right in finding this behaviour weird. Also, as you pointed out, the one example we give hints to that notion. In practice, some people write comments related to the section itself above, others below.

Technically, Configupdater builds a pretty simple hierarchy. On the top-level, we have comments, vertical space (new lines) and sections. Within a section we have comments, vertical spaces and options. So there is currently only the notion of things within a section and outside. A comment above a section will always be considered outside. Thus when you add another section above the current one, you are inserting it between the comment and the current section, what explains the behaviour you see above.

Having somehow containers that relate a comment and a section on the same level, would require a complete redesign of the concepts/architecture we are using right now. Also, what about people that don't expect the described behaviour? In your code, it does exactly what was instructed, i.e. a new section is inserted directly above the section (and thus moving the comment up).

Thinking about a solution for your use-case, what would be the expected behaviour of this after creation?

  (conf["section1"]
   .add_after
   .comment("Larger section 3 comment start")
   .space(1)
   .comment("Larger section 3 comment middle")
   .space(1)
   .comment("Larger section 3 comment end")
   .section("section3"))
  conf["section3"]["sec3key"] = "SEC3VAL"

Would you also consider this one comment that is related to the section and thus should stick to it? If not and we agree that a comment related to a section are N comment lines above it without any vertical spaces, then we could implement a feature with a new method to make what you want explicit, e.g.

  (conf["section3"]
   .add_before_skip_comments
   .section("section2")
   .space(2))
  conf["section2"]["sec2key"] = "SEC2VAL"

So with add_before_skip_comments, you would say that you want to add before a section something, skipping potential section comments. Would that work for you? And would you also be interested in providing a PR? This should not be too hard, as you basically just decrease the index pointing to the section as long as the blocks before it are comments.

Full disclosure: My focus shifted to some other OSS projects, thus everything other than real bugs won't be fixed by myself. I would give feedback to some PR though and would help to integrate it as far as I can.

[abravalheri]

I agree with @FlorianWilhelm, handling this feature request would require a complete rewrite, because the "AST" design cannot cover it.

For now I think the best we can do is to rewrite the example in the docs that might be misleading. If anyone is interested in contributing the proposed add_before_skip_comments, that would also be feasible.