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

gh-130160: use .. program:: directive for documenting venv CLI #130699

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Mr-Sunglasses
Copy link
Contributor

@Mr-Sunglasses Mr-Sunglasses commented Feb 28, 2025

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Feb 28, 2025
@Mr-Sunglasses Mr-Sunglasses changed the title gh-130160 use .. program:: directive for documenting venv CLI gh-130160: use .. program:: directive for documenting venv CLI Feb 28, 2025
Copy link
Member

@vsajip vsajip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see what the .. program:: directive does in the documentation preview. It would be good to see screenshot(s) to show rendering of this section before/after your changes.

@Mr-Sunglasses
Copy link
Contributor Author

Mr-Sunglasses commented Feb 28, 2025

I can't see what the .. program:: directive does in the documentation preview. It would be good to see screenshot(s) to show rendering of this section before/after your changes.

@vsajip these are the changes ( with screenshot attached )

Before:

Screenshot 2025-03-01 at 12 57 13 AM

After:

ref: https://cpython-previews--130699.org.readthedocs.build/en/130699/library/venv.html#cmdoption-venv-system-site-packages

Screenshot 2025-03-01 at 12 56 48 AM

@vsajip
Copy link
Member

vsajip commented Feb 28, 2025

So we've duplicated the options information in the bit below "The command, if run with -h, will show the available options:" , in a slightly different formatting? What has that really achieved? And what did the .. program:: venv directive, specifically, render to? I'm not seeing anything in the documentation preview. Maybe I've missed it - please spell it out for me. I have to say, I'm not especially in favour of this type of "tinkering around the edges" - was the previous presentation lacking in some way? What's your motivation for making this change, @Mr-Sunglasses ?

@Mr-Sunglasses
Copy link
Contributor Author

Mr-Sunglasses commented Feb 28, 2025

So we've duplicated the options information in the bit below "The command, if run with -h, will show the available options:" , in a slightly different formatting? What has that really achieved? And what did the .. program:: venv directive, specifically, render to? I'm not seeing anything in the documentation preview. Maybe I've missed it - please spell it out for me. I have to say, I'm not especially in favour of this type of "tinkering around the edges" - was the previous presentation lacking in some way? What's your motivation for making this change, @Mr-Sunglasses ?

we've duplicated the options information in the bit below

@vsajip You're absolutely right that the options information is now duplicated in two slightly different formats:

  • First, in the output of the -h help text (which is a direct copy-paste from the command-line output).
  • Second, in the .. option:: directives that follow.

Now for the second question

What has that really achieved?

The .. option:: directives make the options stand out more clearly in the documentation. They are formatted in a way that makes them easier to scan and reference, especially for someone skimming the docs. and it also allow for better cross-referencing in the documentation. For example, if someone is reading about a specific option elsewhere in the docs, they can easily link back to its definition here. and also it maintains a consistency with how other Python modules document their CLI options (e.g., argparse), making the documentation more consistent across the board.

That said, I totally get your point about duplication. If you feel the duplication is unnecessary, we could remove the raw -h output and rely solely on the .. option:: directives.

About the .. program:: venv Directive

It tells Sphinx that the following options (.. option::) belong to the venv program. This is useful for generating more accurate and linked documentation and it allows other parts of the documentation to reference the venv program and its options more effectively. In the rendered documentation, the .. program:: venv directive itself doesn’t produce any visible output. It’s more of a behind-the-scenes marker that helps Sphinx organize and link the content. If you’re not seeing any visible change, that’s expected—it’s working as intended.

ref. https://www.sphinx-doc.org/en/master/usage/domains/standard.html#directive-program

What's your motivation for making this change, @Mr-Sunglasses ?

My motivation for making this change comes from a few observations in the issue #130160 :

  • The previous presentation of the CLI options was functional but not especially easy to scan. The raw -h output is great but can feel dense in the docs.

  • Many other Python modules (e.g., argparse, pip) use .. option:: directives to document their CLI options. Adding them to venv with this style makes the overall documentation more consistent.

  • Using .. program:: and .. option:: makes it easier to add cross-references or expand the documentation in the future without needing to restructure everything.

That said, I completely understand your perspective. If the previous presentation wasn’t broken, why fix it? My goal wasn’t to tinker for the sake of tinkering but to subtly improve the docs in a way that aligns with broader Python documentation practices.

If you’re not convinced that these changes add enough value, I’m happy to roll them back or refine them further. Please Let me know what you think!

@vsajip
Copy link
Member

vsajip commented Feb 28, 2025

You're absolutely right that the options information is now duplicated

Well, that's not good, in my view. I have no particular opinion on the .. program::venv if it doesn't render to anything. The benefit of the current setup is that if the command line options change, it's easy enough to run python -m venv -h and paste the output of that back into the documentation - easy to keep the docs in sync with the CLI. With the option directive, you have to update the docs whenever the CLI changes, but with your setup, you have to type in the option directive changes one by one and copy and paste the changed information for the option from the CLI -h run. More work for maintainers, for a slightly better presentation (bold rendering of options etc.)

Many other Python modules (e.g., argparse, pip) use .. option:: directive

Well, I didn't see any in argparse, and pip is an external program ... but sure, ensurepip uses :: option::.

But the duplication has to go, IMO. Note that if you remove the -h output to remove the duplication, you'd have to replace at least some of the usage: ... information that appears above the options in the -h output.

I see #130160 is trying to do this across various modules in the stdlib, so fair enough, I suppose ... my main concern is the extra work for maintainers (for this module, that's often me!) when the CLI changes.

@vsajip
Copy link
Member

vsajip commented Feb 28, 2025

By the way, you've said "fix" in the OP, which would close the issue if this PR is merged ... that's wrong IMO. Because the issue covers multiple modules, just changing venv wouldn't be enough to close the issue, would it?

ss23

@Mr-Sunglasses
Copy link
Contributor Author

Mr-Sunglasses commented Mar 3, 2025

By the way, you've said "fix" in the OP, which would close the issue if this PR is merged ... that's wrong IMO. Because the issue covers multiple modules, just changing venv wouldn't be enough to close the issue, would it?

ss23

Yeah, we did not want to close it until we update the targeted docs, I'll update it, thanks @vsajip

@Mr-Sunglasses
Copy link
Contributor Author

Mr-Sunglasses commented Mar 3, 2025

You're absolutely right that the options information is now duplicated

Well, that's not good, in my view. I have no particular opinion on the .. program::venv if it doesn't render to anything. The benefit of the current setup is that if the command line options change, it's easy enough to run python -m venv -h and paste the output of that back into the documentation - easy to keep the docs in sync with the CLI. With the option directive, you have to update the docs whenever the CLI changes, but with your setup, you have to type in the option directive changes one by one and copy and paste the changed information for the option from the CLI -h run. More work for maintainers, for a slightly better presentation (bold rendering of options etc.)

Many other Python modules (e.g., argparse, pip) use .. option:: directive

Well, I didn't see any in argparse, and pip is an external program ... but sure, ensurepip uses :: option::.

But the duplication has to go, IMO. Note that if you remove the -h output to remove the duplication, you'd have to replace at least some of the usage: ... information that appears above the options in the -h output.

I see #130160 is trying to do this across various modules in the stdlib, so fair enough, I suppose ... my main concern is the extra work for maintainers (for this module, that's often me!) when the CLI changes.

You're absolutely right that the options information is now duplicated

Well, that's not good, in my view. I have no particular opinion on the .. program::venv if it doesn't render to anything. The benefit of the current setup is that if the command line options change, it's easy enough to run python -m venv -h and paste the output of that back into the documentation - easy to keep the docs in sync with the CLI. With the option directive, you have to update the docs whenever the CLI changes, but with your setup, you have to type in the option directive changes one by one and copy and paste the changed information for the option from the CLI -h run. More work for maintainers, for a slightly better presentation (bold rendering of options etc.)

Many other Python modules (e.g., argparse, pip) use .. option:: directive

Well, I didn't see any in argparse, and pip is an external program ... but sure, ensurepip uses :: option::.

But the duplication has to go, IMO. Note that if you remove the -h output to remove the duplication, you'd have to replace at least some of the usage: ... information that appears above the options in the -h output.

I see #130160 is trying to do this across various modules in the stdlib, so fair enough, I suppose ... my main concern is the extra work for maintainers (for this module, that's often me!) when the CLI changes.

Thanks for your suggesstions, @vsajip .

so fair enough, I suppose ... my main concern is the extra work for maintainers (for this module, that's often me!)

Just my two cents, it will need a little bit of extra work while adding new features but it will make the docs overall better.

tagging @picnixz for his views, as he is helping me a lot in this issue and he originally created the issue. TiA.

  • If we're okay with it, I can close this PR, if it does not impact things a lot.

@picnixz
Copy link
Member

picnixz commented Mar 3, 2025

There are benefits for having the program directive and options. The reason is that it allows us to have a link to them, which otherwise is hard. Note that it's normal that the directive produces no output:

Like py:currentmodule, this directive produces no output. Instead, it serves to notify Sphinx that all following option directives document options for the program called name.

If you however think that it's not worth the change, we can skip that one. I can understand that it's easier to C/C if you already have an ArgumentParser outputting you the options. But for some modules, the option description may use backreferences.

Copy link
Contributor Author

@Mr-Sunglasses Mr-Sunglasses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tagging @vsajip for their opinion on this discussion and their opinion on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review docs Documentation in the Doc dir skip news
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants