diff --git a/extensionBase.ts b/extensionBase.ts index 8850cdfa155..be151f3fc84 100644 --- a/extensionBase.ts +++ b/extensionBase.ts @@ -276,6 +276,33 @@ export async function activate( const mh = await getAndUpdateModeHandler(); + const selectionsHash = e.selections.reduce( + (hash, s) => + hash + + `[${s.anchor.line}, ${s.anchor.character}; ${s.active.line}, ${s.active.character}]`, + '' + ); + const idx = mh.vimState.selectionsChanged.ourSelections.indexOf(selectionsHash); + if (idx > -1) { + logger.debug( + `Selections: Ignoring selection: ${selectionsHash}, Count left: ${ + mh.vimState.selectionsChanged.ourSelections.length - 1 + }` + ); + mh.vimState.selectionsChanged.ourSelections.splice(idx, 1); + return; + } else if (mh.vimState.selectionsChanged.ignoreIntermediateSelections) { + logger.debug(`Selections: ignoring intermediate selection change: ${selectionsHash}`); + return; + } else if (mh.vimState.selectionsChanged.ourSelections.length > 0) { + // Some intermediate selection must have slipped in after setting the + // 'ignoreIntermediateSelections' to false. Which means we didn't count + // for it yet, but since we have selections to be ignored then we probably + // wanted this one to be ignored as well. + logger.debug(`Selections: Ignoring slipped selection: ${selectionsHash}`); + return; + } + // We may receive changes from other panels when, having selections in them containing the same file // and changing text before the selection in current panel. if (e.textEditor !== mh.vimState.editor) { @@ -302,7 +329,7 @@ export async function activate( ); }, true, - true + false ); const compositionState = new CompositionState(); diff --git a/src/configuration/remapper.ts b/src/configuration/remapper.ts index f37155836f9..bee7988e46d 100644 --- a/src/configuration/remapper.ts +++ b/src/configuration/remapper.ts @@ -135,12 +135,16 @@ export class Remapper implements IRemapper { if (vimState.currentMode === Mode.Insert) { // Revert every single inserted character. // We subtract 1 because we haven't actually applied the last key. + + // Make sure the resulting selection changed events are ignored + vimState.selectionsChanged.ignoreIntermediateSelections = true; await vimState.historyTracker.undoAndRemoveChanges( Math.max(0, numCharsToRemove * vimState.cursors.length) ); vimState.cursors = vimState.cursors.map((c) => c.withNewStop(c.stop.getLeft(numCharsToRemove)) ); + vimState.selectionsChanged.ignoreIntermediateSelections = false; } // We need to remove the keys that were remapped into different keys from the state. vimState.recordedState.actionKeys = vimState.recordedState.actionKeys.slice( diff --git a/src/mode/modeHandler.ts b/src/mode/modeHandler.ts index 7e26ad7ccb6..89c264b01ae 100644 --- a/src/mode/modeHandler.ts +++ b/src/mode/modeHandler.ts @@ -124,10 +124,37 @@ export class ModeHandler implements vscode.Disposable { * Anyone who wants to change the behavior of this method should make sure * all selection related test cases pass. Follow this spec * https://gist.github.com/rebornix/d21d1cc060c009d4430d3904030bd4c1 to - * perform the manual testing. + * perform the manual testing. Besides this testing you should still test + * commands like 'editor.action.smartSelect.grow' and you should test moving + * continuously up/down or left/right with and without remapped movement keys + * because sometimes vscode lags behind and calls this function with information + * that is not up to date with our selections yet and we need to make sure we don't + * change our cursors to previous information (this usally is only an issue in visual + * mode because of our different ways of handling selections and in those cases + * updating our cursors with not up to date info might result in us changing our + * cursor start position). */ public async handleSelectionChange(e: vscode.TextEditorSelectionChangeEvent): Promise { + if ( + vscode.window.activeTextEditor === undefined || + e.textEditor.document !== vscode.window.activeTextEditor.document + ) { + // we don't care if there is no active editor + // or user selection changed in a paneled window (e.g debug console/terminal) + // This check is made before enqueuing this selection change, but sometimes + // between the enqueueing and the actual calling of this function the editor + // might close or change to other document + return; + } let selection = e.selections[0]; + this._logger.debug( + `Selections: Handling Selection Change! Selection: ${Position.FromVSCodePosition( + selection.anchor + ).toString()}, ${Position.FromVSCodePosition(selection.active)}, SelectionsLength: ${ + e.selections.length + }` + ); + if ( (e.selections.length !== this.vimState.cursors.length || this.vimState.isMultiCursor) && this.vimState.currentMode !== Mode.VisualBlock @@ -143,6 +170,16 @@ export class ModeHandler implements vscode.Disposable { Position.FromVSCodePosition(sel.active) ) ); + if ( + e.textEditor.selections.some((s) => !s.anchor.isEqual(s.active)) && + [Mode.Normal, Mode.Insert, Mode.Replace].includes(this.vimState.currentMode) + ) { + // If we got a visual selection and we are on normal, insert or replace mode, enter visual mode. + // We shouldn't go to visual mode on any other mode, because the other visual modes are handled + // very differently than vscode so only our extension will create them. And the other modes + // like the plugin modes shouldn't be changed or else it might mess up the plugins actions. + await this.setCurrentMode(Mode.Visual); + } return this.updateView(this.vimState); } @@ -152,17 +189,76 @@ export class ModeHandler implements vscode.Disposable { */ if (e.kind !== vscode.TextEditorSelectionChangeKind.Mouse) { if (selection) { + if (e.kind === vscode.TextEditorSelectionChangeKind.Command) { + // This 'Command' kind is triggered when using a command like 'editor.action.smartSelect.grow' + // but it is also triggered when we set the 'editor.selections' on 'updateView'. + if ( + [Mode.Normal, Mode.Visual, Mode.Insert, Mode.Replace].includes( + this.vimState.currentMode + ) + ) { + // Since the selections weren't ignored then probably we got change of selection from + // a command, so we need to update our start and stop positions. This is where commands + // like 'editor.action.smartSelect.grow' are handled. + if (this.vimState.currentMode === Mode.Visual) { + this._logger.debug('Selections: Updating Visual Selection!'); + this.vimState.cursorStopPosition = Position.FromVSCodePosition(selection.active); + this.vimState.cursorStartPosition = Position.FromVSCodePosition(selection.anchor); + await this.updateView(this.vimState, { drawSelection: false, revealRange: false }); + return; + } else if (!selection.active.isEqual(selection.anchor)) { + this._logger.debug('Selections: Creating Visual Selection from command!'); + this.vimState.cursorStopPosition = Position.FromVSCodePosition(selection.active); + this.vimState.cursorStartPosition = Position.FromVSCodePosition(selection.anchor); + await this.setCurrentMode(Mode.Visual); + await this.updateView(this.vimState, { drawSelection: false, revealRange: false }); + return; + } + } + } + // Here we are on the selection changed of kind 'Keyboard' or 'undefined' which is triggered + // when pressing movement keys that are not caught on the 'type' override but also when using + // commands like 'cursorMove'. + if (isVisualMode(this.vimState.currentMode)) { /** * In Visual Mode, our `cursorPosition` and `cursorStartPosition` can not reflect `active`, * `start`, `end` and `anchor` information in a selection. * See `Fake block cursor with text decoration` section of `updateView` method. + * Besides this, sometimes on visual modes our start position is not the same has vscode + * anchor because we need to move vscode anchor one to the right of our start when our start + * is after our stop in order to include the start character on vscodes selection. */ return; } + // We get here when we use a 'cursorMove' command (that is considered a selection changed + // kind of 'Keyboard') that ends past the line break. But our cursors are already on last + // character which is what we want. Even though our cursors will be corrected again when + // checking if they are in bounds on 'runAction' there is no need to be changing them back + // and forth so we check for this situation here. + if ( + this.vimState.cursorStopPosition.isEqual(this.vimState.cursorStartPosition) && + this.vimState.cursorStopPosition.getRight().isLineEnd() && + this.vimState.cursorStopPosition.getLineEnd().isEqual(selection.active) + ) { + return; + } + + // Here we allow other 'cursorMove' commands to update our cursors in case there is another + // extension making cursor changes that we need to catch. + // + // We still need to be careful with this because this here might be changing our cursors + // in ways we don't want to. So with future selection issues this is a good place to start + // looking. + this._logger.debug( + `Selections: Changing Cursors from selection handler... ${Position.FromVSCodePosition( + selection.anchor + ).toString()}, ${Position.FromVSCodePosition(selection.active)}` + ); this.vimState.cursorStopPosition = Position.FromVSCodePosition(selection.active); - this.vimState.cursorStartPosition = Position.FromVSCodePosition(selection.start); + this.vimState.cursorStartPosition = Position.FromVSCodePosition(selection.anchor); + await this.updateView(this.vimState, { drawSelection: false, revealRange: false }); } return; } @@ -328,6 +424,7 @@ export class ModeHandler implements vscode.Disposable { this.vimState = await this.handleKeyEventHelper(key, this.vimState); } } catch (e) { + this.vimState.selectionsChanged.ignoreIntermediateSelections = false; if (e instanceof VimError) { StatusBar.displayError(this.vimState, e); } else { @@ -457,6 +554,7 @@ export class ModeHandler implements vscode.Disposable { ): Promise { let ranRepeatableAction = false; let ranAction = false; + vimState.selectionsChanged.ignoreIntermediateSelections = true; // If arrow keys or mouse was used prior to entering characters while in insert mode, create an undo point // this needs to happen before any changes are made @@ -680,6 +778,7 @@ export class ModeHandler implements vscode.Disposable { }; } + vimState.selectionsChanged.ignoreIntermediateSelections = false; return vimState; } @@ -1269,6 +1368,33 @@ export class ModeHandler implements vscode.Disposable { } } + // Check if the selection we are going to set is different than the current one. + // If they are the same vscode won't trigger a selectionChangeEvent so we don't + // have to add it to the ignore selections. + const willTriggerChange = + selections.length !== vimState.editor.selections.length + ? true + : selections.some( + (s, i) => + !s.anchor.isEqual(vimState.editor.selections[i].anchor) || + !s.active.isEqual(vimState.editor.selections[i].active) + ); + + if (willTriggerChange) { + const selectionsHash = selections.reduce( + (hash, s) => + hash + + `[${s.anchor.line}, ${s.anchor.character}; ${s.active.line}, ${s.active.character}]`, + '' + ); + vimState.selectionsChanged.ourSelections.push(selectionsHash); + this._logger.debug( + `Selections: Adding Selection Change to be Ignored! Hash: ${selectionsHash}, Selections: ${Position.FromVSCodePosition( + selections[0].anchor + ).toString()}, ${Position.FromVSCodePosition(selections[0].active).toString()}` + ); + } + this.vimState.editor.selections = selections; } diff --git a/src/state/vimState.ts b/src/state/vimState.ts index 4438880a91d..a526e179f7b 100644 --- a/src/state/vimState.ts +++ b/src/state/vimState.ts @@ -209,6 +209,26 @@ export class VimState implements vscode.Disposable { */ public lastClickWasPastEol: boolean = false; + /** + * Used internally to ignore selection changes that were performed by us. + * 'ignoreIntermediateSelections': set to true when running an action, during this time + * all selections change events will be ignored. + * 'ourSelections': keeps track of our selections that will trigger a selection change event + * so that we can ignore them. + */ + public selectionsChanged = { + /** + * Set to true when running an action, during this time + * all selections change events will be ignored. + */ + ignoreIntermediateSelections: false, + /** + * keeps track of our selections that will trigger a selection change event + * so that we can ignore them. + */ + ourSelections: Array(), + }; + /** * The mode Vim will be in once this action finishes. */ diff --git a/test/mode/modeVisual.test.ts b/test/mode/modeVisual.test.ts index e33c7fdb10e..1d308a72025 100644 --- a/test/mode/modeVisual.test.ts +++ b/test/mode/modeVisual.test.ts @@ -1506,4 +1506,149 @@ suite('Mode Visual', () => { }); } }); + + suite('Visual mode with command editor.action.smartSelect.grow', () => { + newTest({ + title: 'Command editor.action.smartSelect.grow enters visual mode', + config: { + normalModeKeyBindings: [ + { + before: ['', 'a', 'f'], + commands: ['editor.action.smartSelect.grow'], + }, + ], + leader: ' ', + }, + start: [ + `"vim.normalModeKeyBindingsNonRecursive": [`, + ` {`, + ` "be|fore": ["j"],`, + ` "after": ["g", "j"],`, + ` },`, + `]`, + ], + keysPressed: ' aflh', + end: [ + `"vim.normalModeKeyBindingsNonRecursive": [`, + ` {`, + ` "|before": ["j"],`, + ` "after": ["g", "j"],`, + ` },`, + `]`, + ], + endMode: Mode.Visual, + }); + + newTest({ + title: 'Command editor.action.smartSelect.grow enters visual mode and increases selection', + config: { + normalModeKeyBindings: [ + { + before: ['', 'a', 'f'], + commands: ['editor.action.smartSelect.grow'], + }, + ], + visualModeKeyBindings: [ + { + before: ['', 'a', 'f'], + commands: ['editor.action.smartSelect.grow'], + }, + ], + leader: ' ', + }, + start: [ + `"vim.normalModeKeyBindingsNonRecursive": [`, + ` {`, + ` "be|fore": ["j"],`, + ` "after": ["g", "j"],`, + ` },`, + `]`, + ], + keysPressed: ' af afd', + end: [ + `"vim.normalModeKeyBindingsNonRecursive": [`, + ` {`, + ` | `, + ` "after": ["g", "j"],`, + ` },`, + `]`, + ], + endMode: Mode.Normal, + }); + + newTest({ + title: 'Command editor.action.smartSelect.grow enters visual mode on single character', + config: { + normalModeKeyBindings: [ + { + before: ['', 'a', 'f'], + commands: ['editor.action.smartSelect.grow'], + }, + ], + visualModeKeyBindings: [ + { + before: ['', 'a', 'f'], + commands: ['editor.action.smartSelect.grow'], + }, + ], + leader: ' ', + }, + start: [ + `"vim.normalModeKeyBindingsNonRecursive": [`, + ` {`, + ` "before": ["|j"],`, + ` "after": ["g", "j"],`, + ` },`, + `]`, + ], + keysPressed: ' afd', + end: [ + `"vim.normalModeKeyBindingsNonRecursive": [`, + ` {`, + ` "before": ["|"],`, + ` "after": ["g", "j"],`, + ` },`, + `]`, + ], + endMode: Mode.Normal, + }); + + newTest({ + title: 'Command editor.action.smartSelect.grow enters visual mode on multicursors', + config: { + normalModeKeyBindings: [ + { + before: ['', 'a', 'f'], + commands: ['editor.action.smartSelect.grow'], + }, + ], + visualModeKeyBindings: [ + { + before: ['', 'a', 'f'], + commands: ['editor.action.smartSelect.grow'], + }, + ], + leader: ' ', + }, + start: [ + `"vim.normalModeKeyBindingsNonRecursive": [`, + ` {`, + ` "before": ["|j"],`, + ` "after": ["g", "j"],`, + ` },`, + `]`, + ], + // the initial 'vlgb_ll' is just to create a multicursor on the words 'before' and 'after' + keysPressed: 'vlgb_ll afd', + end: [ + `"vim.normalModeKeyBindingsNonRecursive": [`, + ` {`, + ` "|": ["j"],`, + ` "": ["g", "j"],`, + ` },`, + `]`, + ], + endMode: Mode.Normal, + }); + }); });