From 154615a4cc843a492da1eafccfe43d95c3288355 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= <jodator@jodator.net>
Date: Fri, 3 Apr 2020 14:25:21 +0200
Subject: [PATCH 1/8] Move `removeRow` logic to table utils plugin.

---
 src/commands/removerowcommand.js | 49 +-------------------------------
 src/tableutils.js                | 47 ++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/src/commands/removerowcommand.js b/src/commands/removerowcommand.js
index 7e716a5a..9927bea1 100644
--- a/src/commands/removerowcommand.js
+++ b/src/commands/removerowcommand.js
@@ -9,7 +9,6 @@
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
 
-import TableWalker from '../tablewalker';
 import { findAncestor, updateNumericAttribute } from './utils';
 import { getRowIndexes, getSelectionAffectedTableCells } from '../utils';
 
@@ -69,7 +68,7 @@ export default class RemoveRowCommand extends Command {
 
 			for ( let i = removedRowIndexes.last; i >= removedRowIndexes.first; i-- ) {
 				const removedRowIndex = i;
-				this._removeRow( removedRowIndex, table, writer );
+				this.editor.plugins.get( 'TableUtils' ).removeRow( removedRowIndex, table, writer );
 
 				cellToFocus = getCellToFocus( table, removedRowIndex, columnIndexToFocus );
 			}
@@ -90,52 +89,6 @@ export default class RemoveRowCommand extends Command {
 			writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );
 		} );
 	}
-
-	/**
-	 * Removes a row from the given `table`.
-	 *
-	 * @private
-	 * @param {Number} removedRowIndex Index of the row that should be removed.
-	 * @param {module:engine/model/element~Element} table
-	 * @param {module:engine/model/writer~Writer} writer
-	 */
-	_removeRow( removedRowIndex, table, writer ) {
-		const cellsToMove = new Map();
-		const tableRow = table.getChild( removedRowIndex );
-		const tableMap = [ ...new TableWalker( table, { endRow: removedRowIndex } ) ];
-
-		// Get cells from removed row that are spanned over multiple rows.
-		tableMap
-			.filter( ( { row, rowspan } ) => row === removedRowIndex && rowspan > 1 )
-			.forEach( ( { column, cell, rowspan } ) => cellsToMove.set( column, { cell, rowspanToSet: rowspan - 1 } ) );
-
-		// Reduce rowspan on cells that are above removed row and overlaps removed row.
-		tableMap
-			.filter( ( { row, rowspan } ) => row <= removedRowIndex - 1 && row + rowspan > removedRowIndex )
-			.forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) );
-
-		// Move cells to another row.
-		const targetRow = removedRowIndex + 1;
-		const tableWalker = new TableWalker( table, { includeSpanned: true, startRow: targetRow, endRow: targetRow } );
-		let previousCell;
-
-		for ( const { row, column, cell } of [ ...tableWalker ] ) {
-			if ( cellsToMove.has( column ) ) {
-				const { cell: cellToMove, rowspanToSet } = cellsToMove.get( column );
-				const targetPosition = previousCell ?
-					writer.createPositionAfter( previousCell ) :
-					writer.createPositionAt( table.getChild( row ), 0 );
-				writer.move( writer.createRangeOn( cellToMove ), targetPosition );
-				updateNumericAttribute( 'rowspan', rowspanToSet, cellToMove, writer );
-				previousCell = cellToMove;
-			}
-			else {
-				previousCell = cell;
-			}
-		}
-
-		writer.remove( tableRow );
-	}
 }
 
 // Returns a cell that should be focused before removing the row, belonging to the same column as the currently focused cell.
diff --git a/src/tableutils.js b/src/tableutils.js
index 3da03b66..9417a709 100644
--- a/src/tableutils.js
+++ b/src/tableutils.js
@@ -253,6 +253,53 @@ export default class TableUtils extends Plugin {
 		} );
 	}
 
