PR: Add remote terminal menu using remote client connections info #358
Conversation
Also: - Add reload toolbar action - Set script loading to blocking - Fix fitAddon call when term is undefined
dalthviz
changed the title
ccordoba12
left a comment
ccordoba12
left a comment
There was a problem hiding this comment.
Great work here, thanks @dalthviz!
Besides the few comments and suggestions I left for you below, I'd also like to mention that, according to the gif you posted, a remote console seems to be created on top of the current one. But I think that's not the right UX. Instead, we should open a new tab for remote consoles.
| from spyder.utils.programs import find_program | ||
| from spyder.widgets.tabs import Tabs | ||
| from spyder.utils.misc import select_port | ||
| from spyder.plugins.remoteclient.widgets import AuthenticationMethod |
There was a problem hiding this comment.
I think we should move this enum to spyder.plugins.remoteclient.api.protocol to prevent breaking this or other external plugins in the future.
Do you prefer to do that for 6.1.2 or leave it for 6.2?
There was a problem hiding this comment.
Making a change to that enum will break things as are currently implemented here so I guess 6.2 makes more sense
There was a problem hiding this comment.
Also the current minimum Spyder version that technically the plugin supports is 6.0.0. Due to that I ended up changing the use of ClientType for the AuthenticationMethod class here (8c79b88). Following that, maybe should the Spyder minimum version requirement be changed in this PR too and instead use here the ClientType class?
There was a problem hiding this comment.
Following that, maybe should the Spyder minimum version requirement be changed in this PR too and instead use here the ClientType class?
Yeah, I think that's a better idea. Please set spyder>=6.1.0 as the min version and drop support for Python 3.8 too because 6.1 doesn't support that Python version anymore.
Co-authored-by: Carlos Cordoba <[email protected]>
I think that's due to the way the plugin works. Also noticed that when creating a normal new terminal but haven't checked on detail why that works that way. Is that behavior something that should be changed in this PR too? |
Yep, please do that because the current UX is basically wrong: we can't replace a terminal with a new, remote one. |
…fig is valid for menu
…terminal into remote-terminal
2 participants
Add remote terminal menu from remote client connections info and also:
A preview: