Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Argument Clinic: split out global stateless helpers and constants from clinic.py #113317

Open
erlend-aasland opened this issue Dec 20, 2023 · 6 comments
Labels
topic-argument-clinic type-feature A feature request or enhancement

Comments

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Dec 20, 2023

Feature or enhancement

clinic.py has a lot of globals in various forms. Getting rid of the globals will make it possible to split out clinic.py in multiple files (#113299). Combined, this will impact readability and maintainability of Argument Clinic in a positive way.

Most globals can be easily dealt with. We can structure them into two groups:

  • global constants that are not mutated
  • stateless helper functions and classes

It can make sense split out some of these in separate files (for example a libclinic package, as suggested in #113309). For some globals, it makes more sense to embed them into one of the existing classes (for example the global version variable clearly belongs in DSLParser).

Suggesting to start with the following:

  • move stateless text accumulator helpers into a Tools/clinic/libclinic/accumulators.py file
  • move stateless text formatting helpers into a Tools/clinic/libclinic/formatters.py file
  • move version into DSLParser and version helper into a Tools/clinic/libclinic/version.py file

Linked PRs

@erlend-aasland erlend-aasland added type-feature A feature request or enhancement topic-argument-clinic labels Dec 20, 2023
@erlend-aasland
Copy link
Contributor Author

Proof-of-concept branch for version: Argument-Clinic#28

@erlend-aasland
Copy link
Contributor Author

In #113270 (comment), the need for a refactoring of the docstring generation code arose. If we split out formatting helpers and text accumulators into a libclinic, we can also split out the docstring rendering (format_docstring()).

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Dec 21, 2023

Follow-up item: when libclinic has been established, we can start partitioning the test suite (Lib/test/test_clinic.py) as well.

@erlend-aasland erlend-aasland changed the title Argument Clinic: split out stateless helpers and constants from clinic.py Argument Clinic: split out global stateless helpers and constants from clinic.py Dec 21, 2023
@erlend-aasland
Copy link
Contributor Author

FTR, the global version variable was removed with PR #113341.

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Dec 22, 2023

Moving the text accumulator APIs (TextAccumulator, _TextAccumulator, text_accumulator, and _text_accumulator) into a libclinic would imply exposing underscore prefixed methods in __all__; that feels weird. Suggested first steps:

I did some experimenting in Argument-Clinic#30.

erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 22, 2023
…APIs

Replace the internal accumulator APIs by using lists of strings and join().

Yak-shaving for separating out formatting code into a separate file.
erlend-aasland added a commit that referenced this issue Dec 22, 2023
…113402)

Replace the internal accumulator APIs by using lists of strings and join().

Yak-shaving for separating out formatting code into a separate file.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 22, 2023
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 22, 2023
Split up clinic.py by establishing libclinic as a support package for
Argument Clinic. Get rid of clinic.py globals by either making them
class members, or by putting them into libclinic.

- Move INCLUDE_COMMENT_COLUMN to BlockPrinter
- Move NO_VARARG to CLanguage
- Move formatting helpers to libclinic
- Move some constants to libclinic (and annotate them as Final)
erlend-aasland added a commit that referenced this issue Dec 23, 2023
Split up clinic.py by establishing libclinic as a support package for
Argument Clinic. Get rid of clinic.py globals by either making them
class members, or by putting them into libclinic.

- Move INCLUDE_COMMENT_COLUMN to BlockPrinter
- Move NO_VARARG to CLanguage
- Move formatting helpers to libclinic
- Move some constants to libclinic (and annotate them as Final)
erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 23, 2023
- Move strip_leading_and_trailing_blank_lines() and make it internal to libclinic
- Move normalize_snippet() to libclinic
- Move format_escape() to libclinic
- Move wrap_declarations() to libclinic
erlend-aasland added a commit that referenced this issue Dec 23, 2023
Move the following global helpers into libclinic:
- format_escape()
- normalize_snippet()
- wrap_declarations()

Also move strip_leading_and_trailing_blank_lines() and make it internal to libclinic.
ryan-duve pushed a commit to ryan-duve/cpython that referenced this issue Dec 26, 2023
…APIs (python#113402)

Replace the internal accumulator APIs by using lists of strings and join().

Yak-shaving for separating out formatting code into a separate file.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
ryan-duve pushed a commit to ryan-duve/cpython that referenced this issue Dec 26, 2023
…3414)

Split up clinic.py by establishing libclinic as a support package for
Argument Clinic. Get rid of clinic.py globals by either making them
class members, or by putting them into libclinic.

- Move INCLUDE_COMMENT_COLUMN to BlockPrinter
- Move NO_VARARG to CLanguage
- Move formatting helpers to libclinic
- Move some constants to libclinic (and annotate them as Final)
ryan-duve pushed a commit to ryan-duve/cpython that referenced this issue Dec 26, 2023
…113438)

Move the following global helpers into libclinic:
- format_escape()
- normalize_snippet()
- wrap_declarations()

Also move strip_leading_and_trailing_blank_lines() and make it internal to libclinic.
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Dec 27, 2023

We should put some thought into how we structure the rest of libclinic. Victor hashed out a proof-of-concept libclinic in #113309 #113300. We can use that PR as inspiration. We might want to do some more yak-shaving before we make such a grand transition:

  • rework the error handling so warn and fail no longer depend on global variables
  • refactor global configuration to be part of the application (converters, extensions/languages, destinations?, etc.), instead of hard-coded in the various classes (and as globals)

erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Dec 27, 2023
Rework error handling in the C preprocessor helper. Instead of monkey-
patching the cpp.Monitor.fail() method from within clinic.py, rewrite
cpp.py to use a subclass of the ClinicError exception.

Yak-shaving in preparation for putting cpp.py into libclinic.
erlend-aasland added a commit that referenced this issue Dec 27, 2023
Rework error handling in the C preprocessor helper. Instead of monkey-
patching the cpp.Monitor.fail() method from within clinic.py, rewrite
cpp.py to use a subclass of the ClinicError exception. As a side-effect,
ClinicError is moved into Tools/clinic/libclinic/errors.py.

Yak-shaving in preparation for putting cpp.py into libclinic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-argument-clinic type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

1 participant