-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PR: Revise run_python_script_in_terminal #17635
Conversation
Quote wdir for darwin in write statement. Fix typo for PYTHONHOME environment variable. For Terminal.app, env and cwd kwargs in Popen do not have any affect.
Do not 'cd' in cmd as this is redundant with cwd keyword argument.
remove wdir-option as it is redundant with cwd. run_program does not like quoted python file, whereas run_shell_command is okay with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your help with this @mrclary! I left a small question for you, otherwise looks good to me.
@ccordoba12 @dalthviz, I neglected to mention that I successfully tested the behavior on macOS, Ubuntu (VM), and Windows (VM). Just an FYI; please do verify as well, if you can. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mrclary for helping fixing this! Testing this locally on Windows worked for me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks @mrclary!
Description of Changes
run_python_script_in_terminal
is revised to:wdir
and/or python file pathIssue(s) Resolved
Fixes #17595
Affirmation
By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.
I certify the above statement is true and correct: @mrclary