gh-113299: Move Argument Clinic CLI into libclinic

[erlend-aasland]

• Issue: Refactor Argument Clinic support code into libclinic #113299

[zooba]

Can we not keep Tools/clinic/clinic.py so as not to break anyone who's scripted it/muscle memory?

Maybe also consider adding Tools/clinic/__main__.py so that python Tools/clinic works. May as well train people to type less, rather than more.

[erlend-aasland]

Can we not keep Tools/clinic/clinic.py so as not to break anyone who's scripted it/muscle memory?

Let's consider it. @AlexWaygood: what do you think, as the author of the patch?

Maybe also consider adding Tools/clinic/__main__.py so that python Tools/clinic works. May as well train people to type less, rather than more.

Yes, that is a good idea.

[AlexWaygood]

Can we not keep Tools/clinic/clinic.py so as not to break anyone who's scripted it/muscle memory?

Let's consider it. @AlexWaygood: what do you think, as the author of the patch?

Yeah, I agree that makes more sense on the face of it. I think I tried it, but balked because it led to a much bigger diff, essentially destroying git blame. But maybe we should bite the bullet and do it anyway — UX probably trumps "DevX" here

[AlexWaygood]

Also: thanks so much for picking this up!

[erlend-aasland]

Yeah, I agree that makes more sense on the face of it. I think I tried it, but balked because it led to a much bigger diff, essentially destroying git blame. But maybe we should bite the bullet and do it anyway — UX probably trumps "DevX" here

Ouch, yes that does mess up Git log pretty heavily! git log --follow Tools/clinic/clinic.py now yields the same history as git log --follow Tools/clinic/libclinic/clinic.py; that is unfortunate. I'm not sure Git history beats muscle memory in this case.

[erlend-aasland]

Turns out Git becomes extremely confused if we git mv Tools/clinic/run_clinic.py Tools/clinic/clinic.py. The cherry-picker will not work, since git log is confused. OTOH, as the current PR stands, cherry-picking works as expected. IMO, that is a very good argument for keeping run_clinic.py.

[vstinner]

What do you think of renaming libclinic to just clinic? And also move __main__.py in the sub-directory?

My intent is to be able to run python -m clinic ... when making the assumption that Tools/clinic/ is in the path.

Previously, "clinic" couldn't be used since Tools/clinic/ already contained clinic.py.

I hope that in the long term, we can provide clinic as part of the stdlib! And clinic sounds like a better name than libclinic.

By the way, while we move files all around, what do you think of renaming clinic to argclinic?

[erlend-aasland]

What do you think of renaming libclinic to just clinic? And also move __main__.py in the sub-directory?

Something like this?

$ ls -lR Tools/clinic
total 24
drwxr-xr-x  10 erlend.aasland  staff  320 Feb 16 09:11 clinic
-rw-r--r--   1 erlend.aasland  staff  266 Jan 17 12:58 mypy.ini

Tools/clinic/clinic:
total 536
-rw-r--r--  1 erlend.aasland  staff    1413 Feb 16 09:07 __init__.py
-rw-r--r--  1 erlend.aasland  staff      70 Feb 16 09:11 __main__.py
-rw-r--r--  1 erlend.aasland  staff    6361 Feb 16 09:11 cli.py
-rw-r--r--  1 erlend.aasland  staff  229513 Feb 16 09:11 clinic.py
-rw-r--r--  1 erlend.aasland  staff    5918 Feb 15 23:59 cpp.py
-rw-r--r--  1 erlend.aasland  staff     638 Feb 15 23:59 errors.py
-rw-r--r--  1 erlend.aasland  staff    6671 Feb 16 09:07 formatting.py
-rw-r--r--  1 erlend.aasland  staff    1049 Feb 16 09:07 identifiers.py
-rw-r--r--  1 erlend.aasland  staff    1951 Feb 15 23:59 utils.py

My intent is to be able to run python -m clinic ... when making the assumption that Tools/clinic/ is in the path.

python -m clinic would have been nice, indeed.

I hope that in the long term, we can provide clinic as part of the stdlib! And clinic sounds like a better name than libclinic.

Currently, libclinic makes sense (we have clinic.py and a clinic lib with some refactored out code), but I agree it may be a temporary namespace. Here's a quick glance at what we do elsewhere in Tools/:

$ find Tools -name __main__.py
Tools/c-analyzer/c_parser/preprocessor/__main__.py
Tools/c-analyzer/c_parser/__main__.py
Tools/c-analyzer/cpython/__main__.py
Tools/c-analyzer/c_analyzer/__main__.py
Tools/peg_generator/pegen/__main__.py

Your suggestion aligns with existing practice.

