From 1112c72384838d72e2618d0389bb1d179e5515f4 Mon Sep 17 00:00:00 2001 From: DeeDeeG Date: Tue, 25 Jun 2024 15:04:47 -0400 Subject: [PATCH] Revert "Merge pull request #810 from pulsar-edit/fix-on-change-cursor-pos" This reverts commit 8c6bef4d071dc22c65365aec24b5f24a5f213b6c, reversing changes made to 4dd32931dc95cc931d09e5bbdd5debfdf5789c5e. --- spec/text-editor-spec.js | 111 ++++------------------ src/cursor.js | 48 +--------- src/selection.js | 200 ++++++++++++++------------------------- 3 files changed, 93 insertions(+), 266 deletions(-) diff --git a/spec/text-editor-spec.js b/spec/text-editor-spec.js index d886c88f0..832c482a5 100644 --- a/spec/text-editor-spec.js +++ b/spec/text-editor-spec.js @@ -353,104 +353,27 @@ describe('TextEditor', () => { expect(editor.getCursorBufferPosition()).toEqual([1, 1]); }); - describe('listening to cursor movements', () => { - it('emits an event with the old position, new position, and the cursor that moved', () => { - const cursorCallback = jasmine.createSpy('cursor-changed-position'); - const editorCallback = jasmine.createSpy( - 'editor-changed-cursor-position' - ); + it('emits an event with the old position, new position, and the cursor that moved', () => { + const cursorCallback = jasmine.createSpy('cursor-changed-position'); + const editorCallback = jasmine.createSpy( + 'editor-changed-cursor-position' + ); - editor.getLastCursor().onDidChangePosition(cursorCallback); - editor.onDidChangeCursorPosition(editorCallback); + editor.getLastCursor().onDidChangePosition(cursorCallback); + editor.onDidChangeCursorPosition(editorCallback); - editor.setCursorBufferPosition([2, 4]); + editor.setCursorBufferPosition([2, 4]); - expect(editorCallback).toHaveBeenCalled(); - expect(cursorCallback).toHaveBeenCalled(); - const eventObject = editorCallback.mostRecentCall.args[0]; - expect(cursorCallback.mostRecentCall.args[0]).toEqual(eventObject); + expect(editorCallback).toHaveBeenCalled(); + expect(cursorCallback).toHaveBeenCalled(); + const eventObject = editorCallback.mostRecentCall.args[0]; + expect(cursorCallback.mostRecentCall.args[0]).toEqual(eventObject); - expect(eventObject.oldBufferPosition).toEqual([0, 0]); - expect(eventObject.oldScreenPosition).toEqual([0, 0]); - expect(eventObject.newBufferPosition).toEqual([2, 4]); - expect(eventObject.newScreenPosition).toEqual([2, 4]); - expect(eventObject.cursor).toBe(editor.getLastCursor()); - }); - - it('emits the event with textChanged: true if text was edited', () => { - let callbacks = [] - let callback = evt => { callbacks.push(evt.textChanged) } - editor.getLastCursor().onDidChangePosition(callback); - editor.onDidChangeCursorPosition(callback); - - editor.insertText('bar') - expect(callbacks).toEqual([true, true]) - - const cmds = [ - 'backspace', - 'deleteToBeginningOfWord', - 'deleteToBeginningOfSubword', - 'deleteToPreviousWordBoundary', - 'deleteToBeginningOfLine' - ] - cmds.forEach(command => { - editor.setText("HelloWorld!") - editor.setCursorBufferPosition([0, 5]) - callbacks = [] - editor[command].bind(editor)(); - expect(callbacks).toEqual([true, true], `on command ${command}`) - }) - }); - - it('emits the event with textChanged: true if whole lines were changed', () => { - let callbacks = []; - let callback = evt => { callbacks.push(evt.textChanged) }; - editor.getLastCursor().onDidChangePosition(callback); - editor.onDidChangeCursorPosition(callback); - - editor.setText("HelloWorld!\nGoodbye, world"); - editor.setCursorBufferPosition([0, 5]); - // TODO: Ideally, we want this event to not be called. Unfortunately, - // the world doesn't work like that - there's no way to delete a line - // without changing the current cursor position on TextBuffer. - callbacks = []; - editor.deleteLine(); - // One for the change, and another to reposition the cursor - expect(callbacks).toEqual([true, true, false, false], "on command deleteLine") - - editor.setText("HelloWorld!\nGoodbye, world"); - editor.setCursorBufferPosition([0, 5]); - callbacks = []; - editor.joinLines(); - // TODO: Again, not ideal. But still... - // One for moving to the line that will be deleted, one for the actual change - // and one to move to the "join position" between the lines - expect(callbacks).toEqual([false, false, true, true, false, false], "on command joinLines") - }); - - it("doesn't emit the event if you deleted something forward", () => { - let callbacks = [] - let callback = evt => { - callbacks.push(evt.textChanged) - } - editor.getLastCursor().onDidChangePosition(callback); - editor.onDidChangeCursorPosition(callback); - - const cmds = [ - 'delete', - 'deleteToEndOfSubword', - 'deleteToEndOfWord', - 'deleteToEndOfLine', - 'deleteToNextWordBoundary' - ] - cmds.forEach(command => { - editor.setText("HelloWorld!") - editor.setCursorBufferPosition([0, 5]) - callbacks = [] - editor[command].bind(editor)(); - expect(callbacks).toEqual([], `on command ${command}`) - }) - }); + expect(eventObject.oldBufferPosition).toEqual([0, 0]); + expect(eventObject.oldScreenPosition).toEqual([0, 0]); + expect(eventObject.newBufferPosition).toEqual([2, 4]); + expect(eventObject.newScreenPosition).toEqual([2, 4]); + expect(eventObject.cursor).toBe(editor.getLastCursor()); }); }); diff --git a/src/cursor.js b/src/cursor.js index 70a5b5057..334c5b0d8 100644 --- a/src/cursor.js +++ b/src/cursor.js @@ -300,25 +300,6 @@ module.exports = class Cursor extends Model { const range = this.marker.getScreenRange(); if (moveToEndOfSelection && !range.isEmpty()) { this.setScreenPosition(range.start); - } else { - const point = this.getPreviousColumnScreenPosition( - columnCount, { moveToEndOfSelection } - ) - this.setScreenPosition(point, { clipDirection: 'backward' }); - } - } - - // Public: Retrieves the screen position of where the previous column starts. - // * `columnCount` (optional) {Number} number of columns to move (default: 1) - // * `options` (optional) {Object} with the following keys: - // * `moveToEndOfSelection` if true, move to the right of the selection if a - // selection exists. - // - // Returns a {Point}. - getPreviousColumnScreenPosition(columnCount = 1, { moveToEndOfSelection } = {}) { - const range = this.marker.getScreenRange(); - if (moveToEndOfSelection && !range.isEmpty()) { - return range.start; } else { let { row, column } = this.getScreenPosition(); @@ -329,7 +310,7 @@ module.exports = class Cursor extends Model { } column = column - columnCount; - return new Point( row, column ) + this.setScreenPosition({ row, column }, { clipDirection: 'backward' }); } } @@ -343,25 +324,6 @@ module.exports = class Cursor extends Model { const range = this.marker.getScreenRange(); if (moveToEndOfSelection && !range.isEmpty()) { this.setScreenPosition(range.end); - } else { - const point = this.getNextColumnScreenPosition( - columnCount, { moveToEndOfSelection } - ) - this.setScreenPosition(point, { clipDirection: 'forward' }); - } - } - - // Public: Retrieves the screen position of where the next column starts. - // * `columnCount` (optional) {Number} number of columns to move (default: 1) - // * `options` (optional) {Object} with the following keys: - // * `moveToEndOfSelection` if true, move to the right of the selection if a - // selection exists. - // - // Returns a {Point}. - getNextColumnScreenPosition(columnCount = 1, { moveToEndOfSelection } = {}) { - const range = this.marker.getScreenRange(); - if (moveToEndOfSelection && !range.isEmpty()) { - return range.end; } else { let { row, column } = this.getScreenPosition(); const maxLines = this.editor.getScreenLineCount(); @@ -378,7 +340,7 @@ module.exports = class Cursor extends Model { } column = column + columnCount; - return new Point(row, column); + this.setScreenPosition({ row, column }, { clipDirection: 'forward' }); } } @@ -606,7 +568,7 @@ module.exports = class Cursor extends Model { // * `allowPrevious` A {Boolean} indicating whether the beginning of the // previous word can be returned. // - // Returns a {Point}. + // Returns a {Range}. getBeginningOfCurrentWordBufferPosition(options = {}) { const allowPrevious = options.allowPrevious !== false; const position = this.getBufferPosition(); @@ -667,7 +629,7 @@ module.exports = class Cursor extends Model { // * `wordRegex` A {RegExp} indicating what constitutes a "word" // (default: {::wordRegExp}). // - // Returns a {Point} + // Returns a {Range} getBeginningOfNextWordBufferPosition(options = {}) { const currentBufferPosition = this.getBufferPosition(); const start = this.isInsideWord(options) @@ -712,8 +674,6 @@ module.exports = class Cursor extends Model { // * `options` (optional) {Object} // * `includeNewline` A {Boolean} which controls whether the Range should // include the newline. - // - // Returns a {Range}. getCurrentLineBufferRange(options) { return this.editor.bufferRangeForBufferRow(this.getBufferRow(), options); } diff --git a/src/selection.js b/src/selection.js index 64a2fbdb8..8b1540272 100644 --- a/src/selection.js +++ b/src/selection.js @@ -593,12 +593,8 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) backspace(options = {}) { if (!this.ensureWritable('backspace', options)) return; - - const screenPosition = this.cursor.getPreviousColumnScreenPosition(); - const bufferPosition = this.cursor.marker.layer.translateScreenPosition( - screenPosition, { clipDirection: 'backward' } - ); - this._deleteToPreviousPoint(bufferPosition, options); + if (this.isEmpty()) this.selectLeft(); + this.deleteSelectedText(options); } // Public: Removes the selection or, if nothing is selected, then all @@ -609,10 +605,8 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) deleteToPreviousWordBoundary(options = {}) { if (!this.ensureWritable('deleteToPreviousWordBoundary', options)) return; - this._deleteToPreviousPoint( - this.cursor.getPreviousWordBoundaryBufferPosition(), - options - ); + if (this.isEmpty()) this.selectToPreviousWordBoundary(); + this.deleteSelectedText(options); } // Public: Removes the selection or, if nothing is selected, then all @@ -623,8 +617,8 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) deleteToNextWordBoundary(options = {}) { if (!this.ensureWritable('deleteToNextWordBoundary', options)) return; - const position = this.cursor.getNextWordBoundaryBufferPosition(); - this._deleteToNextPoint(position, options); + if (this.isEmpty()) this.selectToNextWordBoundary(); + this.deleteSelectedText(options); } // Public: Removes from the start of the selection to the beginning of the @@ -634,10 +628,8 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) deleteToBeginningOfWord(options = {}) { if (!this.ensureWritable('deleteToBeginningOfWord', options)) return; - this._deleteToPreviousPoint( - this.cursor.getBeginningOfCurrentWordBufferPosition(), - options - ); + if (this.isEmpty()) this.selectToBeginningOfWord(); + this.deleteSelectedText(options); } // Public: Removes from the beginning of the line which the selection begins on @@ -647,22 +639,12 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) deleteToBeginningOfLine(options = {}) { if (!this.ensureWritable('deleteToBeginningOfLine', options)) return; - const startPoint = this.getBufferRange().start; if (this.isEmpty() && this.cursor.isAtBeginningOfLine()) { - const lastCharPoint = this.cursor.marker.layer.translateScreenPosition( - this.cursor.getPreviousColumnScreenPosition() - ); - const prevRange = new Range( startPoint, lastCharPoint ); - this.editor.buffer.setTextInRange( - prevRange, '', pick(options, 'undo', 'normalizeLineEndings') - ); + this.selectLeft(); } else { - const startOfLinePoint = new Point(this.cursor.getScreenRow(), 0); - const prevRange = new Range(startPoint, startOfLinePoint); - this.editor.buffer.setTextInRange( - prevRange, '', pick(options, 'undo', 'normalizeLineEndings') - ); + this.selectToBeginningOfLine(); } + this.deleteSelectedText(options); } // Public: Removes the selection or the next character after the start of the @@ -672,11 +654,8 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) delete(options = {}) { if (!this.ensureWritable('delete', options)) return; - const screenPosition = this.cursor.getNextColumnScreenPosition(); - const bufferPosition = this.cursor.marker.layer.translateScreenPosition( - screenPosition, { clipDirection: 'forward' } - ); - this._deleteToNextPoint(bufferPosition, options); + if (this.isEmpty()) this.selectRight(); + this.deleteSelectedText(options); } // Public: If the selection is empty, removes all text from the cursor to the @@ -688,14 +667,14 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) deleteToEndOfLine(options = {}) { if (!this.ensureWritable('deleteToEndOfLine', options)) return; - if (this.isEmpty() && this.cursor.isAtEndOfLine()) { - this.delete(options); - } else { - this._deleteToNextPoint( - new Point(this.cursor.getScreenRow(), Infinity), - options - ); + if (this.isEmpty()) { + if (this.cursor.isAtEndOfLine()) { + this.delete(options); + return; + } + this.selectToEndOfLine(); } + this.deleteSelectedText(options); } // Public: Removes the selection or all characters from the start of the @@ -705,8 +684,8 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) deleteToEndOfWord(options = {}) { if (!this.ensureWritable('deleteToEndOfWord', options)) return; - const position = this.cursor.getEndOfCurrentWordBufferPosition(); - this._deleteToNextPoint(position, options); + if (this.isEmpty()) this.selectToEndOfWord(); + this.deleteSelectedText(options); } // Public: Removes the selection or all characters from the start of the @@ -716,10 +695,8 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) deleteToBeginningOfSubword(options = {}) { if (!this.ensureWritable('deleteToBeginningOfSubword', options)) return; - const position = this.cursor.getPreviousWordBoundaryBufferPosition({ - wordRegex: this.cursor.subwordRegExp({ backwards: true }) - }); - this._deleteToPreviousPoint(position, options); + if (this.isEmpty()) this.selectToPreviousSubwordBoundary(); + this.deleteSelectedText(options); } // Public: Removes the selection or all characters from the start of the @@ -729,10 +706,8 @@ module.exports = class Selection { // * `bypassReadOnly` (optional) {Boolean} Must be `true` to modify text within a read-only editor. (default: false) deleteToEndOfSubword(options = {}) { if (!this.ensureWritable('deleteToEndOfSubword', options)) return; - const position = this.cursor.getNextWordBoundaryBufferPosition( - { wordRegex: this.cursor.subwordRegExp() } - ); - this._deleteToNextPoint(position, options); + if (this.isEmpty()) this.selectToNextSubwordBoundary(); + this.deleteSelectedText(options); } // Public: Removes only the selected text. @@ -746,28 +721,6 @@ module.exports = class Selection { if (this.cursor) this.cursor.setBufferPosition(bufferRange.start); } - _deleteToPreviousPoint(point, options) { - if (this.isEmpty()) { - const prevRange = new Range(this.getBufferRange().start, point); - this.editor.buffer.setTextInRange( - prevRange, '', pick(options, 'undo', 'normalizeLineEndings') - ); - } else { - this.deleteSelectedText(options); - } - } - - _deleteToNextPoint(point, options) { - if (this.isEmpty()) { - const nextRange = new Range(point, this.getBufferRange().end); - this.editor.buffer.setTextInRange( - nextRange, '', pick(options, 'undo', 'normalizeLineEndings') - ); - } else { - this.deleteSelectedText(options); - } - } - // Public: Removes the line at the beginning of the selection if the selection // is empty unless the selection spans multiple lines in which case all lines // are removed. @@ -777,23 +730,21 @@ module.exports = class Selection { deleteLine(options = {}) { if (!this.ensureWritable('deleteLine', options)) return; const range = this.getBufferRange(); - this.editor.transact(() => { - if (range.isEmpty()) { - const start = this.cursor.getScreenRow(); - const range = this.editor.bufferRowsForScreenRows(start, start + 1); - if (range[1] > range[0]) { - this.editor.buffer.deleteRows(range[0], range[1] - 1); - } else { - this.editor.buffer.deleteRow(range[0]); - } + if (range.isEmpty()) { + const start = this.cursor.getScreenRow(); + const range = this.editor.bufferRowsForScreenRows(start, start + 1); + if (range[1] > range[0]) { + this.editor.buffer.deleteRows(range[0], range[1] - 1); } else { - const start = range.start.row; - let end = range.end.row; - if (end !== this.editor.buffer.getLastRow() && range.end.column === 0) - end--; - this.editor.buffer.deleteRows(start, end); + this.editor.buffer.deleteRow(range[0]); } - }) + } else { + const start = range.start.row; + let end = range.end.row; + if (end !== this.editor.buffer.getLastRow() && range.end.column === 0) + end--; + this.editor.buffer.deleteRows(start, end); + } this.cursor.setBufferPosition({ row: this.cursor.getBufferRow(), column: range.start.column @@ -810,10 +761,9 @@ module.exports = class Selection { joinLines(options = {}) { if (!this.ensureWritable('joinLines', options)) return; let joinMarker; - const buffer = this.editor.getBuffer(); const selectedRange = this.getBufferRange(); if (selectedRange.isEmpty()) { - if (selectedRange.start.row === buffer.getLastRow()) return; + if (selectedRange.start.row === this.editor.buffer.getLastRow()) return; } else { joinMarker = this.editor.markBufferRange(selectedRange, { invalidate: 'never' @@ -821,51 +771,45 @@ module.exports = class Selection { } const rowCount = Math.max(1, selectedRange.getRowCount() - 1); - let cursorPositionAfterEdit = null; - buffer.transact(() => { - for (let i = 0; i < rowCount; i++) { - // Remove trailing whitespace from the current line - const scanRange = this.cursor.getCurrentLineBufferRange(); - this.editor.scanInBufferRange(/[ \t]+$/, scanRange, ({ range }) => { - buffer.setTextInRange(range, ''); - }); - const currentRow = selectedRange.start.row; - const nextRow = currentRow + 1; - const haveNextLine = nextRow <= buffer.getLastRow(); + for (let i = 0; i < rowCount; i++) { + this.cursor.setBufferPosition([selectedRange.start.row]); + this.cursor.moveToEndOfLine(); - if(haveNextLine) { - // Remove leading whitespace from the line below - const nextLineRange = this.editor.bufferRangeForBufferRow(nextRow); - this.editor.scanInBufferRange(/^[ \t]+/, nextLineRange, ({ range }) => { - buffer.setTextInRange(range, ''); - }); - - const nextLineIsntEmpty = buffer.lineLengthForRow(nextRow) > 0; - const insertSpace = nextLineIsntEmpty && buffer.lineLengthForRow(currentRow) > 0; - - const endOfLine = new Point(currentRow, Infinity); - const startOfNextLine = new Point(nextRow, 0); - if(i === rowCount - 1) { - // We "trick" the editor into thinking we made a change to where - // the cursor was. Not ideal, but it fixes https://github.com/pulsar-edit/pulsar/issues/802 - this.cursor.setBufferPosition(startOfNextLine); - } - const removeRange = new Range(endOfLine, startOfNextLine); - cursorPositionAfterEdit = - buffer.setTextInRange(removeRange, insertSpace ? ' ' : '').end; - if(nextLineIsntEmpty) { - cursorPositionAfterEdit = cursorPositionAfterEdit.traverse([0, -1]); - } - } + // Remove trailing whitespace from the current line + const scanRange = this.cursor.getCurrentLineBufferRange(); + let trailingWhitespaceRange = null; + this.editor.scanInBufferRange(/[ \t]+$/, scanRange, ({ range }) => { + trailingWhitespaceRange = range; + }); + if (trailingWhitespaceRange) { + this.setBufferRange(trailingWhitespaceRange); + this.deleteSelectedText(options); } - }); + + const currentRow = selectedRange.start.row; + const nextRow = currentRow + 1; + const insertSpace = + nextRow <= this.editor.buffer.getLastRow() && + this.editor.buffer.lineLengthForRow(nextRow) > 0 && + this.editor.buffer.lineLengthForRow(currentRow) > 0; + if (insertSpace) this.insertText(' ', options); + + this.cursor.moveToEndOfLine(); + + // Remove leading whitespace from the line below + this.modifySelection(() => { + this.cursor.moveRight(); + this.cursor.moveToFirstCharacterOfLine(); + }); + this.deleteSelectedText(options); + + if (insertSpace) this.cursor.moveLeft(); + } if (joinMarker) { const newSelectedRange = joinMarker.getBufferRange(); this.setBufferRange(newSelectedRange); joinMarker.destroy(); - } else if(cursorPositionAfterEdit) { - this.cursor.setBufferPosition(cursorPositionAfterEdit); } }