-
-
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: Fix Run option in File and Project explorers #4864
PR: Fix Run option in File and Project explorers #4864
Conversation
spyder/utils/programs.py
Outdated
programs = [{'cmd': 'gnome-terminal', | ||
programs = [{'cmd': 'x-terminal-emulator', | ||
'wdir-option': '--working-directory', | ||
'execute-option': '-x'}, |
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.
It seems this change broke one of our tests. Is it really necessary to solve issue #4769?
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.
Actually no, I observed that external scripts are opened on a different terminal instead on the system default one. On my case I use Tilix, but it opens gnome-terminal, so I thought it would be better to open the custom one. However I can remove this one
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.
Yes, please open a new PR for this change.
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.
I'm not against it, it's just that this PR is to solve issue #4769 only.
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.
I put the test in if I remember correctly, but it just reflects the code as it was at the time, so feel free to change this (in another PR).
spyder/plugins/projects.py
Outdated
@@ -153,6 +154,9 @@ def register_plugin(self): | |||
self.sig_project_closed.connect( | |||
lambda v: self.editor.setup_open_files()) | |||
self.recent_project_menu.aboutToShow.connect(self.setup_menu_actions) | |||
self.run.connect(lambda fname: self.main.open_external_console( |
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.
@andfoy, let's make the file to be run in a dedicated console (instead of in an external one). I think it makes more sense (at least to me).
The problem is external consoles are immediately closed, unless you change the run options for your file.
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.
I think we don't have an option to run a file on a dedicated console: app/mainwindow.py#L2423
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.
Please use self.main.ipyconsole.create_cleint_for_file
instead. That's the way to run a file in a dedicated console.
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.
Does create_client_for_file
executes the file? It seems that it opens a new console without executing the file
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.
We should use run_script
instead
spyder/plugins/explorer.py
Outdated
@@ -77,7 +77,7 @@ def register_plugin(self): | |||
lambda fname: | |||
self.main.open_external_console(to_text_string(fname), | |||
osp.dirname(to_text_string(fname)), | |||
'', False, False, True, '', False)) |
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.
Why did you change this?
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.
See above comment :-)
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.
Do I use self.main.ipyconsole.create_cleint_for_file
also for this one?
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.
Yeah, I think so. I think it's better than running in an external console.
@ccordoba12 I think this one is ready |
Fixes #4769