pytest-dev / iniconfig

MIT License
53 stars 31 forks source link

Comments included in values #55

Open jefferyto opened 1 year ago

jefferyto commented 1 year ago

I'm looking into updating the OpenWrt package for iniconfig (from 1.1.1 to 2.0.0), and when I try the example from the readme with 2.0.0 (using the example ini file) I get:

Python 3.11.4 (main, Aug  3 2023, 22:34:22) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import iniconfig
>>> ini = iniconfig.IniConfig("example.ini")
>>> ini['section1']['name1']
'value1  # comment'
>>> ini.get('section1', 'name1b', [], lambda x: x.split(","))
['value1', 'value2  # comment']
>>> ini.get('section1', 'notexist', [], lambda x: x.split(","))
[]
>>> [x.name for x in list(ini)]
['section1', 'section2']
>>> list(list(ini)[0].items())
[('name1', 'value1  # comment'), ('name1b', 'value1,value2  # comment')]
>>> 'section1' in ini
True
>>> 'inexistendsection' in ini
False
>>> 

Note that # comment is included with the value of name1 and the second value of name1b. This appears to differ from the output in the readme - is the above the expected/correct output?

RonnyPfannschmidt commented 1 year ago

I won't be able to get into this today, does the behavior replicate on a desktop python?

jefferyto commented 1 year ago

I won't be able to get into this today, does the behavior replicate on a desktop python?

Yes, I get the same results in a venv on Ubuntu 23.04 (Python 3.11.4).

RonnyPfannschmidt commented 1 year ago

wow, seems like this is a allwinner, the bug got pulled in from the pylib codebase in https://github.com/pytest-dev/iniconfig/commit/3ebb68c774c6a77bae76fe0018e7e626eee96e05 in 2016 by me

now i gotta figure how that was added/missed in pylib as well this might end up being a 10 year missed bug

RonnyPfannschmidt commented 1 year ago

introduced in https://github.com/pytest-dev/py/commit/ba3873685fe55c8bf5acf683f00442ad2fb94685 it seems

RonnyPfannschmidt commented 1 year ago

as far as i can tell it seems necessary to elevate this bug to a default feature and allow a "opt out" :scream:

RonnyPfannschmidt commented 1 year ago

@jefferyto as far as i can tell now after research, the behavior you observe is indeed intended, but its introduction was a bit roundabout, a design decission is needed on how to change it now

jefferyto commented 1 year ago

Thanks for investigating - I'm not really a user of this module so I'm okay with whatever resolution is deemed appropriate.