Skip to content

Improve how editors are handled in pip config#10716

Merged
pradyunsg merged 1 commit intopypa:mainfrom
pradyunsg:better-editor-handling
Jul 29, 2022
Merged

Improve how editors are handled in pip config#10716
pradyunsg merged 1 commit intopypa:mainfrom
pradyunsg:better-editor-handling

Conversation

@pradyunsg
Copy link
Copy Markdown
Member

@pradyunsg pradyunsg commented Dec 8, 2021 •
edited
Loading

Closes #7392, I think.

@pfmoore
Copy link
Copy Markdown
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
Copy Markdown
Member Author

pradyunsg commented Dec 9, 2021

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
Copy Markdown
Member

uranusjr commented Dec 12, 2021

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
@pradyunsg pradyunsg force-pushed the better-editor-handling branch from cf0f79b to 57ae74f Compare June 17, 2022 10:45
@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 pradyunsg force-pushed the better-editor-handling branch from 57ae74f to 89983e9 Compare July 29, 2022 13:53
@pradyunsg
Copy link
Copy Markdown
Member Author

pradyunsg commented Jul 29, 2022

@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