By the way, while we move files all around, what do you think of renaming clinic to argclinic?

Let the painting begin 🎨🚲😄 My preference would be:

1. clinic (no change)
2. argument-clinic
3. argument_clinic
4. argclinic

[erlend-aasland]

@vstinner: but we are digressing; the linked issue is about refactoring Argument Clinic into smaller and more manageable pieces, and this PR is about separating out the command-line interface. If we want a new Argument Clinic namespace, or make Argument Clinic a part of the stdlib, we should have a broader discussion 😃 Let us not derail further from the PR.

[vstinner]

LGTM if you remove __main__.py.

[zooba]

You could use two commits/PRs to handle the Git history.

IIUC, Git doesn't track renames, it just calculates them when generating its history. That ought to happen for each commit, so if there's a commit that is clearly a move rather than a big edit, followed by another one that's clearly a move rather than a big edit, it ought to stay happy.

You can test with two commits on your own, but because we squash merge, it'll need to be separate PRs. The second one should just be a rename.

[erlend-aasland]

@zooba, I see I did not clearly explain what I tested in #115542 (comment):

1. squash this PR to main (simulate merge)
2. git mv Tools/clinic/run_clinic.py Tools/clinic/clinic.py
3. git log Tools/clinic/clinic.py is (still) confused
4. create a dummy feature branch off of the test main branch; hack on Tools/clinic/libclinic/clinic.py
5. cherry-pick the commit from 4. onto 3.12; this fails

If we don't rename run_clinic.py to clinic.py (that is, skip 2.), 5. works, because Git is able to follow the Tools/clinic/libclinic/clinic.py history back to Tools/clinic/clinic.py.

[zooba]

Yeah, I guess Git is going to make this painful for everyone regardless of the choice.

Personally, calling it clinic-run.py rather than run_clinic.py would suit me (it's not meant to be imported so the hyphen is fine, and I usually tab complete the last bit). But whatever has to be done has to be done.

[AlexWaygood]

clinic-run.py is fine by me!

[erlend-aasland]

Yeah, I guess Git is going to make this painful for everyone regardless of the choice.

Well, there is the option just keeping the CLI in Tools/clinic/clinic.py, and putting everything else into libclinic.

[erlend-aasland]

LGTM if you remove __main__.py.

Removed with 730c3f9

[vstinner]

git mv Tools/clinic/run_clinic.py Tools/clinic/clinic.py

Please don't do that :-( If we have a clinic/ package (directory), put a __main__.py inside, and run it with python -m clinic in the right directory, or a script with any name except of clinic.py. If we have clinic.py and clinic/, it sounds very confusing :-(

Also, run_clinic.py can live in a different directory, like Tools/build/.

[vstinner]

LGTM, but be careful of outdated NEWS entry (and maybe the commit message? I didn't check.)

[vstinner]

The NEWS entry is outdated, there is no __main__.py script.

[erlend-aasland]

Ohh, thanks; the NEWS entry was not part of the commit I reverted. Good catch.

[erlend-aasland]

Personally, calling it clinic-run.py rather than run_clinic.py would suit me (it's not meant to be imported so the hyphen is fine, and I usually tab complete the last bit). But whatever has to be done has to be done.

@zooba: How about @vstinner's suggestion (a clinic package)? With Tools/clinic/clinic/__main__.py you could invoke Argument Clinic pretty much like you do today, just without the .py at the end.

$ python3 Tools/clinic/clinic

[erlend-aasland]

Suggested change

	The Argument Clinic CLI is now invoked as either :program:`python3
	Tools/clinic` or :program:`python3 Tools/clinic/run_clinic.py`, as
	:file:`Tools/clinic/clinic.py` has been refactored into ``libclinic``.
	The Argument Clinic CLI is now invoked as
	:program:`python3 Tools/clinic/run_clinic.py`,
	since :file:`Tools/clinic/clinic.py` has been refactored into ``libclinic``.

[vstinner]

Yep, I like this NEWS entry.

[zooba]

I'd rather make Tools/clinic executable than Tools/clinic/clinic, but that's because I'd rather have Tools on PYTHONPATH than Tools/clinic.

However, since I don't do either right now, and because clinic is the only one I ever run, I don't have a strong preference. Just don't commit us to something that is going to be difficult to change in the future or massively interferes with muscle-memory without being a usability/discoverability improvement.

[vstinner]

I'd rather make Tools/clinic executable than Tools/clinic/clinic

I would like to move clinic to the stdlib in the long term, so I would prefer to have Tools/clinic/clinic/__maini__.py. Since it sounds controversial, I proposed to not add __main__.py for now.

[erlend-aasland]

Superseded by #117513.