Skip to content

Commit

Permalink
Implement better synchronization of selections
Browse files Browse the repository at this point in the history 
- Store our selections when calling 'updateView' to be later ignored.
- Create a 'ignoreIntermediateSelections' flag that is set when running
an action to ignore any selection changes triggered by any step of that
action.
- When getting a selection change event we first check if it is one of
our selections. If it is we ignore it, if it is not we still ignore it
if the 'ignoreIntermediateSelections' flag is set or if we have other
selections to ignore, because that means this selection slipped in after
the end of the action. Otherwise we handle it.
- Create new handle for selection changes with kind 'Command'. (this is
triggered by editor commands like 'smartSelect.grow' but is also
triggered when we set the 'editor.selections' on 'updateView', except
this last one will now be ignored)
  • Loading branch information
@berknam
berknam committed Jul 19, 2020
1 parent 386ebb9 commit db2b564
Show file tree
Hide file tree
Showing 5 changed files with 325 additions and 3 deletions.
29 changes: 28 additions & 1 deletion extensionBase.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -302,7 +329,7 @@ export async function activate(
);
},
true,
true
false
);

const compositionState = new CompositionState();
Expand Down
4 changes: 4 additions & 0 deletions src/configuration/remapper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
130 changes: 128 additions & 2 deletions src/mode/modeHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
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
Expand All @@ -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);
}

Expand All @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -457,6 +554,7 @@ export class ModeHandler implements vscode.Disposable {
): Promise<VimState> {
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
Expand Down Expand Up @@ -680,6 +778,7 @@ export class ModeHandler implements vscode.Disposable {
};
}

vimState.selectionsChanged.ignoreIntermediateSelections = false;
return vimState;
}

Expand Down Expand Up @@ -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;
}

Expand Down
20 changes: 20 additions & 0 deletions src/state/vimState.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>(),
};

/**
* The mode Vim will be in once this action finishes.
*/
Expand Down
Loading

0 comments on commit db2b564

Please sign in to comment.