Add a simple 'undo' for cell deletion.

[dwf]

@Carreau's amazing work on the slideshow mode branch inspired me to try and fix something in IPython that's been bothering me for a little while: that there's no way to undo cell deletion.

I've accidentally deleted the wrong cell quite a few times and it has been quite frustrating. This enables a very hacky backup of the last cell deletion.

I'm sure the core devs will have some opinions on both the feature and its implementation, I consider this very much a first pass. A conversation starter, I guess.

[Carreau]

I suppose 90 is z a little comment below would be great :-)

[dwf]

Fixed in 332a14b.

[Carreau]

That seem like a good start :-)

If we want multiple cell in the undo, we can still have as lists

this.undelete_backup = null;
this.undelete_index = null;
this.undelete_below = false;

(well list of dict) [{backup:..,index:...,below:...},{backup:..,index:...,below:...},] and push and pop from it.
But it might be a little too overkill and fill the memory to have a list of undo.

@ellisonbg might want to take a look.

I'm wondering if there is possibility to share more implementation with copy/past like a
past_cell_at_index(celldata,index) then you just have to call this either with cut/copy/past buffer or the undo buffer.

[dwf]

I thought about an undo stack. If we were to go the stack route, limiting the size (and making the least recently deleted "fall off" when you push on a full stack) is almost certainly the way to go. It's a bit weird to have an undo stack for only one kind of action. Given that it's main use is to recover from an "OH SH--" moment of accidentally hitting Ctrl+M D in the wrong cell, it may be overkill. It might even make sense to invalidate the undelete buffer on other kinds of actions, I don't know.

I thought about refactoring to share code but had the problem that in certain special cases I needed to paste below in order to get sensible behaviour, so there wasn't much refactoring I could do. I suppose a paste_cell_at_index(celldata, index, below) with a boolean third argument could be a useful instantiation of that idea.

[tkf]

Why not implement copy/paste stack and disable the delete command? And then user can use cut command when he want to remove a cell. I am almost sure implementing copy/paste stack is easier than undo/redo stack. But I guess you are going to implement undo/redo command at some point anyway so probably it's not bad to start it now.

[Carreau]

Why not implement copy/paste stack and disable the delete command? And then user can use cut command when he want to remove a cell

This is annoying for most user, you might want to delete without putting in the clipboard.

I suppose a paste_cell_at_index(celldata, index, below) with a boolean third argument could be a useful instantiation of that idea.

I'm not sure I see why, if index is the index of inserted cell after it has been inserted it looks enough for me... but I'll re-look at it later, still I'm not against a third argument.

[tkf]

Loosing cell is more annoying than filling up clipboard, I suppose. If you have a way to choose what's in the clipboard, I'd say it's an OK UI.

I was suggesting to implement clipboard stack first because I think it is difficult to implement undo stack due to asynchronous nature of notebook. What should be the undo of "execute a code cell" command? What is the undo after you execute a cell, clear the output after some of execution replies arrived, and then the rest of execution replies arrive? Implementing undo is certainly a doable job but I had the impression that ipython dev won't release notebook with half baked undo feature. So I was just suggesting the easier step.

[Carreau]

In the sens of a full undo stack, I understand. Though, entangling clipboard and undo means the user will get used to it and will complain if one day we remove.

There already is some undoing on a cell level, but I don't think we will move toward a bigger 'undoing stack'.
The reason is that we want to go to collaborative mode, where undoing is a really complex subject.
With a vcs backend, we could provide snapshot of the notebook.

[dwf]

This is annoying for most user, you might want to delete without putting in the clipboard.

Agreed.

I'm not sure I see why, if index is the index of inserted cell after it has been inserted it looks enough for me... but I'll re-look at it later, still I'm not against a third argument.

If you look at https://github.com/ipython/ipython/pull/2549/files#L0R555 this is what I'm talking about: without this logic I couldn't get it to work right in that it would paste above the last cell. If paste above with an index greater than ncells() works as expected, then maybe this could be simplified.

Also, I just realized that implementing more than one level of undo makes the index bookkeeping complicated, if we want to have them reappear in sensible places. Perhaps insertions before undelete_index should also increment it if it is not null?

Usually if I want to undelete something I realize it pretty much immediately, so this might be added complexity for not much practical gain.

[Carreau]

Also, I just realized that implementing more than one level of undo makes the index bookkeeping complicated, if we want to have them reappear in sensible places.

Other option would be to only 'hide' cell with css display none. but then bookkeeping an index of visible cell and the cursor to next visible cell is not pretty...

I think undooing one cell deletion is good enough.

I'll leave @ellisonbg have a look before merging.

[Carreau]

Ok, let's keep it this way.
Merging.