Description

erlend-aasland

opened on Dec 20, 2023

Issue body actions

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

• gh-113317: Argument Clinic: tear out internal text accumulator APIs #113402
• gh-113317: Remove TextAccumulator type alias from clinic.py #113413
• gh-113317: Clean up Argument Clinic global namespace #113414
• gh-113317: Move more formatting helpers into libclinic #113438
• gh-113317: Rework Argument Clinic cpp.py error handling #113525
• gh-113317: Move global utility functions into libclinic #113986
• gh-113317: Move FormatCounterFormatter into libclinic #114066
• gh-113317: Pass clinic to AC parse_arg() methods #114319
• gh-113317: AC converter: Use add_include() in bad_argument() #114324
• gh-113317: Don't use global clinic instance in bad_argument() #114330
• gh-113317: Remove Argument Clinic global variable 'clinic' #114752
• gh-113317, Argument Clinic: Add BlockParser.fail() method #115369
• gh-113317, Argument Clinic: Add DSLParser.fail() method #115370
• gh-113317, Argument Clinic: Add Clinic.fail() method #115371
• gh-113317: Argument Clinic: don't use global state in warn() and fail() #115510
• gh-113317: Argument Clinic: don't use fail() in CLI #115513
• gh-113317: Argument Clinic: remove global clinic instance #115517
• gh-113317: Argument Clinic: move linear_format into libclinic #115518
• gh-113317: Argument Clinic: move C/Py identifier helpers into libclinic #115520
• gh-113317: Argument Clinic: inline required_type_for_self_for_parser() in self converter #115522
• gh-113317: Move warn() and fail() to libclinic.utils #116770
• gh-113317: Argument Clinic: Add libclinic.function #116807
• gh-113317: Add libclinic.block_parser and libclinic.language modules #116819
• gh-113317: Add libclinic.converter and libclinic.crenderdata modules #116821
• gh-113317: Move Argument Clinic converters to sub-modules #116836
• gh-113317: Change how Argument Clinic lists converters #116853
• gh-113317: Argument Clinic: Add libclinic.converters module #117315
• gh-113317: Argument Clinic: Add libclinic.return_converters #117451
• gh-113317: Argument Clinic: Add libclinic.clanguage #117455
• gh-113317: Finish splitting Argument Clinic into sub-files #117513
• gh-113317: Rename libclinic to argclinic #117533
• gh-113317: Remove unused INVALID constant in Argument Clinic #117624
• gh-113317: Add Codegen class to Argument Clinic #117626
• gh-113317: Add ParseArgsCodeGen class #117707

Reacted by Alex Waygood

Activity

1. erlend-aasland

   added

   type-featureA feature request or enhancementA feature request or enhancement

   topic-argument-clinic

   on Dec 20, 2023
2. erlend-aasland commented on Dec 20, 2023

   

   erlend-aasland

   on Dec 20, 2023

   ContributorAuthor

   More actions

   Proof-of-concept branch for version: Argument-Clinic/cpython#28
3. erlend-aasland commented on Dec 20, 2023

   

   erlend-aasland

   on Dec 20, 2023

   ContributorAuthor

   More actions

   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()).
4. erlend-aasland commented on Dec 21, 2023

   

   erlend-aasland

   on Dec 21, 2023

   ContributorAuthor

   More actions

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

   Reacted by Alex Waygood
5. 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[/+] on Dec 21, 2023
6. erlend-aasland commented on Dec 21, 2023

   

   erlend-aasland

   on Dec 21, 2023

   ContributorAuthor

   More actions

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

   Reacted by Alex Waygood
7. erlend-aasland commented on Dec 22, 2023

   

   erlend-aasland

   on Dec 22, 2023

   ContributorAuthor

   More actions

   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:

   • Clean up the accumulator APIs: gh-113317: Argument Clinic: tear out internal text accumulator APIs #113402
   • Move (remaining) accumulators into libclinic
   • Move stateless formatting helpers into libclinic
   • Move constants into libclinic

   I did some experimenting in Argument-Clinic/cpython#30.
8. erlend-aasland

   added 7 commits that reference this issue on Dec 22, 2023

   pythongh-113317: Argument Clinic: tear out internal text accumulator …

   cb85794

   gh-113317: Argument Clinic: tear out internal text accumulator APIs (#…

   daa658a

   pythongh-113317: Remove TextAccumulator type alias from clinic.py

   871ee2d

   gh-113317: Remove TextAccumulator type alias from clinic.py (#113413)

   9c3ddf3

   pythongh-113317: Clean up Argument Clinic global namespace

   6dc7246

   gh-113317: Clean up Argument Clinic global namespace (#113414)

   c3f92f6

   pythongh-113317: Move more formatting helpers into libclinic

   e03e575
9. 121 remaining items
10. diegorusso

    added a commit that references this issue on Apr 17, 2024

    pythongh-113317: Add ParseArgsCodeGen class (python#117707)

    8218ad9
11. vstinner commented on Jun 2, 2024

    

    vstinner

    on Jun 2, 2024

    Member

    More actions

    @erlend-aasland: Can we close this issue now?
12. erlend-aasland commented on Jun 2, 2024

    

    erlend-aasland

    on Jun 2, 2024

    ContributorAuthor

    More actions

    Definitely!
13. erlend-aasland

    closed this as completedon Jun 2, 2024
14. Glyphack

    added 8 commits that reference this issue on Sep 2, 2024

    pythongh-113317: Argument Clinic: tear out internal text accumulator …

    953e82d

    pythongh-113317: Remove TextAccumulator type alias from clinic.py (py…

    b9e3ddb

    pythongh-113317: Clean up Argument Clinic global namespace (python#11…

    5b0a168

    pythongh-113317: Move more formatting helpers into libclinic (python#…

    1577fac

    pythongh-113317: Rework Argument Clinic cpp.py error handling (python…

    21a0288

    pythongh-113317: Move global utility functions into libclinic (python…

    5c7494d

    pythongh-113317: Move FormatCounterFormatter into libclinic (python#1…

    2b39330

    pythongh-113317: Don't use global clinic instance in bad_argument() (p…

    603e4e5
15. LukasWoodtli

    added 3 commits that reference this issue on Jan 22, 2025

    pythongh-113317: Argument Clinic: inline required_type_for_self_for_p…

    28d512f

    pythongh-113317: Argument Clinic: move linear_format into libclinic (p…

    a373913

    pythongh-113317: Argument Clinic: move C/Py identifier helpers into l…

    dc6433e

Issue actions