From 450597d754addeeb256bc904d0a567bf9889287f Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Sun, 9 Aug 2015 16:26:00 -0700 Subject: [PATCH 1/6] First attempt at multi-cell selection It works, but I'm not quite happy with how it works --- notebook/static/notebook/js/actions.js | 22 +++++++++ notebook/static/notebook/js/cell.js | 19 ++++---- notebook/static/notebook/js/codecell.js | 8 ++-- .../static/notebook/js/keyboardmanager.js | 2 + notebook/static/notebook/js/notebook.js | 46 ++++++++++++++++--- 5 files changed, 79 insertions(+), 18 deletions(-) diff --git a/notebook/static/notebook/js/actions.js b/notebook/static/notebook/js/actions.js index a99ab0c99..368ad0a36 100644 --- a/notebook/static/notebook/js/actions.js +++ b/notebook/static/notebook/js/actions.js @@ -100,6 +100,28 @@ define(function(require){ } } }, + 'extend-selection-previous' : { + help: 'extend selection above', + help_index : 'dc', + handler : function (env) { + var index = env.notebook.get_selected_index(); + if (index !== (env.notebook.ncells()-1) && index !== null) { + env.notebook.select_prev(true); + env.notebook.focus_cell(); + } + } + }, + 'extend-selection-next' : { + help: 'extend selection below', + help_index : 'dd', + handler : function (env) { + var index = env.notebook.get_selected_index(); + if (index !== (env.notebook.ncells()-1) && index !== null) { + env.notebook.select_next(true); + env.notebook.focus_cell(); + } + } + }, 'cut-selected-cell' : { icon: 'fa-cut', help_index : 'ee', diff --git a/notebook/static/notebook/js/cell.js b/notebook/static/notebook/js/cell.js index 56f8328a7..2a0e07403 100644 --- a/notebook/static/notebook/js/cell.js +++ b/notebook/static/notebook/js/cell.js @@ -55,6 +55,7 @@ define([ this.placeholder = config.placeholder || ''; this.selected = false; + this.in_selection = false; this.rendered = false; this.mode = 'command'; @@ -142,7 +143,7 @@ define([ * Call after this.element exists to initialize the css classes * related to selected, rendered and mode. */ - if (this.selected) { + if (this.in_selection) { this.element.addClass('selected'); } else { this.element.addClass('unselected'); @@ -254,6 +255,7 @@ define([ this.element.addClass('selected'); this.element.removeClass('unselected'); this.selected = true; + this.in_selection = true; return true; } else { return false; @@ -261,19 +263,20 @@ define([ }; /** - * handle cell level logic when a cell is unselected + * handle cell level logic when the cursor moves away from a cell * @method unselect + * @param {bool} leave_selected - true to move cursor away and extend selection * @return is the action being taken */ - Cell.prototype.unselect = function () { - if (this.selected) { + Cell.prototype.unselect = function (leave_selected) { + var was_selected_cell = this.selected; + this.selected = false; + if ((!leave_selected) && this.in_selection) { + this.in_selection = false; this.element.addClass('unselected'); this.element.removeClass('selected'); - this.selected = false; - return true; - } else { - return false; } + return was_selected_cell; }; /** diff --git a/notebook/static/notebook/js/codecell.js b/notebook/static/notebook/js/codecell.js index a3d7fd227..6f5ebc19b 100644 --- a/notebook/static/notebook/js/codecell.js +++ b/notebook/static/notebook/js/codecell.js @@ -535,14 +535,14 @@ define([ }; /** - * handle cell level logic when a cell is unselected + * handle cell level logic when the cursor moves away from a cell * @method unselect * @return is the action being taken */ - CodeCell.prototype.unselect = function () { - var cont = Cell.prototype.unselect.apply(this); + CodeCell.prototype.unselect = function (leave_selected) { + var cont = Cell.prototype.unselect.apply(this, [leave_selected]); if (cont) { - // When a code cell is usnelected, make sure that the corresponding + // When a code cell is unselected, make sure that the corresponding // tooltip and completer to that cell is closed. this.tooltip.remove_and_cancel_tooltip(true); if (this.completer !== null) { diff --git a/notebook/static/notebook/js/keyboardmanager.js b/notebook/static/notebook/js/keyboardmanager.js index 505b0b597..1a220d36c 100644 --- a/notebook/static/notebook/js/keyboardmanager.js +++ b/notebook/static/notebook/js/keyboardmanager.js @@ -98,6 +98,8 @@ define([ 'up' : 'ipython.select-previous-cell', 'k' : 'ipython.select-previous-cell', 'j' : 'ipython.select-next-cell', + 'shift-k': 'ipython.extend-selection-previous', + 'shift-j': 'ipython.extend-selection-next', 'x' : 'ipython.cut-selected-cell', 'c' : 'ipython.copy-selected-cell', 'v' : 'ipython.paste-cell-after', diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index 06179597a..a5146673a 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -600,6 +600,17 @@ define(function (require) { return result; }; + /** + * Get an array of the cells in the currently selected range + * + * @return {Array} The selected cells + */ + Notebook.prototype.get_selected_cells = function () { + return this.get_cells().filter(function(cell) { + return cell.in_selection; + }); + }; + // Cell selection. @@ -607,9 +618,10 @@ define(function (require) { * Programmatically select a cell. * * @param {integer} index - A cell's index + * @param {bool} add_to_selection - whether to add to the current selection * @return {Notebook} This notebook */ - Notebook.prototype.select = function (index) { + Notebook.prototype.select = function (index, add_to_selection) { if (this.is_valid_cell_index(index)) { var sindex = this.get_selected_index(); if (sindex !== null && index !== sindex) { @@ -618,8 +630,12 @@ define(function (require) { if (this.mode !== 'command') { this.command_mode(); } - this.get_cell(sindex).unselect(); } + var current_selection = this.get_selected_cells(); + for (var i=0; i Date: Sun, 9 Aug 2015 17:23:48 -0700 Subject: [PATCH 2/6] Multi-cell selection based on a selection anchor cell I'm happier with this mechanism --- notebook/static/notebook/js/actions.js | 6 +- notebook/static/notebook/js/cell.js | 2 + notebook/static/notebook/js/notebook.js | 92 +++++++++++++++---------- 3 files changed, 61 insertions(+), 39 deletions(-) diff --git a/notebook/static/notebook/js/actions.js b/notebook/static/notebook/js/actions.js index 368ad0a36..5fe0db074 100644 --- a/notebook/static/notebook/js/actions.js +++ b/notebook/static/notebook/js/actions.js @@ -105,8 +105,8 @@ define(function(require){ help_index : 'dc', handler : function (env) { var index = env.notebook.get_selected_index(); - if (index !== (env.notebook.ncells()-1) && index !== null) { - env.notebook.select_prev(true); + if (index !== 0 && index !== null) { + env.notebook.extend_selection('up'); env.notebook.focus_cell(); } } @@ -117,7 +117,7 @@ define(function(require){ handler : function (env) { var index = env.notebook.get_selected_index(); if (index !== (env.notebook.ncells()-1) && index !== null) { - env.notebook.select_next(true); + env.notebook.extend_selection('down'); env.notebook.focus_cell(); } } diff --git a/notebook/static/notebook/js/cell.js b/notebook/static/notebook/js/cell.js index 2a0e07403..a2f1a6e7f 100644 --- a/notebook/static/notebook/js/cell.js +++ b/notebook/static/notebook/js/cell.js @@ -56,6 +56,7 @@ define([ this.placeholder = config.placeholder || ''; this.selected = false; this.in_selection = false; + this.selection_anchor = false; this.rendered = false; this.mode = 'command'; @@ -273,6 +274,7 @@ define([ this.selected = false; if ((!leave_selected) && this.in_selection) { this.in_selection = false; + this.selection_anchor = false; this.element.addClass('unselected'); this.element.removeClass('selected'); } diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index a5146673a..976c9f95b 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -600,6 +600,21 @@ define(function (require) { return result; }; + /** + * Get the index of the anchor cell for range selection + * + * @return {integer} The anchor cell's numeric index + */ + Notebook.prototype.get_selection_anchor = function() { + var result = null; + this.get_cell_elements().filter(function (index) { + if ($(this).data("cell").selection_anchor === true) { + result = index; + } + }); + return result; + }; + /** * Get an array of the cells in the currently selected range * @@ -618,10 +633,9 @@ define(function (require) { * Programmatically select a cell. * * @param {integer} index - A cell's index - * @param {bool} add_to_selection - whether to add to the current selection * @return {Notebook} This notebook */ - Notebook.prototype.select = function (index, add_to_selection) { + Notebook.prototype.select = function (index) { if (this.is_valid_cell_index(index)) { var sindex = this.get_selected_index(); if (sindex !== null && index !== sindex) { @@ -633,64 +647,70 @@ define(function (require) { } var current_selection = this.get_selected_cells(); for (var i=0; i anchor_ix) ? 'down' : 'up'; + var contracting = (cursor_ix !== anchor_ix) && + (direction !== range_direction); + var ix_delta = (direction === 'up') ? -1 : 1; + var new_ix = cursor_ix + ix_delta; + if (new_ix < 0 || new_ix >= this.ncells()) { + return false; + } + if (this.mode !== 'command') { + this.command_mode(); + } + this.get_cell(cursor_ix).unselect(!contracting); + this._select(new_ix); + return true; + }; + // Edit/Command mode From 92e266f4e286c9ae31b213167648aed5aaf07f72 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Sun, 9 Aug 2015 18:09:53 -0700 Subject: [PATCH 3/6] Merge cells based on range selection --- notebook/static/notebook/js/actions.js | 7 + .../static/notebook/js/keyboardmanager.js | 2 +- notebook/static/notebook/js/notebook.js | 120 ++++++++++-------- 3 files changed, 77 insertions(+), 52 deletions(-) diff --git a/notebook/static/notebook/js/actions.js b/notebook/static/notebook/js/actions.js index 5fe0db074..1601f6a16 100644 --- a/notebook/static/notebook/js/actions.js +++ b/notebook/static/notebook/js/actions.js @@ -310,6 +310,13 @@ define(function(require){ env.notebook.merge_cell_below(); } }, + 'merge-selected-cells' : { + help : 'merge selected cells', + help_index: 'el', + handler: function(env) { + env.notebook.merge_selected_cells(); + } + }, 'close-pager' : { help_index : 'gd', handler : function (env) { diff --git a/notebook/static/notebook/js/keyboardmanager.js b/notebook/static/notebook/js/keyboardmanager.js index 1a220d36c..f2ef6cd85 100644 --- a/notebook/static/notebook/js/keyboardmanager.js +++ b/notebook/static/notebook/js/keyboardmanager.js @@ -86,7 +86,7 @@ define([ return { 'shift-space': 'ipython.scroll-up', 'shift-v' : 'ipython.paste-cell-before', - 'shift-m' : 'ipython.merge-selected-cell-with-cell-after', + 'shift-m' : 'ipython.merge-selected-cells', 'shift-o' : 'ipython.toggle-output-scrolling-selected-cell', 'enter' : 'ipython.enter-edit-mode', 'space' : 'ipython.scroll-down', diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index 976c9f95b..9e7b36392 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -626,6 +626,20 @@ define(function (require) { }); }; + /** + * Get the indices of the currently selected range of cells. + * + * @return {Array} The selected cells' numeric indices + */ + Notebook.prototype.get_selected_indices = function () { + var result = []; + this.get_cell_elements().filter(function (index) { + if ($(this).data("cell").in_selection === true) { + result.push(index); + } + }); + return result; + }; // Cell selection. @@ -1343,37 +1357,66 @@ define(function (require) { } }; - /** - * Merge the selected cell into the cell above it. - */ - Notebook.prototype.merge_cell_above = function () { - var index = this.get_selected_index(); - var cell = this.get_cell(index); - var render = cell.rendered; - if (!cell.is_mergeable()) { + Notebook.prototype.merge_cells = function(indices, into_last) { + if (indices.length <= 1) { return; } - if (index > 0) { - var upper_cell = this.get_cell(index-1); - if (!upper_cell.is_mergeable()) { + for (var i=0; i < indices.length; i++) { + if (!this.get_cell(indices[i]).is_mergeable()) { return; } - var upper_text = upper_cell.get_text(); - var text = cell.get_text(); - if (cell instanceof codecell.CodeCell) { - cell.set_text(upper_text+'\n'+text); - } else { - cell.unrender(); // Must unrender before we set_text. - cell.set_text(upper_text+'\n\n'+text); - if (render) { - // The rendered state of the final cell should match - // that of the original selected cell; - cell.render(); - } + } + var target = this.get_cell(into_last ? indices.pop() : indices.shift()); + + // Get all the cells' contents + var contents = []; + for (i=0; i < indices.length; i++) { + contents.push(this.get_cell(indices[i]).get_text()); + } + if (into_last) { + contents.push(target.get_text()) + } else { + contents.unshift(target.get_text()) + } + + // Update the contents of the target cell + if (target instanceof codecell.CodeCell) { + target.set_text(contents.join('\n\n')) + } else { + var was_rendered = target.rendered; + target.unrender(); // Must unrender before we set_text. + target.set_text(contents.join('\n\n')); + if (was_rendered) { + // The rendered state of the final cell should match + // that of the original selected cell; + target.render(); } - this.delete_cell(index-1); - this.select(this.find_cell_index(cell)); } + + // Delete the other cells + // If we started deleting cells from the top, the later indices would + // get offset. We sort them into descending order to avoid that. + indices.sort(function(a, b) {return b-a;}); + for (i=0; i < indices.length; i++) { + this.delete_cell(indices[i]); + } + + this.select(this.find_cell_index(target)); + }; + + /** + * Merge the selected range of cells + */ + Notebook.prototype.merge_selected_cells = function() { + this.merge_cells(this.get_selected_indices()); + }; + + /** + * Merge the selected cell into the cell above it. + */ + Notebook.prototype.merge_cell_above = function () { + var index = this.get_selected_index(); + this.merge_cells([index-1, index], true) }; /** @@ -1381,32 +1424,7 @@ define(function (require) { */ Notebook.prototype.merge_cell_below = function () { var index = this.get_selected_index(); - var cell = this.get_cell(index); - var render = cell.rendered; - if (!cell.is_mergeable()) { - return; - } - if (index < this.ncells()-1) { - var lower_cell = this.get_cell(index+1); - if (!lower_cell.is_mergeable()) { - return; - } - var lower_text = lower_cell.get_text(); - var text = cell.get_text(); - if (cell instanceof codecell.CodeCell) { - cell.set_text(text+'\n'+lower_text); - } else { - cell.unrender(); // Must unrender before we set_text. - cell.set_text(text+'\n\n'+lower_text); - if (render) { - // The rendered state of the final cell should match - // that of the original selected cell; - cell.render(); - } - } - this.delete_cell(index+1); - this.select(this.find_cell_index(cell)); - } + this.merge_cells([index, index+1], false) }; From dcd676d499231d70d1f671976e75a7ab5fd5a614 Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Sun, 9 Aug 2015 18:19:48 -0700 Subject: [PATCH 4/6] Clear selected range on entering edit mode --- notebook/static/notebook/js/actions.js | 7 +++++++ notebook/static/notebook/js/notebook.js | 13 +++++++++++++ 2 files changed, 20 insertions(+) diff --git a/notebook/static/notebook/js/actions.js b/notebook/static/notebook/js/actions.js index 1601f6a16..664f50b5e 100644 --- a/notebook/static/notebook/js/actions.js +++ b/notebook/static/notebook/js/actions.js @@ -122,6 +122,13 @@ define(function(require){ } } }, + 'reset-selection': { + help: 'clear selected cells', + help_index: 'de', + handler: function(env) { + env.notebook.reset_selection(); + } + }, 'cut-selected-cell' : { icon: 'fa-cut', help_index : 'ee', diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index 9e7b36392..a6aea7856 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -725,6 +725,18 @@ define(function (require) { return true; }; + /** + * Clear selection of multiple cells (except the cell at the cursor) + */ + Notebook.prototype.reset_selection = function() { + var current_selection = this.get_selected_cells(); + for (var i=0; i Date: Sun, 9 Aug 2015 18:55:31 -0700 Subject: [PATCH 5/6] Fix cell merge tests --- notebook/tests/notebook/dualmode_merge.js | 3 ++- notebook/tests/notebook/merge_cells_api.js | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/notebook/tests/notebook/dualmode_merge.js b/notebook/tests/notebook/dualmode_merge.js index 8ec32405f..9c386884f 100644 --- a/notebook/tests/notebook/dualmode_merge.js +++ b/notebook/tests/notebook/dualmode_merge.js @@ -1,7 +1,7 @@ // Test casper.notebook_test(function () { - var a = 'ab\ncd'; + var a = 'ab\n\ncd'; var b = 'print("b")'; var c = 'print("c")'; @@ -41,6 +41,7 @@ casper.notebook_test(function () { this.test.assertEquals(this.get_cell_text(1), 'cd', 'split; Verify that cell 1 has the second half.'); this.validate_notebook_state('split', 'edit', 1); this.select_cell(0); // Move up to cell 0 + this.evaluate(function() { IPython.notebook.extend_selection('down');}); this.trigger_keydown('shift-m'); // Merge this.validate_notebook_state('merge', 'command', 0); this.test.assertEquals(this.get_cell_text(0), a, 'merge; Verify that cell 0 has the merged contents.'); diff --git a/notebook/tests/notebook/merge_cells_api.js b/notebook/tests/notebook/merge_cells_api.js index 9dd2fbdcb..665a9d1ef 100644 --- a/notebook/tests/notebook/merge_cells_api.js +++ b/notebook/tests/notebook/merge_cells_api.js @@ -36,8 +36,8 @@ casper.notebook_test(function() { return IPython.notebook.get_selected_cell().get_text(); }); - this.test.assertEquals(output_above, 'a = 5\nprint(a)', + this.test.assertEquals(output_above, 'a = 5\n\nprint(a)', 'Successful merge_cell_above().'); - this.test.assertEquals(output_below, 'a = 5\nprint(a)', + this.test.assertEquals(output_below, 'a = 5\n\nprint(a)', 'Successful merge_cell_below().'); }); From 0cde7269a509026bd473b7b27d88967ac39f60cb Mon Sep 17 00:00:00 2001 From: Thomas Kluyver Date: Mon, 10 Aug 2015 13:47:08 -0700 Subject: [PATCH 6/6] JSdoc for merge_cells method --- notebook/static/notebook/js/notebook.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/notebook/static/notebook/js/notebook.js b/notebook/static/notebook/js/notebook.js index a6aea7856..731131ad7 100644 --- a/notebook/static/notebook/js/notebook.js +++ b/notebook/static/notebook/js/notebook.js @@ -1370,6 +1370,12 @@ define(function (require) { } }; + /** + * Merge a series of cells into one + * + * @param {Array} indices - the numeric indices of the cells to be merged + * @param {bool} into_last - merge into the last cell instead of the first + */ Notebook.prototype.merge_cells = function(indices, into_last) { if (indices.length <= 1) { return;