+	/**
+	 * Removes a row from the given `table`.
+	 *
+	 * This method properly re-calculate `rowspan` attribute of any table cell that is overlapping removed row.
+	 *
+	 * @private
+	 * @param {Number} rowIndex Index of the row that should be removed.
+	 * @param {module:engine/model/element~Element} table
+	 * @param {module:engine/model/writer~Writer} writer
+	 */
+	removeRow( rowIndex, table, writer ) {
+		const cellsToMove = new Map();
+		const tableRow = table.getChild( rowIndex );
+		const tableMap = [ ...new TableWalker( table, { endRow: rowIndex } ) ];
+
+		// Get cells from removed row that are spanned over multiple rows.
+		tableMap
+			.filter( ( { row, rowspan } ) => row === rowIndex && rowspan > 1 )
+			.forEach( ( { column, cell, rowspan } ) => cellsToMove.set( column, { cell, rowspanToSet: rowspan - 1 } ) );
+
+		// Reduce rowspan on cells that are above removed row and overlaps removed row.
+		tableMap
+			.filter( ( { row, rowspan } ) => row <= rowIndex - 1 && row + rowspan > rowIndex )
+			.forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) );
+
+		// Move cells to another row.
+		const targetRow = rowIndex + 1;
+		const tableWalker = new TableWalker( table, { includeSpanned: true, startRow: targetRow, endRow: targetRow } );
+		let previousCell;
+
+		for ( const { row, column, cell } of [ ...tableWalker ] ) {
+			if ( cellsToMove.has( column ) ) {
+				const { cell: cellToMove, rowspanToSet } = cellsToMove.get( column );
+				const targetPosition = previousCell ?
+					writer.createPositionAfter( previousCell ) :
+					writer.createPositionAt( table.getChild( row ), 0 );
+				writer.move( writer.createRangeOn( cellToMove ), targetPosition );
+				updateNumericAttribute( 'rowspan', rowspanToSet, cellToMove, writer );
+				previousCell = cellToMove;
+			} else {
+				previousCell = cell;
+			}
+		}
+
+		writer.remove( tableRow );
+	}
+
 	/**
 	 * Divides a table cell vertically into several ones.
 	 *

From 5e1e1ca0729ff7ea79416b6b10909d19cebe2951 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= <jodator@jodator.net>
Date: Fri, 3 Apr 2020 16:05:43 +0200
Subject: [PATCH 2/8] Move remove multiple rows logic to
 TableUtils#removeRows().

---
 src/commands/removerowcommand.js |  36 +-----
 src/tableutils.js                | 125 ++++++++++++------
 tests/tableutils.js              | 213 +++++++++++++++++++++++++++++++
 3 files changed, 309 insertions(+), 65 deletions(-)

diff --git a/src/commands/removerowcommand.js b/src/commands/removerowcommand.js
index 9927bea1..641d5b25 100644
--- a/src/commands/removerowcommand.js
+++ b/src/commands/removerowcommand.js
@@ -9,7 +9,7 @@
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
 
-import { findAncestor, updateNumericAttribute } from './utils';
+import { findAncestor } from './utils';
 import { getRowIndexes, getSelectionAffectedTableCells } from '../utils';
 
 /**
@@ -64,27 +64,14 @@ export default class RemoveRowCommand extends Command {
 			// This prevents the "model-selection-range-intersects" error, caused by removing row selected cells.
 			writer.setSelection( writer.createSelection( table, 'on' ) );
 
-			let cellToFocus;
+			const rowsToRemove = removedRowIndexes.last - removedRowIndexes.first + 1;
 
-			for ( let i = removedRowIndexes.last; i >= removedRowIndexes.first; i-- ) {
-				const removedRowIndex = i;
-				this.editor.plugins.get( 'TableUtils' ).removeRow( removedRowIndex, table, writer );
+			this.editor.plugins.get( 'TableUtils' ).removeRows( table, {
+				at: removedRowIndexes.first,
+				rows: rowsToRemove
+			} );
 
-				cellToFocus = getCellToFocus( table, removedRowIndex, columnIndexToFocus );
-			}
-
-			const model = this.editor.model;
-			const headingRows = table.getAttribute( 'headingRows' ) || 0;
-
-			if ( headingRows && removedRowIndexes.first < headingRows ) {
-				const newRows = getNewHeadingRowsValue( removedRowIndexes, headingRows );
-
-				// Must be done after the changes in table structure (removing rows).
-				// Otherwise the downcast converter for headingRows attribute will fail. ckeditor/ckeditor5#6391.
-				model.enqueueChange( writer.batch, writer => {
-					updateNumericAttribute( 'headingRows', newRows, table, writer, 0 );
-				} );
-			}
+			const cellToFocus = getCellToFocus( table, removedRowIndexes.first, columnIndexToFocus );
 
 			writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );
 		} );
@@ -112,12 +99,3 @@ function getCellToFocus( table, removedRowIndex, columnToFocus ) {
 
 	return cellToFocus;
 }
-
-// Calculates a new heading rows value for removing rows from heading section.
-function getNewHeadingRowsValue( removedRowIndexes, headingRows ) {
-	if ( removedRowIndexes.last < headingRows ) {
-		return headingRows - ( ( removedRowIndexes.last - removedRowIndexes.first ) + 1 );
-	}
-
-	return removedRowIndexes.first;
-}
diff --git a/src/tableutils.js b/src/tableutils.js
index 9417a709..db1da761 100644
--- a/src/tableutils.js
+++ b/src/tableutils.js
@@ -256,48 +256,55 @@ export default class TableUtils extends Plugin {
 	/**
 	 * Removes a row from the given `table`.
 	 *
-	 * This method properly re-calculate `rowspan` attribute of any table cell that is overlapping removed row.
+	 * This method properly re-calculate table geometry including `rowspan` attribute of any table cell that is overlapping removed row
+	 * and table headings values.
 	 *
+	 *		editor.plugins.get( 'TableUtils' ).removeRows( table, { at: 1, rows: 2 } );
+	 *
+	 * Assuming the table on the left, the above code will transform it to the table on the right:
+	 *
+	 *		row index
+	 *		    ┌───┬───┬───┐        `at` = 1,       ┌───┬───┬───┐
+	 *		  0 │ a │ b │ c │        `rows` = 2,     │ a │ b │ c │ 0
+	 *		    │   ├───┼───┤                        │   ├───┼───┤
+	 *		  1 │   │ d │ e │  <-- remove from here  │   │ i │ j │ 1
+	 *		    │   ├───┼───┤        will give:      ├───┼───┼───┤
+	 *		  2 │   │ g │ h │                        │ k │ l │ m │ 2
+	 *		    │   ├───┼───┤                        └───┴───┴───┘
+	 *		  3 │   │ i │ j │
+	 *		    ├───┼───┼───┤
+	 *		  4 │ k │ l │ m │
+	 *		    └───┴───┴───┘
 	 * @private
-	 * @param {Number} rowIndex Index of the row that should be removed.
 	 * @param {module:engine/model/element~Element} table
-	 * @param {module:engine/model/writer~Writer} writer
+	 * @param {Object} options
+	 * @param {Number} options.at The row index at which the removing rows will start.
+	 * @param {Number} [options.rows=1] The number of rows to remove.
 	 */
