PR: Add option to open an IPython console connected to the current notebook #79
Conversation
spyder_notebook/widgets/client.py
Outdated
| for session in sessions: | ||
| if session['notebook']['path'] == path: | ||
| kernel_id = session['kernel']['id'] | ||
| return (kernel_id, sessions_url) |
There was a problem hiding this comment.
Why do we need to return sessions_url here? If this function is called get_kernel_id, we should only return the kernel id, no?
There was a problem hiding this comment.
This is a method that I create initialy for a test (it was on test_plugin.py as a utility function). For the test where is used it, kernel_id and session_id are needed, should I rename it with something like get_kernel_info?
There was a problem hiding this comment.
No, please create a new function to get session_id and make this one to only return kernel_id
spyder_notebook/notebookplugin.py
Outdated
| if self.ipyconsole is not None: | ||
| kernel_name = 'kernel-' + client.get_kernel_id()[0] + '.json' | ||
| kernel_path = osp.join(jupyter_runtime_dir(), kernel_name) | ||
| self.ipyconsole._create_client_for_kernel(kernel_path, None, None, |
There was a problem hiding this comment.
It should be enough to pass kernel_id here instead of kernel_path or kernel_name. Is that not working?
There was a problem hiding this comment.
Not working, I think it is because osp.dirname is returning an empty string instead of None. Should I fix this?
|
?@dalthviz images |
|
@goanpeca
|
spyder_notebook/notebookplugin.py
Outdated
| @@ -20,6 +20,7 @@ | |||
| from qtpy.QtWidgets import QApplication, QMessageBox, QVBoxLayout, QMenu | |||
|
|
|||
| # Third-party imports | |||
| from jupyter_core.paths import jupyter_runtime_dir | |||
There was a problem hiding this comment.
This import is no longer necessary.
spyder_notebook/notebookplugin.py
Outdated
| menu_btn = create_toolbutton(self, icon=ima.icon('tooloptions'), | ||
| tip=_('Options')) | ||
|
|
||
| self.menu = QMenu(self) | ||
| menu_btn.setMenu(self.menu) | ||
| menu_btn.setPopupMode(menu_btn.InstantPopup) | ||
| add_actions(self.menu, self.menu_actions) | ||
| corner_widgets = {Qt.TopRightCorner: [new_notebook_btn, menu_btn]} | ||
| corner_widgets = {Qt.TopRightCorner: [new_notebook_btn, | ||
| open_console_btn, menu_btn]} |
There was a problem hiding this comment.
Let's remove open_console_btn from here. It takes too much space on the tabbar.
spyder_notebook/widgets/client.py
Outdated
| if session['notebook']['path'] == path: | ||
| kernel_id = session['kernel']['id'] | ||
| return kernel_id | ||
|
|
||
| def shutdown_kernel(self): | ||
| """Shutdown the kernel of the client.""" | ||
| sessions_url = self.add_token(url_path_join(self.server_url, |
There was a problem hiding this comment.
We can use self.get_session_url here.
There was a problem hiding this comment.
Sorry, we can simplify this whole function by simply calling self.get_kernel_id().
Please do it so we can remove this chunk of repeated code here.
|
A couple of things are missing before merging this PR, but they depend on spyder-ide/spyder#4691, which I hope to merge later today:
Note: You can select the client create with |
|
@dalthviz, please post a screenshot with your latest changes. |
|
Preview: |
Fixes #75
Dependent of this PR