Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

tweaked close dialog and added prompts to prevent silent remote close

  • Loading branch information...
commit c0bdb216972e5c4ef593a64d09e109876e795870 1 parent 925c98c
@minrk authored
View
38 IPython/frontend/qt/console/frontend_widget.py
@@ -101,6 +101,7 @@ class FrontendWidget(HistoryConsoleWidget, BaseFrontendMixin):
_CompletionRequest = namedtuple('_CompletionRequest', ['id', 'pos'])
_ExecutionRequest = namedtuple('_ExecutionRequest', ['id', 'kind'])
_input_splitter_class = InputSplitter
+ _local_kernel = False
#---------------------------------------------------------------------------
# 'object' interface
@@ -141,6 +142,9 @@ def __init__(self, *args, **kw):
# Connect signal handlers.
document = self._control.document()
document.contentsChange.connect(self._document_contents_change)
+
+ # set flag for whether we are connected via localhost
+ self._local_kernel = kw.get('local_kernel', False)
@fperez
fperez added a note

Perhaps instead of explicit False here, we could use

FrontendWidget._local_kernel

that way if the default ever changes, we don't have a mismatch because we forget to update the value in the init. It's less pretty, but it's the price we pay for our nicely organized class-level attributes.

@minrk Owner
minrk added a note

Perhaps cleaner is:
if 'local_kernel' in kw:
self._local_kernel = kw['local_kernel']

That way we don't even set the variable if it's not in the args.

@fperez
fperez added a note

I've been thinking about this... When a variable isn't found in the instance dict, it triggers a second fallback lookup in the class dict, which is slower. So I think in general, we do want to apply to the instance all arguments specified declaratively in the class, to ensure that normal attribute lookups go in general through the instance and don't have to fall back. Thoughts?

@minrk Owner
minrk added a note

Ah, I'm not nearly as familiar with the inner workings of objects. I was just thinking in terms of cleanliness of code.

I don't generally think of the class attributes as providing the defaults, so much as just facilitating introspection (>>> MyClass?) prior to instantiating an object, and a cleaner view of attribute names when writing code without digging through init logic.

Does self.attr go through the class every time, or just the first time it's requested?

@fperez
fperez added a note

That's the problem: if it's not set in the instance explicitly, it has to do a double lookup every single time.

Which is why I think with this pattern of class-based declarations (which I very much like) we should also follow up with setting things in the instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
#---------------------------------------------------------------------------
# 'ConsoleWidget' public interface
@@ -366,14 +370,32 @@ def _handle_shutdown_reply(self, msg):
""" Handle shutdown signal, only if from other console.
"""
if not self._hidden and not self._is_from_this_session(msg):
- if not msg['content']['restart']:
- sys.exit(0)
- else:
- # we just got notified of a restart!
- time.sleep(0.25) # wait 1/4 sec to reset
- # lest the request for a new prompt
- # goes to the old kernel
- self.reset()
+ if self._local_kernel:
+ if not msg['content']['restart']:
+ sys.exit(0)
+ else:
+ # we just got notified of a restart!
+ time.sleep(0.25) # wait 1/4 sec to reset
+ # lest the request for a new prompt
+ # goes to the old kernel
+ self.reset()
+ else: # remote kernel, prompt on Kernel shutdown/reset
+ title = self.window().windowTitle()
+ if not msg['content']['restart']:
+ reply = QtGui.QMessageBox.question(self, title,
+ "Kernel has been shutdown permanently. Close the Console?",
+ QtGui.QMessageBox.Yes,QtGui.QMessageBox.No)
+ if reply == QtGui.QMessageBox.Yes:
+ sys.exit(0)
+ else:
+ reply = QtGui.QMessageBox.question(self, title,
+ "Kernel has been reset. Clear the Console?",
+ QtGui.QMessageBox.Yes,QtGui.QMessageBox.No)
+ if reply == QtGui.QMessageBox.Yes:
+ time.sleep(0.25) # wait 1/4 sec to reset
+ # lest the request for a new prompt
+ # goes to the old kernel
+ self.reset()
def _started_channels(self):
""" Called when the KernelManager channels have started listening or
View
14 IPython/frontend/qt/console/ipythonqt.py
@@ -57,9 +57,9 @@ def closeEvent(self, event):
if kernel_manager and kernel_manager.channels_running:
title = self.window().windowTitle()
reply = QtGui.QMessageBox.question(self, title,
- "Close just this console, or shutdown the kernel and close "+
- "all windows attached to it?",
- 'Cancel', 'Close Console', 'Close All')
+ "You are closing this Console window."+
+ "\nWould you like to quit the Kernel and all attached Consoles as well?",
+ 'Cancel', 'No, just this Console', 'Yes, quit everything')
if reply == 2: # close All
kernel_manager.shutdown_kernel()
#kernel_manager.stop_channels()
@@ -68,6 +68,7 @@ def closeEvent(self, event):
if not self._existing:
# I have the kernel: don't quit, just close the window
self._app.setQuitOnLastWindowClosed(False)
+ self.deleteLater()
event.accept()
else:
event.ignore()
@@ -133,15 +134,16 @@ def main():
kernel_manager.start_kernel()
kernel_manager.start_channels()
+ local_kernel = (args.ip == LOCALHOST)
# Create the widget.
app = QtGui.QApplication([])
if args.pure:
kind = 'rich' if args.rich else 'plain'
- widget = FrontendWidget(kind=kind, paging=args.paging)
+ widget = FrontendWidget(kind=kind, paging=args.paging, local_kernel=local_kernel)
elif args.rich or args.pylab:
- widget = RichIPythonWidget(paging=args.paging)
+ widget = RichIPythonWidget(paging=args.paging, local_kernel=local_kernel)
else:
- widget = IPythonWidget(paging=args.paging)
+ widget = IPythonWidget(paging=args.paging, local_kernel=local_kernel)
widget.gui_completion = args.gui_completion
widget.kernel_manager = kernel_manager
@fperez

Perhaps instead of explicit False here, we could use

FrontendWidget._local_kernel

that way if the default ever changes, we don't have a mismatch because we forget to update the value in the init. It's less pretty, but it's the price we pay for our nicely organized class-level attributes.

@minrk

Perhaps cleaner is:
if 'local_kernel' in kw:
self._local_kernel = kw['local_kernel']

That way we don't even set the variable if it's not in the args.

@fperez

I've been thinking about this... When a variable isn't found in the instance dict, it triggers a second fallback lookup in the class dict, which is slower. So I think in general, we do want to apply to the instance all arguments specified declaratively in the class, to ensure that normal attribute lookups go in general through the instance and don't have to fall back. Thoughts?

@minrk

Ah, I'm not nearly as familiar with the inner workings of objects. I was just thinking in terms of cleanliness of code.

I don't generally think of the class attributes as providing the defaults, so much as just facilitating introspection (>>> MyClass?) prior to instantiating an object, and a cleaner view of attribute names when writing code without digging through init logic.

Does self.attr go through the class every time, or just the first time it's requested?

@fperez

That's the problem: if it's not set in the instance explicitly, it has to do a double lookup every single time.

Which is why I think with this pattern of class-based declarations (which I very much like) we should also follow up with setting things in the instance.

Please sign in to comment.
Something went wrong with that request. Please try again.