New keybinding for code cell execution + cell insertion #2090

v923z · 2012-07-03T19:03:42Z

Hi all,

As discussed in http://mail.scipy.org/pipermail/ipython-dev/2012-July/009844.html and thereafter, I have added a new Alt+Enter combination for executing a cell and inserting a new code cell, even if the present cell is not at the bottom of the notebook. Files notebook.js, and quickhelp.js are affected. Let me know, if this combination is not appropriate for some reason. We could still go with Cntr+Shift+Enter.

Cheers,
Zoltán

fperez · 2012-07-04T23:56:39Z

Can't look at the code right now, but I think this is super useful functionality. I keep myself wanting it all the time. Thanks!

Carreau · 2012-07-12T14:12:11Z

IPython/frontend/html/notebook/static/js/notebook.js

+            } else if (event.which === key.ENTER && event.altKey) {
+                that.execute_selected_cell();
+                that.insert_cell_above('code');
+                return false;


I guess you are using insert cell above because when executing the focus goes to the next cell...
Could you add a comment to explain it, it is not obvious when looking at the code.
Also, how does it behave at the end of a notebook ?

You could also add one line in one of the example notebook for user to discover it.

v923z · 2012-07-14T20:00:25Z

@Carreau You are right in your reasoning, and one of my reasons for implementing the feature in this particular way was that I didn't want to create untested code, it seemed so much safer to use proven and working code. As for the end of the notebook, you are right that this is going to create newer and newer cells, and perhaps, I could check for that. That should not be too hard, for there is code for that, too:)

I also wanted to point out earlier that I hadn't updated the user manual pages either. I will do that in the next iteration. But I would have a very dumb question here. There is a pending pull request here. What happens, if I implement what you suggest (checking of the last code cell), update the manual, and extend one of the notebooks? How will the PR be handled? Do I have to issue a new PR then, or I could still work on the code that I uploaded to my branch?

minrk · 2012-07-15T00:02:02Z

@v923z any changes you make to this branch will be reflected in the PR. For this reason, it is appropriate to create a PR when you have enough code to start a discussion about it (e.g. if you have a question about part of the implementation), and just continue to commit to the branch until it is done. You can also rebase on master and force push, in case you want to clean up a PR or squash commits.

…ng whether a new code cell is really needed.

v923z · 2012-07-19T19:45:25Z

@Carreau Thanks for the suggestions, I have a new implementation now. First, the code contains a couple of comment lines, second, I also check whether we really have to insert a new code cell. That would cover the case, when cell execution happens at a place where there is already an empty cell below. In such a case, the cursor simply jumps to the next (and empty) cell, but no new cell is created. This should prevent pollution of the notebook by lots of empty cells.

I am not sure whether the method that I used for checking for the emptiness of the cell is the best one. Brian could perhaps comment on this?

I have also added a section to the doc source file. In order to accommodate the explanation of the Alt-Enter combination, I had to change the section on 'Ctrl-Enter' a bit. I hope it is OK.

Carreau · 2012-07-20T04:50:21Z

IPython/frontend/html/notebook/static/js/notebook.js

+                // Execute code cell, and insert new in place
+                that.execute_selected_cell();
+                 // Only insert a new cell, if we ended up in an already populated cell
+                if (that.get_selected_cell().toJSON().input !== "") {


.toJSON().input == .get_text()in this case, and it won't work if cell has only \nand inside (and obviously it is considered empty).

Is the equality really true in this case? Would there be any differences in terms of performance? I have left .toJSON() in place, but if you say that get_text() is better, I could change that.

codecell.js

301 CodeCell.prototype.toJSON = function () {$ 302 var data = IPython.Cell.prototype.toJSON.apply(this);$ 303 >>>> data.input = this.get_text();$ 304 data.cell_type = 'code';$ 305 if (this.input_prompt_number) {$ 306 data.prompt_number = this.input_prompt_number;$ 307 };$ 308 var outputs = this.output_area.toJSON();$ 309 data.outputs = outputs;$ 310 data.language = 'python';$ 311 data.collapsed = this.collapsed;$ 312 return data;$ 313 };$

so toJSON is an unnecessary extra call as it uses .get_text() under the hood, and does not apply any transform to it.

Thanks for the comments! I have changed the code accordingly.

Carreau · 2012-07-20T04:53:16Z

And I'm far from beeing the best to review english in doc...

fperez · 2012-07-20T05:22:55Z

English looks good, and I'm +1 on the feature. Haven't reviewed the implementation though.

v923z · 2012-07-20T12:05:58Z

@Carreau You are absolutely right, so I added checking for this. A new cell will be inserted only if the cell below contains non-whitespace characters (tabs, spaces, and linefeeds will be regarded as empty cell.) I think this should address the issue you raised.

v923z · 2012-07-20T13:25:28Z

I don't want to throw this discussion off track, but I was wondering whether, along the lines that Matthias brought up, we should prevent "execution" of a cell, if it is empty, i.e., it contains only whitespaces. This could be done by adding
the single line

    if(/\S/.test(cell.get_text()) == false) return;

to the prototype function

Notebook.prototype.execute_selected_cell = function (options)

This would simply return, if there is nothing in the current code cell. Should I add this to the present pull request?

Carreau · 2012-07-20T14:17:17Z

I would say no, as it is a convenient way to execute an empty cell to remove output.

v923z · 2012-07-20T14:22:25Z

On one hand, I see your point, but on the other hand, one could simply delete the cell, couldn't they? If you are executing an empty cell in order to remove the output, you have to remove the input, too, so nothing is left, really...

Carreau · 2012-07-20T14:41:02Z

No, what I mean, if that if you want to start a cell from scratch, you will Ctrl-A, Ctrl-Enter, type.
Even Ctrl-A,Ctrl-C, Ctrl-Enter,Ctrl-V is faster than go for the menu to clear output.

Anyway, this shouldn't be added to the same pull request.

Notebook, Alt-enter : execute cell, append codecell below.

v923z · 2012-07-20T20:51:27Z

Matthias, thanks for pulling this in!

wltrimbl · 2013-01-14T17:06:47Z

According to this discussion thread:
http://grokbase.com/t/scipy.org/ipython-user/12bxs04f8f/notebook-alt-enter-on-the-mac
and my experience with ipython 0.13.1 on OSX 10.8 / Firefox 18
the Alt-enter keybinding has been implemented in a way that simply does not work on some Mac OS systems. These mac users must use the mouse to create insert new cells in the middle of a notebook.

According to this article, capturing alt- and such are involved:
http://unixpapa.com/js/key.html

Carreau · 2013-01-14T18:43:37Z

This will probably be fixable once we have properly configurable keybindings in notebook.
There are a lot of gotchas with all os/browser/layout and we can't please everyone right now.

But thanks for the link that might help when we work on that later.

Notebook, Alt-enter : execute cell, append codecell below.

Added new short key for cell execution

8659008

Carreau reviewed Jul 12, 2012
View reviewed changes

v923z added 2 commits July 19, 2012 22:32

Added new subsection to explain the Alt-Enter keyboard combination

d4e561e

Added some comments to the Alt-Enter code part, and also added checki…

39adfcd

…ng whether a new code cell is really needed.

Carreau reviewed Jul 20, 2012
View reviewed changes

Added checking for emptiness of cell below.

ef6efd0

Replaced .toJSON().input by .get_text()

d066af2

Carreau added a commit that referenced this pull request Jul 20, 2012

Merge pull request #2090 from v923z/master

3a57667

Notebook, Alt-enter : execute cell, append codecell below.

Carreau merged commit 3a57667 into ipython:master Jul 20, 2012

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request ipython#2090 from v923z/master

ef58da4

Notebook, Alt-enter : execute cell, append codecell below.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New keybinding for code cell execution + cell insertion #2090

New keybinding for code cell execution + cell insertion #2090

v923z commented Jul 3, 2012

fperez commented Jul 4, 2012

Carreau Jul 12, 2012

v923z commented Jul 14, 2012

minrk commented Jul 15, 2012

v923z commented Jul 19, 2012

Carreau Jul 20, 2012

v923z Jul 20, 2012

Carreau Jul 20, 2012

v923z Jul 20, 2012

Carreau commented Jul 20, 2012

fperez commented Jul 20, 2012

v923z commented Jul 20, 2012

v923z commented Jul 20, 2012

Carreau commented Jul 20, 2012

v923z commented Jul 20, 2012

Carreau commented Jul 20, 2012

v923z commented Jul 20, 2012

wltrimbl commented Jan 14, 2013

Carreau commented Jan 14, 2013

New keybinding for code cell execution + cell insertion #2090

New keybinding for code cell execution + cell insertion #2090

Conversation

v923z commented Jul 3, 2012

fperez commented Jul 4, 2012

Carreau Jul 12, 2012

Choose a reason for hiding this comment

v923z commented Jul 14, 2012

minrk commented Jul 15, 2012

v923z commented Jul 19, 2012

Carreau Jul 20, 2012

Choose a reason for hiding this comment

v923z Jul 20, 2012

Choose a reason for hiding this comment

Carreau Jul 20, 2012

Choose a reason for hiding this comment

v923z Jul 20, 2012

Choose a reason for hiding this comment

Carreau commented Jul 20, 2012

fperez commented Jul 20, 2012

v923z commented Jul 20, 2012

v923z commented Jul 20, 2012

Carreau commented Jul 20, 2012

v923z commented Jul 20, 2012

Carreau commented Jul 20, 2012

v923z commented Jul 20, 2012

wltrimbl commented Jan 14, 2013

Carreau commented Jan 14, 2013