GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — add stdin to the notebook by minrk · Pull Request #3089 · ipython/ipython

Conversation

@minrk

@minrk minrk commented Mar 27, 2013

Copy link
Copy Markdown
Member

adds a simple stdin handler to the notebook, so things like raw_input and %debug work.

%debug in action:

debug

This PR depends on #3011 and #3088. Incentive!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@minrk

minrk commented Mar 27, 2013

Copy link
Copy Markdown
Member Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
minrk added 6 commits April 25, 2013 12:24
@minrk
share code between zmq channel handlers
1b0c2f7
@minrk
add stdin channel to NotebookApp
8fae131
@minrk
specify socket identity from kernel.js
24dcb6b 
required for stdin routing
@minrk
add stdin to notebook
97db8db 
dumb / gross / ugly jQuery modal dialog for now,
but it works!
@minrk
flush stale messages on stdin channel prior to raw_input
8b1382e
@minrk
use inline raw_input instead of a dialog
ed8a7ce
@ellisonbg

ellisonbg commented Apr 25, 2013

Copy link
Copy Markdown
Member

Minor UI comments:

  • The text area used to type stdin should have the font properties as the regular monospace we are using, namely black.
  • We should probably think about how to size the width of the text area.
  • When a user hits return for stdin, it inserts a static version of the prompt and typed text below the input cell. This static version should have the same margins/padding as the actual prompt/text area, so there isn't a jump when a user presses return.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
minrk added 2 commits April 25, 2013 14:18
@minrk
cleanup stdin event submission
5631528 
follow example in rename notebook: remove form, bind keydown for enter,
avoiding shift-enter submitting the cell again.
@minrk
tweak raw-input styling
5a8093e 
should fix alignment of raw_input and prompt
ellisonbg
ellisonbg reviewed Apr 25, 2013
View reviewed changes
Comment thread IPython/frontend/html/notebook/handlers.py Outdated

@ellisonbg ellisonbg Apr 25, 2013

Copy link
Copy Markdown
Member

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 the identity should be handled at the level of the handler rather than in the JS code and sent to the handler.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@minrk minrk Apr 25, 2013

Copy link
Copy Markdown
Member Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@ellisonbg ellisonbg Apr 26, 2013

Copy link
Copy Markdown
Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ellisonbg

ellisonbg commented Apr 25, 2013

Copy link
Copy Markdown
Member

Shouldn't some of the channels have an on_message method that is different from the base class (like a pass)?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ellisonbg

ellisonbg commented Apr 25, 2013

Copy link
Copy Markdown
Member

I really like the cleanup/refactor of the zmq handlers. Much cleaner and easy to maintain. Amazing how simple it is!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ellisonbg
ellisonbg reviewed Apr 25, 2013
View reviewed changes
Comment thread IPython/frontend/html/notebook/static/js/kernel.js

@ellisonbg ellisonbg Apr 25, 2013

Copy link
Copy Markdown
Member

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.

Maybe use stdin rather than input to be consistent in the naming?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@ellisonbg ellisonbg Apr 25, 2013

Copy link
Copy Markdown
Member

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.

Also below...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@minrk minrk Apr 25, 2013

Copy link
Copy Markdown
Member Author

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's called an input_request, that's the message type. See our message spec

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ellisonbg

ellisonbg commented Apr 25, 2013

Copy link
Copy Markdown
Member

OK I have reviewed all of the code. Mostly minor comments and a bit of UI cleanup.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@minrk

minrk commented Apr 25, 2013

Copy link
Copy Markdown
Member Author

When a user hits return for stdin, it inserts a static version of the prompt and typed text below the input cell. This static version should have the same margins/padding as the actual prompt/text area, so there isn't a jump when a user presses return.

I disagree - it should be a plain pre tag for this, but I'm not sure that I can make an input area with zero margins, so I don't think this is possible.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
minrk added 3 commits April 25, 2013 16:07
@minrk
add no-op on_message for iopub
2142444
@minrk
fix color in raw_input
59f59aa
@minrk
tweak raw_input style
641fc2f 
more crowded, less jumpy
@minrk

minrk commented Apr 26, 2013

Copy link
Copy Markdown
Member Author

I spoke too soon, the layout should match reasonably well now.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ellisonbg

ellisonbg commented Apr 26, 2013

Copy link
Copy Markdown
Member

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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ellisonbg
ellisonbg reviewed Apr 26, 2013
View reviewed changes
Comment thread IPython/frontend/html/notebook/handlers.py Outdated

@ellisonbg ellisonbg Apr 26, 2013

Copy link
Copy Markdown
Member

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.

This error message is a bit cryptic.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ellisonbg

ellisonbg commented Apr 26, 2013

Copy link
Copy Markdown
Member

Any response to the on_message behavior?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@minrk
clarify first ws message names / messages
5d6d133
@minrk

minrk commented Apr 26, 2013

Copy link
Copy Markdown
Member Author

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 pass, since IOPub downstream messages don't make any sense.

I also fixed the bsession / error message when the first message is not given.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@minrk
js comment about session id in first message
5b087cd
ellisonbg added a commit that referenced this pull request Apr 26, 2013
@ellisonbg
Merge pull request #3089 from minrk/stdin
c3a9044 
add stdin to the notebook
@ellisonbg
ellisonbg merged commit c3a9044 into ipython:master Apr 26, 2013
@tkf tkf mentioned this pull request Jun 11, 2013
Implement stdin channel tkf/emacs-ipython-notebook#115
Open
tkf added a commit to tkf/emacs-ipython-notebook that referenced this pull request Jul 10, 2013
@tkf
Use IPython 1.0-compatible "handshake"
146fc45 
IPython changed the way to start channel [1].
fixes #121

[1] see commit ipython/ipython@24dcb6b
in ipython/ipython#3089
@tkf tkf mentioned this pull request Jul 10, 2013
Use IPython 1.0-compatible "handshake" to open kernel channels tkf/emacs-ipython-notebook#122
Open
@minrk
minrk deleted the stdin branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
@ellisonbg
Merge pull request ipython#3089 from minrk/stdin
1ff0ee6 
add stdin to the notebook
@v0dro v0dro mentioned this pull request Nov 3, 2016
Implement stdin SciRuby/iruby#24
Closed
@davidedelvento davidedelvento mentioned this pull request May 3, 2017
Problems with shell commands (magic) expecting output #10499
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@minrk @ellisonbg
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.