From 990425f034461e0138f8fcf965ce8a07c11874ba Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 25 Sep 2014 16:14:28 -0700 Subject: [PATCH 1/4] Handle 'deletable' cell metadata --- IPython/html/static/notebook/js/cell.js | 21 ++++++++++++++++----- IPython/html/static/notebook/js/notebook.js | 10 +++++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/IPython/html/static/notebook/js/cell.js b/IPython/html/static/notebook/js/cell.js index e695b3e54..d764ab786 100644 --- a/IPython/html/static/notebook/js/cell.js +++ b/IPython/html/static/notebook/js/cell.js @@ -385,7 +385,8 @@ define([ **/ Cell.prototype.toJSON = function () { var data = {}; - data.metadata = this.metadata; + // deepcopy the metadata so copied cells don't share the same object + data.metadata = JSON.parse(JSON.stringify(this.metadata)); data.cell_type = this.cell_type; return data; }; @@ -404,22 +405,32 @@ define([ /** - * can the cell be split into two cells + * can the cell be split into two cells (false if not deletable) * @method is_splittable **/ Cell.prototype.is_splittable = function () { - return true; + return this.is_deletable(); }; /** - * can the cell be merged with other cells + * can the cell be merged with other cells (false if not deletable) * @method is_mergeable **/ Cell.prototype.is_mergeable = function () { - return true; + return this.is_deletable(); }; + /** + * is the cell deletable? (true by default) + * @method is_deletable + **/ + Cell.prototype.is_deletable = function () { + if (this.metadata.deletable === undefined) { + return true; + } + return Boolean(this.metadata.deletable); + }; /** * @return {String} - the text before the cursor diff --git a/IPython/html/static/notebook/js/notebook.js b/IPython/html/static/notebook/js/notebook.js index ac8b220a3..4235774d8 100644 --- a/IPython/html/static/notebook/js/notebook.js +++ b/IPython/html/static/notebook/js/notebook.js @@ -765,7 +765,11 @@ define([ */ Notebook.prototype.delete_cell = function (index) { var i = this.index_or_selected(index); - var cell = this.get_selected_cell(); + var cell = this.get_cell(i); + if (!cell.is_deletable()) { + return this; + } + this.undelete_backup = cell.toJSON(); $('#undelete_cell').removeClass('disabled'); if (this.is_valid_cell_index(i)) { @@ -1193,6 +1197,10 @@ define([ Notebook.prototype.copy_cell = function () { var cell = this.get_selected_cell(); this.clipboard = cell.toJSON(); + // remove undeletable status from the copied cell + if (this.clipboard.metadata.deletable !== undefined) { + delete this.clipboard.metadata.deletable; + } this.enable_paste(); }; From a018cb42994b11132ec7a60d8b24923e1be39224 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 25 Sep 2014 16:14:49 -0700 Subject: [PATCH 2/4] Add tests for undeletable cells --- IPython/html/tests/notebook/deletecell.js | 118 ++++++++++++++++++ IPython/html/tests/notebook/dualmode_merge.js | 118 +++++++++++++++++- 2 files changed, 234 insertions(+), 2 deletions(-) create mode 100644 IPython/html/tests/notebook/deletecell.js diff --git a/IPython/html/tests/notebook/deletecell.js b/IPython/html/tests/notebook/deletecell.js new file mode 100644 index 000000000..477a8bddc --- /dev/null +++ b/IPython/html/tests/notebook/deletecell.js @@ -0,0 +1,118 @@ + +// Test +casper.notebook_test(function () { + var that = this; + var cell_is_deletable = function (index) { + // Get the deletable status of a cell. + return that.evaluate(function (index) { + var cell = IPython.notebook.get_cell(index); + return cell.is_deletable(); + }, index); + }; + + var a = 'print("a")'; + var index = this.append_cell(a); + + var b = 'print("b")'; + index = this.append_cell(b); + + var c = 'print("c")'; + index = this.append_cell(c); + + this.thenEvaluate(function() { + IPython.notebook.get_cell(1).metadata.deletable = false; + IPython.notebook.get_cell(2).metadata.deletable = 0; + IPython.notebook.get_cell(3).metadata.deletable = true; + }); + + this.then(function () { + // Check deletable status of the cells + this.test.assert(cell_is_deletable(0), 'Cell 0 is deletable'); + this.test.assert(!cell_is_deletable(1), 'Cell 1 is not deletable'); + this.test.assert(!cell_is_deletable(2), 'Cell 2 is not deletable'); + this.test.assert(cell_is_deletable(3), 'Cell 3 is deletable'); + }); + + // Try to delete cell 0 (should succeed) + this.then(function () { + this.select_cell(0); + this.trigger_keydown('esc'); + this.trigger_keydown('d', 'd'); + this.test.assertEquals(this.get_cells_length(), 3, 'Delete cell 0: There are now 3 cells'); + this.test.assertEquals(this.get_cell_text(0), a, 'Delete cell 0: Cell 1 is now cell 0'); + this.test.assertEquals(this.get_cell_text(1), b, 'Delete cell 0: Cell 2 is now cell 1'); + this.test.assertEquals(this.get_cell_text(2), c, 'Delete cell 0: Cell 3 is now cell 2'); + this.validate_notebook_state('dd', 'command', 0); + }); + + // Try to delete cell 0 (should fail) + this.then(function () { + this.select_cell(0); + this.trigger_keydown('d', 'd'); + this.test.assertEquals(this.get_cells_length(), 3, 'Delete cell 0: There are still 3 cells'); + this.test.assertEquals(this.get_cell_text(0), a, 'Delete cell 0: Cell 0 was not deleted'); + this.test.assertEquals(this.get_cell_text(1), b, 'Delete cell 0: Cell 1 was not affected'); + this.test.assertEquals(this.get_cell_text(2), c, 'Delete cell 0: Cell 2 was not affected'); + this.validate_notebook_state('dd', 'command', 0); + }); + + // Try to delete cell 1 (should fail) + this.then(function () { + this.select_cell(1); + this.trigger_keydown('d', 'd'); + this.test.assertEquals(this.get_cells_length(), 3, 'Delete cell 1: There are still 3 cells'); + this.test.assertEquals(this.get_cell_text(0), a, 'Delete cell 1: Cell 0 was not affected'); + this.test.assertEquals(this.get_cell_text(1), b, 'Delete cell 1: Cell 1 was not deleted'); + this.test.assertEquals(this.get_cell_text(2), c, 'Delete cell 1: Cell 2 was not affected'); + this.validate_notebook_state('dd', 'command', 1); + }); + + // Try to delete cell 2 (should succeed) + this.then(function () { + this.select_cell(2); + this.trigger_keydown('d', 'd'); + this.test.assertEquals(this.get_cells_length(), 2, 'Delete cell 2: There are now 2 cells'); + this.test.assertEquals(this.get_cell_text(0), a, 'Delete cell 2: Cell 0 was not affected'); + this.test.assertEquals(this.get_cell_text(1), b, 'Delete cell 2: Cell 1 was not affected'); + this.validate_notebook_state('dd', 'command', 1); + }); + + // Change the deletable status of the last two cells + this.thenEvaluate(function() { + IPython.notebook.get_cell(0).metadata.deletable = true; + IPython.notebook.get_cell(1).metadata.deletable = true; + }); + + this.then(function () { + // Check deletable status of the cells + this.test.assert(cell_is_deletable(0), 'Cell 0 is deletable'); + this.test.assert(cell_is_deletable(1), 'Cell 1 is deletable'); + + // Try to delete cell 1 (should succeed) + this.select_cell(1); + this.trigger_keydown('d', 'd'); + this.test.assertEquals(this.get_cells_length(), 1, 'Delete cell 1: There is now 1 cell'); + this.test.assertEquals(this.get_cell_text(0), a, 'Delete cell 1: Cell 0 was not affected'); + this.validate_notebook_state('dd', 'command', 0); + + // Try to delete the last cell (should succeed) + this.select_cell(0); + this.trigger_keydown('d', 'd'); + this.test.assertEquals(this.get_cells_length(), 1, 'Delete last cell: There is still 1 cell'); + this.test.assertEquals(this.get_cell_text(0), "", 'Delete last cell: Cell 0 was deleted'); + this.validate_notebook_state('dd', 'command', 0); + }); + + // Make sure copied cells are deletable + this.thenEvaluate(function() { + IPython.notebook.get_cell(0).metadata.deletable = false; + }); + this.then(function () { + this.select_cell(0); + this.trigger_keydown('c', 'v'); + this.test.assertEquals(this.get_cells_length(), 2, 'Copy cell: There are 2 cells'); + this.test.assert(!cell_is_deletable(0), 'Cell 0 is not deletable'); + this.test.assert(cell_is_deletable(1), 'Cell 1 is deletable'); + this.validate_notebook_state('cv', 'command', 1); + }); +}); diff --git a/IPython/html/tests/notebook/dualmode_merge.js b/IPython/html/tests/notebook/dualmode_merge.js index 573b4575d..8ec32405f 100644 --- a/IPython/html/tests/notebook/dualmode_merge.js +++ b/IPython/html/tests/notebook/dualmode_merge.js @@ -1,6 +1,33 @@ // Test casper.notebook_test(function () { + var a = 'ab\ncd'; + var b = 'print("b")'; + var c = 'print("c")'; + + var that = this; + var cell_is_mergeable = function (index) { + // Get the mergeable status of a cell. + return that.evaluate(function (index) { + var cell = IPython.notebook.get_cell(index); + return cell.is_mergeable(); + }, index); + }; + + var cell_is_splittable = function (index) { + // Get the splittable status of a cell. + return that.evaluate(function (index) { + var cell = IPython.notebook.get_cell(index); + return cell.is_splittable(); + }, index); + }; + + var close_dialog = function () { + this.evaluate(function(){ + $('div.modal-footer button.btn-default').click(); + }, {}); + }; + this.then(function () { // Split and merge cells this.select_cell(0); @@ -16,6 +43,93 @@ casper.notebook_test(function () { this.select_cell(0); // Move up to cell 0 this.trigger_keydown('shift-m'); // Merge this.validate_notebook_state('merge', 'command', 0); - this.test.assertEquals(this.get_cell_text(0), 'ab\ncd', 'merge; Verify that cell 0 has the merged contents.'); + this.test.assertEquals(this.get_cell_text(0), a, 'merge; Verify that cell 0 has the merged contents.'); + }); + + // add some more cells and test splitting/merging when a cell is not deletable + this.then(function () { + this.append_cell(b); + this.append_cell(c); + }); + + this.thenEvaluate(function() { + IPython.notebook.get_cell(1).metadata.deletable = false; + }); + + // Check that merge/split status are correct + this.then(function () { + this.test.assert(cell_is_splittable(0), 'Cell 0 is splittable'); + this.test.assert(cell_is_mergeable(0), 'Cell 0 is mergeable'); + this.test.assert(!cell_is_splittable(1), 'Cell 1 is not splittable'); + this.test.assert(!cell_is_mergeable(1), 'Cell 1 is not mergeable'); + this.test.assert(cell_is_splittable(2), 'Cell 2 is splittable'); + this.test.assert(cell_is_mergeable(2), 'Cell 2 is mergeable'); + }); + + // Try to merge cell 0 below with cell 1 + this.then(function () { + this.select_cell(0); + this.trigger_keydown('esc'); + this.trigger_keydown('shift-m'); + this.test.assertEquals(this.get_cells_length(), 3, 'Merge cell 0 down: There are still 3 cells'); + this.test.assertEquals(this.get_cell_text(0), a, 'Merge cell 0 down: Cell 0 is unchanged'); + this.test.assertEquals(this.get_cell_text(1), b, 'Merge cell 0 down: Cell 1 is unchanged'); + this.test.assertEquals(this.get_cell_text(2), c, 'Merge cell 0 down: Cell 2 is unchanged'); + this.validate_notebook_state('shift-m', 'command', 0); + }); + + // Try to merge cell 1 above with cell 0 + this.then(function () { + this.select_cell(1); + }); + this.thenEvaluate(function () { + IPython.notebook.merge_cell_above(); + }); + this.then(function () { + this.test.assertEquals(this.get_cells_length(), 3, 'Merge cell 1 up: There are still 3 cells'); + this.test.assertEquals(this.get_cell_text(0), a, 'Merge cell 1 up: Cell 0 is unchanged'); + this.test.assertEquals(this.get_cell_text(1), b, 'Merge cell 1 up: Cell 1 is unchanged'); + this.test.assertEquals(this.get_cell_text(2), c, 'Merge cell 1 up: Cell 2 is unchanged'); + this.validate_notebook_state('merge up', 'command', 1); + }); + + // Try to split cell 1 + this.then(function () { + this.select_cell(1); + this.trigger_keydown('enter'); + this.set_cell_editor_cursor(1, 0, 2); + this.trigger_keydown('ctrl-shift-subtract'); // Split + this.test.assertEquals(this.get_cells_length(), 3, 'Split cell 1: There are still 3 cells'); + this.test.assertEquals(this.get_cell_text(0), a, 'Split cell 1: Cell 0 is unchanged'); + this.test.assertEquals(this.get_cell_text(1), b, 'Split cell 1: Cell 1 is unchanged'); + this.test.assertEquals(this.get_cell_text(2), c, 'Split cell 1: Cell 2 is unchanged'); + this.validate_notebook_state('ctrl-shift-subtract', 'edit', 1); + }); + + // Try to merge cell 1 down + this.then(function () { + this.select_cell(1); + this.trigger_keydown('esc'); + this.trigger_keydown('shift-m'); + this.test.assertEquals(this.get_cells_length(), 3, 'Merge cell 1 down: There are still 3 cells'); + this.test.assertEquals(this.get_cell_text(0), a, 'Merge cell 1 down: Cell 0 is unchanged'); + this.test.assertEquals(this.get_cell_text(1), b, 'Merge cell 1 down: Cell 1 is unchanged'); + this.test.assertEquals(this.get_cell_text(2), c, 'Merge cell 1 down: Cell 2 is unchanged'); + this.validate_notebook_state('shift-m', 'command', 1); + }); + + // Try to merge cell 2 above with cell 1 + this.then(function () { + this.select_cell(2); + }); + this.thenEvaluate(function () { + IPython.notebook.merge_cell_above(); + }); + this.then(function () { + this.test.assertEquals(this.get_cells_length(), 3, 'Merge cell 2 up: There are still 3 cells'); + this.test.assertEquals(this.get_cell_text(0), a, 'Merge cell 2 up: Cell 0 is unchanged'); + this.test.assertEquals(this.get_cell_text(1), b, 'Merge cell 2 up: Cell 1 is unchanged'); + this.test.assertEquals(this.get_cell_text(2), c, 'Merge cell 2 up: Cell 2 is unchanged'); + this.validate_notebook_state('merge up', 'command', 2); }); -}); \ No newline at end of file +}); From 10d500525a048869a9f6322b0e3571c245cc5fe8 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 25 Sep 2014 16:16:49 -0700 Subject: [PATCH 3/4] Make cell be undeletable ONLY when metadata is explicitly false --- IPython/html/static/notebook/js/cell.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/IPython/html/static/notebook/js/cell.js b/IPython/html/static/notebook/js/cell.js index d764ab786..e0a81655f 100644 --- a/IPython/html/static/notebook/js/cell.js +++ b/IPython/html/static/notebook/js/cell.js @@ -422,14 +422,17 @@ define([ }; /** - * is the cell deletable? (true by default) + * is the cell deletable? only false (undeletable) if + * metadata.deletable is explicitly false -- everything else + * counts as true + * * @method is_deletable **/ Cell.prototype.is_deletable = function () { - if (this.metadata.deletable === undefined) { - return true; + if (this.metadata.deletable === false) { + return false; } - return Boolean(this.metadata.deletable); + return true; }; /** From 756d4063c3fe648e9fa4ac8c3da0afac99d93c09 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 25 Sep 2014 16:23:04 -0700 Subject: [PATCH 4/4] Fix tests --- IPython/html/tests/notebook/deletecell.js | 33 ++++++++--------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/IPython/html/tests/notebook/deletecell.js b/IPython/html/tests/notebook/deletecell.js index 477a8bddc..6c98f28cc 100644 --- a/IPython/html/tests/notebook/deletecell.js +++ b/IPython/html/tests/notebook/deletecell.js @@ -21,7 +21,7 @@ casper.notebook_test(function () { this.thenEvaluate(function() { IPython.notebook.get_cell(1).metadata.deletable = false; - IPython.notebook.get_cell(2).metadata.deletable = 0; + IPython.notebook.get_cell(2).metadata.deletable = 0; // deletable only when exactly false IPython.notebook.get_cell(3).metadata.deletable = true; }); @@ -29,7 +29,7 @@ casper.notebook_test(function () { // Check deletable status of the cells this.test.assert(cell_is_deletable(0), 'Cell 0 is deletable'); this.test.assert(!cell_is_deletable(1), 'Cell 1 is not deletable'); - this.test.assert(!cell_is_deletable(2), 'Cell 2 is not deletable'); + this.test.assert(cell_is_deletable(2), 'Cell 2 is deletable'); this.test.assert(cell_is_deletable(3), 'Cell 3 is deletable'); }); @@ -56,44 +56,33 @@ casper.notebook_test(function () { this.validate_notebook_state('dd', 'command', 0); }); - // Try to delete cell 1 (should fail) + // Try to delete cell 1 (should succeed) this.then(function () { this.select_cell(1); this.trigger_keydown('d', 'd'); - this.test.assertEquals(this.get_cells_length(), 3, 'Delete cell 1: There are still 3 cells'); + this.test.assertEquals(this.get_cells_length(), 2, 'Delete cell 1: There are now 2 cells'); this.test.assertEquals(this.get_cell_text(0), a, 'Delete cell 1: Cell 0 was not affected'); - this.test.assertEquals(this.get_cell_text(1), b, 'Delete cell 1: Cell 1 was not deleted'); - this.test.assertEquals(this.get_cell_text(2), c, 'Delete cell 1: Cell 2 was not affected'); + this.test.assertEquals(this.get_cell_text(1), c, 'Delete cell 1: Cell 1 was not affected'); this.validate_notebook_state('dd', 'command', 1); }); - // Try to delete cell 2 (should succeed) + // Try to delete cell 1 (should succeed) this.then(function () { - this.select_cell(2); + this.select_cell(1); this.trigger_keydown('d', 'd'); - this.test.assertEquals(this.get_cells_length(), 2, 'Delete cell 2: There are now 2 cells'); + this.test.assertEquals(this.get_cells_length(), 1, 'Delete cell 1: There is now 1 cell'); this.test.assertEquals(this.get_cell_text(0), a, 'Delete cell 2: Cell 0 was not affected'); - this.test.assertEquals(this.get_cell_text(1), b, 'Delete cell 2: Cell 1 was not affected'); - this.validate_notebook_state('dd', 'command', 1); + this.validate_notebook_state('dd', 'command', 0); }); - // Change the deletable status of the last two cells + // Change the deletable status of the last cells this.thenEvaluate(function() { IPython.notebook.get_cell(0).metadata.deletable = true; - IPython.notebook.get_cell(1).metadata.deletable = true; }); this.then(function () { - // Check deletable status of the cells + // Check deletable status of the cell this.test.assert(cell_is_deletable(0), 'Cell 0 is deletable'); - this.test.assert(cell_is_deletable(1), 'Cell 1 is deletable'); - - // Try to delete cell 1 (should succeed) - this.select_cell(1); - this.trigger_keydown('d', 'd'); - this.test.assertEquals(this.get_cells_length(), 1, 'Delete cell 1: There is now 1 cell'); - this.test.assertEquals(this.get_cell_text(0), a, 'Delete cell 1: Cell 0 was not affected'); - this.validate_notebook_state('dd', 'command', 0); // Try to delete the last cell (should succeed) this.select_cell(0);