-	removeRow( rowIndex, table, writer ) {
-		const cellsToMove = new Map();
-		const tableRow = table.getChild( rowIndex );
-		const tableMap = [ ...new TableWalker( table, { endRow: rowIndex } ) ];
-
-		// Get cells from removed row that are spanned over multiple rows.
-		tableMap
-			.filter( ( { row, rowspan } ) => row === rowIndex && rowspan > 1 )
-			.forEach( ( { column, cell, rowspan } ) => cellsToMove.set( column, { cell, rowspanToSet: rowspan - 1 } ) );
-
-		// Reduce rowspan on cells that are above removed row and overlaps removed row.
-		tableMap
-			.filter( ( { row, rowspan } ) => row <= rowIndex - 1 && row + rowspan > rowIndex )
-			.forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) );
-
-		// Move cells to another row.
-		const targetRow = rowIndex + 1;
-		const tableWalker = new TableWalker( table, { includeSpanned: true, startRow: targetRow, endRow: targetRow } );
-		let previousCell;
-
-		for ( const { row, column, cell } of [ ...tableWalker ] ) {
-			if ( cellsToMove.has( column ) ) {
-				const { cell: cellToMove, rowspanToSet } = cellsToMove.get( column );
-				const targetPosition = previousCell ?
-					writer.createPositionAfter( previousCell ) :
-					writer.createPositionAt( table.getChild( row ), 0 );
-				writer.move( writer.createRangeOn( cellToMove ), targetPosition );
-				updateNumericAttribute( 'rowspan', rowspanToSet, cellToMove, writer );
-				previousCell = cellToMove;
-			} else {
-				previousCell = cell;
+	removeRows( table, options ) {
+		const model = this.editor.model;
+		const first = options.at;
+		const rowsToRemove = options.rows || 1;
+
+		const last = first + rowsToRemove - 1;
+
+		model.change( writer => {
+			for ( let i = last; i >= first; i-- ) {
+				removeRow( table, i, writer );
 			}
-		}
 
-		writer.remove( tableRow );
+			const headingRows = table.getAttribute( 'headingRows' ) || 0;
+
+			if ( headingRows && first < headingRows ) {
+				const newRows = getNewHeadingRowsValue( first, last, headingRows );
+
+				// Must be done after the changes in table structure (removing rows).
+				// Otherwise the downcast converter for headingRows attribute will fail. ckeditor/ckeditor5#6391.
+				model.enqueueChange( writer.batch, writer => {
+					updateNumericAttribute( 'headingRows', newRows, table, writer, 0 );
+				} );
+			}
+		} );
 	}
 
 	/**
@@ -662,3 +669,49 @@ function breakSpanEvenly( span, numberOfCells ) {
 
 	return { newCellsSpan, updatedSpan };
 }
+
+function removeRow( table, rowIndex, writer ) {
+	const cellsToMove = new Map();
+	const tableRow = table.getChild( rowIndex );
+	const tableMap = [ ...new TableWalker( table, { endRow: rowIndex } ) ];
+
+	// Get cells from removed row that are spanned over multiple rows.
+	tableMap
+		.filter( ( { row, rowspan } ) => row === rowIndex && rowspan > 1 )
+		.forEach( ( { column, cell, rowspan } ) => cellsToMove.set( column, { cell, rowspanToSet: rowspan - 1 } ) );
+
+	// Reduce rowspan on cells that are above removed row and overlaps removed row.
+	tableMap
+		.filter( ( { row, rowspan } ) => row <= rowIndex - 1 && row + rowspan > rowIndex )
+		.forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) );
+
+	// Move cells to another row.
+	const targetRow = rowIndex + 1;
+	const tableWalker = new TableWalker( table, { includeSpanned: true, startRow: targetRow, endRow: targetRow } );
+	let previousCell;
+
+	for ( const { row, column, cell } of [ ...tableWalker ] ) {
+		if ( cellsToMove.has( column ) ) {
+			const { cell: cellToMove, rowspanToSet } = cellsToMove.get( column );
+			const targetPosition = previousCell ?
+				writer.createPositionAfter( previousCell ) :
+				writer.createPositionAt( table.getChild( row ), 0 );
+			writer.move( writer.createRangeOn( cellToMove ), targetPosition );
+			updateNumericAttribute( 'rowspan', rowspanToSet, cellToMove, writer );
+			previousCell = cellToMove;
+		} else {
+			previousCell = cell;
+		}
+	}
+
+	writer.remove( tableRow );
+}
+
+// Calculates a new heading rows value for removing rows from heading section.
+function getNewHeadingRowsValue( first, last, headingRows ) {
+	if ( last < headingRows ) {
+		return headingRows - ( last - first + 1 );
+	}
+
+	return first;
+}
diff --git a/tests/tableutils.js b/tests/tableutils.js
index a9f1199c..8412907b 100644
--- a/tests/tableutils.js
+++ b/tests/tableutils.js
@@ -747,4 +747,217 @@ describe( 'TableUtils', () => {
 			expect( tableUtils.getRows( root.getNodeByPath( [ 0 ] ) ) ).to.equal( 3 );
 		} );
 	} );
+
+	describe( 'removeRow()', () => {
+		describe( 'single row', () => {
+			it( 'should remove a given row from a table start', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				] ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '10', '11' ],
+					[ '20', '21' ]
+				] ) );
+			} );
+
+			it( 'should remove last row', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ]
+				] ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 1 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '00', '01' ]
+				] ) );
+			} );
+
+			it( 'should change heading rows if removing a heading row', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 2 } ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 1 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '00', '01' ],
+					[ '20', '21' ]
+				], { headingRows: 1 } ) );
+			} );
+
+			it( 'should decrease rowspan of table cells from previous rows', () => {
+				setData( model, modelTable( [
+					[ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ],
+					[ { rowspan: 2, contents: '13' }, '14' ],
+					[ '22', '23', '24' ],
+					[ '30', '31', '32', '33', '34' ]
+				] ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 2 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ],
+					[ '13', '14' ],
+					[ '30', '31', '32', '33', '34' ]
+				] ) );
+			} );
+
+			it( 'should move rowspaned cells to row below removing it\'s row', () => {
+				setData( model, modelTable( [
+					[ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, '02' ],
+					[ '12' ],
+					[ '22' ],
+					[ '30', '31', '32' ]
+				] ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ { rowspan: 2, contents: '00' }, '01', '12' ],
+					[ '22' ],
+					[ '30', '31', '32' ]
+				] ) );
+			} );
+		} );
+
+		describe( 'many rows', () => {
+			it( 'should properly remove middle rows', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 1, rows: 2 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '00', '01' ],
+					[ '30', '31' ]
+				] ) );
+			} );
+
+			it( 'should properly remove tailing rows', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 2, rows: 2 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ]
+				] ) );
+			} );
+
+			it( 'should properly remove beginning rows', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+			} );
+
+			it( 'should support removing multiple headings (removed rows in heading section)', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				], { headingRows: 3 } ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '20', '21' ],
+					[ '30', '31' ]
+				], { headingRows: 1 } ) );
+			} );
+
+			it( 'should support removing multiple headings (removed rows in heading and body section)', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ],
+					[ '40', '41' ]
+				], { headingRows: 3 } ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 1, rows: 3 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '00', '01' ],
+					[ '40', '41' ]
+				], { headingRows: 1 } ) );
+			} );
+
+			it( 'should support removing mixed heading and cell rows', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 1 } ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '20', '21' ]
+				] ) );
+			} );
+
+			it( 'should properly calculate truncated rowspans', () => {
+				setData( model, modelTable( [
+					[ '00', { contents: '01', rowspan: 3 } ],
+					[ '10' ],
+					[ '20' ]
+				] ) );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+
+				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
+					[ '20', '01' ]
+				] ) );
+			} );
+
+			it( 'should create one undo step (1 batch)', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				], { headingRows: 3 } ) );
+
+				const createdBatches = new Set();
+
+				model.on( 'applyOperation', ( evt, args ) => {
+					const operation = args[ 0 ];
+
+					createdBatches.add( operation.batch );
+				} );
+
+				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+
+				expect( createdBatches.size ).to.equal( 1 );
+			} );
+		} );
+	} );
 } );

From c7d0eddf09adda8b3e03788253cec27100f8ae64 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= <jodator@jodator.net>
Date: Fri, 3 Apr 2020 16:07:12 +0200
Subject: [PATCH 3/8] Code style fix.

---
 src/tableutils.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/tableutils.js b/src/tableutils.js
index db1da761..e98bbfd3 100644
--- a/src/tableutils.js
+++ b/src/tableutils.js
@@ -264,8 +264,8 @@ export default class TableUtils extends Plugin {
 	 * Assuming the table on the left, the above code will transform it to the table on the right:
 	 *
 	 *		row index
-	 *		    ┌───┬───┬───┐        `at` = 1,       ┌───┬───┬───┐
-	 *		  0 │ a │ b │ c │        `rows` = 2,     │ a │ b │ c │ 0
+	 *		    ┌───┬───┬───┐        `at` = 1        ┌───┬───┬───┐
+	 *		  0 │ a │ b │ c │        `rows` = 2      │ a │ b │ c │ 0
 	 *		    │   ├───┼───┤                        │   ├───┼───┤
 	 *		  1 │   │ d │ e │  <-- remove from here  │   │ i │ j │ 1
 	 *		    │   ├───┼───┤        will give:      ├───┼───┼───┤

From 48ee10eda4757f94b1783ffe0e93a5a69cbd9547 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= <jodator@jodator.net>
Date: Fri, 3 Apr 2020 16:11:03 +0200
Subject: [PATCH 4/8] Remove @private annotation from TableUtils#removeRows().

---
 src/tableutils.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tableutils.js b/src/tableutils.js
index e98bbfd3..69bd7a22 100644
--- a/src/tableutils.js
+++ b/src/tableutils.js
@@ -275,7 +275,7 @@ export default class TableUtils extends Plugin {
 	 *		    ├───┼───┼───┤
 	 *		  4 │ k │ l │ m │
 	 *		    └───┴───┴───┘
-	 * @private
+	 *
 	 * @param {module:engine/model/element~Element} table
 	 * @param {Object} options
 	 * @param {Number} options.at The row index at which the removing rows will start.

From 6a6c926368d7a62df125805afe4dd64095ce3abd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= <jodator@jodator.net>
Date: Tue, 7 Apr 2020 12:01:50 +0200
Subject: [PATCH 5/8] Use getChild instead getNodeByPath to get table in tests.

---
 tests/tableutils.js | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/tests/tableutils.js b/tests/tableutils.js
index 8412907b..92af9891 100644
--- a/tests/tableutils.js
+++ b/tests/tableutils.js
@@ -748,7 +748,7 @@ describe( 'TableUtils', () => {
 		} );
 	} );
 
-	describe( 'removeRow()', () => {
+	describe( 'removeRows()', () => {
 		describe( 'single row', () => {
 			it( 'should remove a given row from a table start', () => {
 				setData( model, modelTable( [
@@ -757,7 +757,7 @@ describe( 'TableUtils', () => {
 					[ '20', '21' ]
 				] ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 0 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '10', '11' ],
@@ -771,7 +771,7 @@ describe( 'TableUtils', () => {
 					[ '10', '11' ]
 				] ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 1 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 1 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '00', '01' ]
@@ -785,7 +785,7 @@ describe( 'TableUtils', () => {
 					[ '20', '21' ]
 				], { headingRows: 2 } ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 1 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 1 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '00', '01' ],
@@ -801,7 +801,7 @@ describe( 'TableUtils', () => {
 					[ '30', '31', '32', '33', '34' ]
 				] ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 2 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 2 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ],
@@ -818,7 +818,7 @@ describe( 'TableUtils', () => {
 					[ '30', '31', '32' ]
 				] ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 0 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ { rowspan: 2, contents: '00' }, '01', '12' ],
@@ -837,7 +837,7 @@ describe( 'TableUtils', () => {
 					[ '30', '31' ]
 				] ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 1, rows: 2 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 1, rows: 2 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '00', '01' ],
@@ -853,7 +853,7 @@ describe( 'TableUtils', () => {
 					[ '30', '31' ]
 				] ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 2, rows: 2 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 2, rows: 2 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '00', '01' ],
@@ -869,7 +869,7 @@ describe( 'TableUtils', () => {
 					[ '30', '31' ]
 				] ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '20', '21' ],
@@ -885,7 +885,7 @@ describe( 'TableUtils', () => {
 					[ '30', '31' ]
 				], { headingRows: 3 } ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '20', '21' ],
@@ -902,7 +902,7 @@ describe( 'TableUtils', () => {
 					[ '40', '41' ]
 				], { headingRows: 3 } ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 1, rows: 3 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 1, rows: 3 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '00', '01' ],
@@ -917,7 +917,7 @@ describe( 'TableUtils', () => {
 					[ '20', '21' ]
 				], { headingRows: 1 } ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '20', '21' ]
@@ -931,7 +931,7 @@ describe( 'TableUtils', () => {
 					[ '20' ]
 				] ) );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } );
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ '20', '01' ]
@@ -954,7 +954,7 @@ describe( 'TableUtils', () => {
 					createdBatches.add( operation.batch );
 				} );
 
-				tableUtils.removeRows( root.getNodeByPath( [ 0 ] ), { at: 0, rows: 2 } );
+				tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } );
 
 				expect( createdBatches.size ).to.equal( 1 );
 			} );

From b9f5a924ccfa5f9bd48b11f9e8c5773201f29034 Mon Sep 17 00:00:00 2001
From: Maciej <jodator@jodator.net>
Date: Tue, 7 Apr 2020 13:15:33 +0200
Subject: [PATCH 6/8] Apply suggestions from code review.

Co-Authored-By: Aleksander Nowodzinski <a.nowodzinski@cksource.com>
---
 src/tableutils.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/tableutils.js b/src/tableutils.js
index a13eb091..3fd7ba18 100644
--- a/src/tableutils.js
+++ b/src/tableutils.js
@@ -254,14 +254,14 @@ export default class TableUtils extends Plugin {
 	}
 
 	/**
-	 * Removes a row from the given `table`.
+	 * Removes rows from the given `table`.
 	 *
-	 * This method properly re-calculate table geometry including `rowspan` attribute of any table cell that is overlapping removed row
+	 * This method re-calculates the table geometry including `rowspan` attribute of table cells overlapping removed rows
 	 * and table headings values.
 	 *
 	 *		editor.plugins.get( 'TableUtils' ).removeRows( table, { at: 1, rows: 2 } );
 	 *
-	 * Assuming the table on the left, the above code will transform it to the table on the right:
+	 * Executing the above code in the context of the table on the left will transform its structure as presented on the right:
 	 *
 	 *		row index
 	 *		    ┌───┬───┬───┐        `at` = 1        ┌───┬───┬───┐

From 6e350531381dd961c31bfbac14521a812ca50953 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= <jodator@jodator.net>
Date: Tue, 7 Apr 2020 13:46:02 +0200
Subject: [PATCH 7/8] Fix TableUtils.removeRows() sample.

---
 src/tableutils.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/tableutils.js b/src/tableutils.js
index 3fd7ba18..efe541c6 100644
--- a/src/tableutils.js
+++ b/src/tableutils.js
@@ -267,13 +267,13 @@ export default class TableUtils extends Plugin {
 	 *		    ┌───┬───┬───┐        `at` = 1        ┌───┬───┬───┐
 	 *		  0 │ a │ b │ c │        `rows` = 2      │ a │ b │ c │ 0
 	 *		    │   ├───┼───┤                        │   ├───┼───┤
-	 *		  1 │   │ d │ e │  <-- remove from here  │   │ i │ j │ 1
+	 *		  1 │   │ d │ e │  <-- remove from here  │   │ h │ i │ 1
 	 *		    │   ├───┼───┤        will give:      ├───┼───┼───┤
-	 *		  2 │   │ g │ h │                        │ k │ l │ m │ 2
+	 *		  2 │   │ f │ g │                        │ j │ k │ l │ 2
 	 *		    │   ├───┼───┤                        └───┴───┴───┘
-	 *		  3 │   │ i │ j │
+	 *		  3 │   │ h │ i │
 	 *		    ├───┼───┼───┤
-	 *		  4 │ k │ l │ m │
+	 *		  4 │ j │ k │ l │
 	 *		    └───┴───┴───┘
 	 *
 	 * @param {module:engine/model/element~Element} table

From fe3c1df72811e4543244344f998c02d0d795335d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= <jodator@jodator.net>
Date: Tue, 7 Apr 2020 13:49:39 +0200
Subject: [PATCH 8/8] Fix "should move rowspaned cells to row below removing
 it's row" test cases.

---
 tests/commands/removerowcommand.js | 4 ++--
 tests/tableutils.js                | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/commands/removerowcommand.js b/tests/commands/removerowcommand.js
index fde382ed..21ccb315 100644
--- a/tests/commands/removerowcommand.js
+++ b/tests/commands/removerowcommand.js
@@ -464,7 +464,7 @@ describe( 'RemoveRowCommand', () => {
 			setData( model, modelTable( [
 				[ { rowspan: 3, contents: '[]00' }, { rowspan: 2, contents: '01' }, '02' ],
 				[ '12' ],
-				[ '22' ],
+				[ '21', '22' ],
 				[ '30', '31', '32' ]
 			] ) );
 
@@ -472,7 +472,7 @@ describe( 'RemoveRowCommand', () => {
 
 			assertEqualMarkup( getData( model ), modelTable( [
 				[ { rowspan: 2, contents: '[]00' }, '01', '12' ],
-				[ '22' ],
+				[ '21', '22' ],
 				[ '30', '31', '32' ]
 			] ) );
 		} );
diff --git a/tests/tableutils.js b/tests/tableutils.js
index 92af9891..ab229677 100644
--- a/tests/tableutils.js
+++ b/tests/tableutils.js
@@ -814,7 +814,7 @@ describe( 'TableUtils', () => {
 				setData( model, modelTable( [
 					[ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, '02' ],
 					[ '12' ],
-					[ '22' ],
+					[ '21', '22' ],
 					[ '30', '31', '32' ]
 				] ) );
 
@@ -822,7 +822,7 @@ describe( 'TableUtils', () => {
 
 				assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [
 					[ { rowspan: 2, contents: '00' }, '01', '12' ],
-					[ '22' ],
+					[ '21', '22' ],
 					[ '30', '31', '32' ]
 				] ) );
 			} );