Add inline_comment_prefixes to ConfigParser

[abravalheri]

According to a recent an error report in #3322, prior to version 62.3 it would seem that setuptools could accept inline comments in setup.cfg [metadata] version, for example:

[metadata]
name = yt
version = 4.1.dev0  # keep in sync with yt/_version.__version__

This was actually surprising for me because setuptools simply instantiate ConfigParser with no argument, which means inline comments are not stripped during the parsing. Moreover as shown in the diff between 62.2 and 62.3, no code path used for handling version seems to have been changed (with the exception of the update of the pyparsing dependency).

I really don't understand how the changes in v62.3 could have prevented inline comments from being stripped (v62.3 is mostly about adding warnings and seems completely unrelated), but the changes proposed in this PR seem to be the most appropriated way of handling inline comments (iif we want to...).

Summary of changes

• Add inline_comment_prefixes=(" #", "\t#") to ConfigParser instantiation.
  • Here I avoided using "#" (single character) because URL values can have fragments (which start with #).

Closes #3322

Pull Request Checklist

• Changes have tests
• News fragment added in changelog.d/.
  (See documentation for details)

[abravalheri]

Hi @jaraco, do you have an opinion about this change?

Although the file format is very much ad-hoc, it seem to be fairly natural that users would just assume that inline comments can be used in a .ini/.cfg file (ConfigParser does not do that by default, however).

[abravalheri]

Is there any chance this change is disruptive enough to justify classifying it as a "breaking change"?

Is there anyone relying on the fact that setuptools does not strip inline comments?

[abravalheri]

I plan to merge this change soon.
I am still in doubt of making a minor or a major release... I think in practice this change should not be considered a breaking change in API, but more an "alignment of expectations", so a minor release should be enough.

[jaraco]

nitpick:

Suggested change

	"""Make the ConfigParser forget everything (so we retain the original filenames
	that options come from). Private API for internal setuptools use.
	"""Make the ConfigParser forget everything (retain the original filenames
	from options). Private API for internal setuptools use.

[jaraco]

I'd like to see verification of the original claim. I'll apply the test to a commit prior to 62.3 and it should pass there, but fail after merging with 62.3.

[jaraco]

Given that pyproject.toml is preferred and since we've determined that this wasn't a regression, but just a feature request, I'd be okay to decline the change and just ask them not to use trailing comments. Your call.

[abravalheri]

Thank you very much @jaraco for the in-depth investigation (sorry for not checking it before).

I agree with you!

[jaraco]

I am still in doubt of making a minor or a major release... I think in practice this change should not be considered a breaking change in API, but more an "alignment of expectations", so a minor release should be enough.

I'd consider this a breaking change, as it will affect other fields in setup.cfg that may have had legitimate content with " #". In fact, it makes me wonder if this might be a serious breaking change. What if someone wants to legitimately include these symbols in their long-description? e.g. "description: The world's #1 package" would get interpreted as "The world's".