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.

Sign up for GitHub

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

Jump to bottom

Improve how editors are handled in pip config #10716

Merged
merged 1 commit into from
Jul 29, 2022
Merged

Improve how editors are handled in pip config #10716

merged 1 commit into from
Jul 29, 2022

Conversation

pradyunsg
Copy link
Member

@pradyunsg pradyunsg commented Dec 8, 2021 •
edited
Loading

Closes #7392, I think.

@pfmoore
Copy link
Member

pfmoore commented Dec 8, 2021

Are we happy with the security risks here? Particularly given the frustrating tendency of our users to not follow the advice to not run pip as root...

At the very least, I think we should include a fairly strong warning in the docs.

@pradyunsg
Copy link
Member Author

If it's good-enough for git, it's good enough for us IMO.

https://github.com/git/git/blob/a7d14a44285d3ec4b25bf9e3b7df701221350661/editor.c#L84

The only situation this is used is pip config edit, so I think I'm fine with this.

@uranusjr
Copy link
Member

The only situation this is used is pip config edit

Currently pip also respects VISUAL and EDITOR environment variables if a pip-specific editor config is not set.

@github-actions github-actions bot added the needs rebase or merge PR has conflicts with current master label Feb 3, 2022
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Jun 17, 2022
@pradyunsg pradyunsg marked this pull request as ready for review June 17, 2022 10:46
@pradyunsg pradyunsg added type: enhancement Improvements to functionality C: configuration Configuration management and loading labels Jun 17, 2022
@pradyunsg pradyunsg requested a review from a team June 17, 2022 10:46
@pradyunsg
Use shell=True for opening the editor with pip config edit
89983e9 
This makes the behavior compatible with git and other tools that invoke
the editor in this manner.
@pradyunsg
Copy link
Member Author

@pypa/pip-committers Could I get a review on this? 😇

@pradyunsg pradyunsg merged commit 4fd08e1 into pypa:main Jul 29, 2022
@pradyunsg pradyunsg deleted the better-editor-handling branch July 29, 2022 15:56
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pfmoore pfmoore pfmoore approved these changes

Assignees
No one assigned
Labels
C: configuration Configuration management and loading type: enhancement Improvements to functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pip config and $EDITOR containing arguments
4 participants
@pradyunsg @pfmoore @uranusjr @pypa-bot