Skip to content

docs: in venv table use executable name #124315

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

Merged
merged 1 commit into from
Oct 9, 2024
Merged

docs: in venv table use executable name #124315

merged 1 commit into from
Oct 9, 2024

Conversation

musvaage
Copy link
Contributor

@musvaage musvaage commented Sep 22, 2024

changes PowerShell to pwsh and pwsh.exe
changes POSIX PowerShell to pwsh


📚 Documentation preview 📚: https://cpython-previews--124315.org.readthedocs.build/

@musvaage musvaage requested a review from vsajip as a code owner September 22, 2024 16:23
@bedevere-app bedevere-app bot added awaiting review docs Documentation in the Doc dir skip news labels Sep 22, 2024
@vsajip
Copy link
Member

vsajip commented Sep 23, 2024

On my Windows computer, there's no pwsh.exe - PowerShell is powershell.exe. So I'm not sure if your change should be applied as is - perhaps have powershell.exe or pwsh.exe replace PowerShell?

@AA-Turner
Copy link
Member

On my Windows computer, there's no pwsh.exe - PowerShell is powershell.exe. So I'm not sure if your change should be applied as is - perhaps have powershell.exe or pwsh.exe replace PowerShell?

I have both -- pwsh.exe is PowerShell 7 and powershell.exe is PowerShell 5 (Windows PowerShell). This appears to be official.

A

Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

The change for POSIX seems fine but per Vinay's note the Windows entry should read either "PowerShell" or indicate that both powershell.exe and pwsh.exe are supported.

A

@bedevere-app
Copy link

bedevere-app bot commented Sep 23, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@musvaage
Copy link
Contributor Author

Windows entry

unless something like the following is workable, I'll revert to the string PowerShell, and perform a soft reset

please clarify Member preference

Doc/library/venv.rst

  .. versionadded:: 3.8
     PowerShell activation scripts installed under POSIX for PowerShell Core
-    support.
+    support (users of PowerShell 5.1 replace pwsh.exe with powershell.exe in
+    the table).

@musvaage
Copy link
Contributor Author

musvaage commented Sep 25, 2024

@AA-Turner

if neither the herein nor last comment's ideas are workable, I'll revert to the string PowerShell and perform a soft reset

in the last comment it was suggested the text in .. versionadded:: 3.8 could be supplemented

a different approach might be adding a Note: directly after the text of .. versionadded:: 3.8

     support.
  
+ .. note::
  
+     Users of PowerShell 5.1 replace pwsh.exe with powershell.exe in
+     the table.
+    
  You don't specifically *need* to activate a virtual environment,

@musvaage musvaage marked this pull request as draft September 29, 2024 21:10
@musvaage musvaage changed the title docs: in venv table use executable names docs: in venv table use executable name Sep 30, 2024
@musvaage
Copy link
Contributor Author

musvaage commented Sep 30, 2024

pwsh.exe
powershell.exe

I was disinclined to widen the table's second column.


Lib/venv/scripts/common/activate

CPython and PyPI packages' docs would be simpler if the bash/zsh script was named activate.sh.

python -m venv venv

Then it could be coded that resultant of the above command venv/bin/activate.sh was copied to venv/bin/activate.

That could remain in the code for some years until a notification was made on file copying deprecation.

regarding documentation simplification

At minimum then is a packager notice to the user to source or run the appropriate script for their preferred command-line shell.

@musvaage musvaage marked this pull request as ready for review September 30, 2024 17:42
@musvaage musvaage requested a review from AA-Turner September 30, 2024 17:43
@vsajip vsajip merged commit 7f93dbf into python:main Oct 9, 2024
25 checks passed
@vsajip vsajip added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Oct 9, 2024
@miss-islington-app
Copy link

Thanks @musvaage for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @musvaage for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 9, 2024
(cherry picked from commit 7f93dbf)

Co-authored-by: musvaage <[email protected]>
Co-authored-by: musvaage <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 9, 2024
(cherry picked from commit 7f93dbf)

Co-authored-by: musvaage <[email protected]>
Co-authored-by: musvaage <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Oct 9, 2024

GH-125171 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 9, 2024
@bedevere-app
Copy link

bedevere-app bot commented Oct 9, 2024

GH-125172 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 9, 2024
efimov-mikhail pushed a commit to efimov-mikhail/cpython that referenced this pull request Oct 9, 2024
vsajip pushed a commit that referenced this pull request Oct 9, 2024
vsajip pushed a commit that referenced this pull request Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants