add stdin to the notebook #3089
Conversation
|
The main thing remaining to do here is to prevent shift-enter from executing code while the raw_input filed is active. That can mess things up. |
required for stdin routing
dumb / gross / ugly jQuery modal dialog for now, but it works!
|
Minor UI comments:
|
follow example in rename notebook: remove form, bind keydown for enter, avoiding shift-enter submitting the cell again.
should fix alignment of raw_input and prompt
| @@ -507,6 +507,12 @@ def _inject_cookie_message(self, msg): | |||
| # under Python 2.x for some reason | |||
| msg = msg.encode('utf8', 'replace') | |||
| try: | |||
| bsession, msg = msg.split(':', 1) | |||
There was a problem hiding this comment.
I think the identity should be handled at the level of the handler rather than in the JS code and sent to the handler.
There was a problem hiding this comment.
Discussed on HipChat, this isn't possible - the identity of the StdIn and Shell sockets must match for input_request routing, and the only thing that has a handle on both at the same time is the Javascript.
There was a problem hiding this comment.
The name "bsession" is a bit confusing. Can you rename the variable to emphasize that this is the socket identity and provide some comments reminding us why we have to pass this and what it is for.
|
Shouldn't some of the channels have an |
|
I really like the cleanup/refactor of the zmq handlers. Much cleaner and easy to maintain. Amazing how simple it is! |
| this.iopub_channel.onmessage = $.proxy(this._handle_iopub_reply,this); | ||
| this.shell_channel.onmessage = $.proxy(this._handle_shell_reply, this); | ||
| this.iopub_channel.onmessage = $.proxy(this._handle_iopub_reply, this); | ||
| this.stdin_channel.onmessage = $.proxy(this._handle_input_request, this); |
There was a problem hiding this comment.
Maybe use stdin rather than input to be consistent in the naming?
There was a problem hiding this comment.
it's called an input_request, that's the message type. See our message spec
|
OK I have reviewed all of the code. Mostly minor comments and a bit of UI cleanup. |
I disagree - it should be a plain |
more crowded, less jumpy
|
I spoke too soon, the layout should match reasonably well now. |
|
OK, the look of the input area matches the pre quite well. Great work! Right now the input area is statically sized to 80 cols. When the notebook window is narrower than that, the layout gets messed up. It looks like it would be difficult to make it variable width. Do you think we need the full 80 cols for this? |
| bsession, msg = msg.split(':', 1) | ||
| self.session.session = bsession.decode('ascii') | ||
| except Exception: | ||
| logging.error("No bsession!", exc_info=True) |
There was a problem hiding this comment.
This error message is a bit cryptic.
|
Any response to the |
|
As for 80 cols - no, I don't expect it to be needed often, but I don't know how to make it auto-width. Sorry I didn't comment, but I did set the IOPub on_message to I also fixed the bsession / error message when the first message is not given. |
IPython changed the way to start channel [1]. fixes #121 [1] see commit ipython/ipython@24dcb6b in ipython/ipython#3089
add stdin to the notebook
adds a simple stdin handler to the notebook, so things like raw_input and %debug work.
%debug in action:
This PR depends on #3011 and #3088. Incentive!