Support Editor env with options in config edit#7391
Support Editor env with options in config edit#7391haooodev wants to merge 1 commit intopypa:masterfrom
Conversation
Generally Eidtor env is a single string referring to a program but nothing prevents user from attaching options within it. To fully conform to the convention `pip config edit` should split the env first.
|
Could you provide reference whether this is the right thing to do? I briefly searched, but all references I found to the |
There was a problem hiding this comment.
This could be a reasonable approach that addresses the point raised by @uranusjr:
def get_editor_cmd():
editor = os.environ["EDITOR"]
result = shutil.which(editor)
if result:
return [result]
if os.path.exists(editor):
return [editor]
return shlex.split(editor)Then if we want to deprecate non-shell-syntax in the future we could warn if os.path.exists(editor) and ([editor] != shlex.split(editor)).
We would also need tests for this.
(discussion on whether to do this is on #7392).
|
Thanks for experimenting with this @chrahunt! So, we should do this. I think we should pass |
|
Be careful to test I'm not saying don't use Actually, there may be other issues too - has anyone considered how switching to |
This would be backwards-incompatible and may be less obvious for users who just want to set their editor path directly. I updated my previous comment with an example implementation in case that makes it clearer how much code we're talking about.
I don't think using |
|
@chrahunt’s suggestion looks good to me, except |
|
It has been some time without movement on this, so I will close it. I've left a comment on #7392 with details for anyone that wants to pick this back up. |
Generally the
EDITORenv is a single string referring to a program butnothing prevents user from attaching options within it. To fully conform
to the convention
pip config editshould split the env first.Fixes #7392.