add comment stripping from config file (issue #332)

[ghost]

Comments will now be stripped while parsing config file.

[RonnyPfannschmidt]

it may be sensible to upstream this into https://github.com/RonnyPfannschmidt/iniconfig as well (i plan to de-vendor it from pylib)

[ghost]

@RonnyPfannschmidt Hello, what exactly would you want me to do?

[obestwalter]

I don't get around reviewing and testing this, as I have a lot of other things on my plate. I just skimmed the changes and do not have a good feeling, when I see all the regexes :) - so any takers?

[hpk42]

@sifuraz could you simplify the regexes or get rid of them if you just implement "comment stripping works by removing "#" and things right of it if that "#" sign is preceded by whitespace or a line beginning. This way URLs with hashparts would still work. if i see your tests i think the suggested "comment stripping" rule would already work. And it should be part of the docs.

[vlaci]

I have some concerns regarding this proposal:

1. supporting comments inside a value specification would make tox.ini no longer an ini file.
2. supporting # as a comment indicator while still supporting it as well inside values (e.g. in URLs) is confusing.
3. could comment indicators be escaped? How would one specify a command line containing #s?

My 50 cents on the topic:

• I would only support comments in their own separate lines with optional whitespace indentations in a value specification (inside a deps definition for example). I am seeing inline comment support more confusing than helpful in this case.
• OR I'd use a more expressive configuration file format (e.g TOML)

[obestwalter]

Thank you @sifuraz for your work, but I would propose to close this as wontfix. I have no trust that we can implement this reliably without causing maintenance nightmares down the road.

If you want to avoid work on PRs that might not get merged, make sure to open an issue first if there is none yet and discuss if this would be integrated (or start a discussion on tox-dev mailing list). In this case we even had an issue about this that was already marked as wontfix: #50

I rather like the idea of @vlaci - introducing another configuration file format in the future as I feel that we have driven the extension of the tox specific ini format quite far already (just had to suffer through that at the weekend fixing #595).

[obestwalter]

P.S. created #600 to discuss that (one can dream, can't one?).

[obestwalter]

@sifuraz I feel quite horrible about this, as I actually was the one who encouraged to pick that up, having completely forgotten about the closed dupe ... sorry again!

[ghost]

@obestwalter Don't worry about it 😉