Getting rid of the globals

[erlend-aasland]

There are various types of globals in clinic:

• various constants (regular expressions, keyword lists, markers, etc.)
• global helper functions with state (for example, warn and fail)
• global helper functions without state (for example, the accumulators)
• clinic

We should start with the low-hanging frui: stateless helpers and plain constants. Some constants can be baked into their respective class (for example version). Others can be stuffed away in a "libclinic.py" or "util.py" file. Ditto for the helpers: some helpers clearly belong in a specific class, so we can stash them there. Other helpers are completely stateless, and can thus live in a "util.py" file.

[erlend-aasland]

Low-hanging fruit:

• version should live in the DSL parser
• INCLUDE_COMMENT_COLUMN should live in the BlockPrinter scope
• NO_VARARG should live in the CLanguage scope

[AlexWaygood]

We should maybe distinguish between different types of globals.

Global variables that are never mutated, have a constant type, and are annotated with typing.Final — these are fine, in my opinion. They're easy to reason about, easy to move into submodules, I have no issues.

Globals that are mutated from inside apparently unrelated functions, and/or have different types at various stages of the code — these are evil, and must be eviscerated! Anything that requires the global keyword is a code smell.

Personally, I only really care about category (2). Moving global variables from category (2) to category (1) can be just as good as getting rid of the global variable altogether

[erlend-aasland]

Yep, and making sure globals are not mutated (category 2 => category 1) can easily be done in parallel with taking care of other low-hanging fruit (separating out stateless helpers and "final" constants).

[erlend-aasland]

The following can easily be dealt with:

• Accumulators (TextAccumultor, text_accumulator, etc.) can be separated out to a accumulators.py file
• Stateless text formatters can be separated out to a formatters.py file:
  • quoted_for_c_string
  • c_repr
  • wrapped_c_string_literal
  • rstrip_lines
  • format_escape
  • linear_format
  • indent_all_lines
  • suffix_all_lines
  • pprint_words

The resulting files can be put in a libclinic package.

[erlend-aasland]

I did some experimenting in #30. Helpers that call fail(...) must of course wait for now, so linear_format is out of the question for now. Anyways, this looks promising. We should be able to follow up with some of the support classes (FormatCounterFormatter, CRenderData, Include, BufferSeries, FunctionKind, ...).

[erlend-aasland]

Regarding the two fails in linear_format():

https://github.com/python/cpython/blob/1ff02385944924db7e683a607da2882462594764/Tools/clinic/clinic.py#L313-L318

It seems to me only the latter can be triggered by user input[^1]; the former is an assert that the internal templates are correctly formatted. Perhaps the former can be made an assert?

[^1]: It lacks a test, but that is pretty easy to fix: add a docstring with a {parameters} placeholder, and put a non-whitespace character right in front of the placeholder: a{parameters}.

[erlend-aasland]

Regarding the global parsers and converters, one can argue that they're part of the configuration of the application, so they could end up living with the app.

[erlend-aasland]

Also: I've been trying to move the option group helpers, but they need forward declarations for a lot of stuff (Parameter, Function, etc.). We could relax their typing rules, though; they are type agnostic (we use just tuples of ints for the tests, IIRC).

[vstinner]

This issue has been fixed.