Merge pull request #3302 from VSCodeVim/a

fix: #3277

src/actions/commands/actions.ts

@@ -3,7 +3,7 @@ import * as vscode from 'vscode';
 import { RecordedState } from '../../state/recordedState';
 import { ReplaceState } from '../../state/replaceState';
 import { VimState } from '../../state/vimState';
-import { getCursorsAfterSync } from '../../util/util';
+import { getCursorsAfterSync, waitForCursorSync } from '../../util/util';
 import { Clipboard } from '../../util/clipboard';
 import { FileCommand } from './../../cmd_line/commands/file';
 import { OnlyCommand } from './../../cmd_line/commands/only';
@@ -2580,7 +2580,10 @@ class CommandGoToDefinition extends BaseCommand {
     const oldActiveEditor = vimState.editor;
 
     await vscode.commands.executeCommand('editor.action.goToDeclaration');
-
+    // `executeCommand` returns immediately before cursor is updated
+    // wait for the editor to update before updating the vim state
+    // https://github.com/VSCodeVim/Vim/issues/3277
+    await waitForCursorSync(1000);
     if (oldActiveEditor === vimState.editor) {
       vimState.cursorPosition = Position.FromVSCodePosition(vimState.editor.selection.start);
     }

src/actions/commands/insert.ts

@@ -199,13 +199,13 @@ class CommandInsertIndentInCurrentLine extends BaseCommand {
     const tabSize = configuration.tabstop || Number(vimState.editor.options.tabSize);
     const newIndentationWidth = (indentationWidth / tabSize + 1) * tabSize;
 
-    TextEditor.replaceText(
-      vimState,
-      TextEditor.setIndentationLevel(originalText, newIndentationWidth),
-      position.getLineBegin(),
-      position.getLineEnd(),
-      new PositionDiff(0, newIndentationWidth - indentationWidth)
-    );
+    vimState.recordedState.transformations.push({
+      type: 'replaceText',
+      text: TextEditor.setIndentationLevel(originalText, newIndentationWidth),
+      start: position.getLineBegin(),
+      end: position.getLineEnd(),
+      diff: new PositionDiff(0, newIndentationWidth - indentationWidth),
+    });
 
     return vimState;
   }

src/actions/plugins/surround.ts

@@ -650,15 +650,20 @@ export class CommandSurroundAddToReplacement extends BaseCommand {
       }
 
       if (startReplaceRange) {
-        TextEditor.replaceText(
-          vimState,
-          startReplace,
-          startReplaceRange.start,
-          startReplaceRange.stop
-        );
+        vimState.recordedState.transformations.push({
+          type: 'replaceText',
+          text: startReplace,
+          start: startReplaceRange.start,
+          end: startReplaceRange.stop,
+        });
       }
       if (endReplaceRange) {
-        TextEditor.replaceText(vimState, endReplace, endReplaceRange.start, endReplaceRange.stop);
+        vimState.recordedState.transformations.push({
+          type: 'replaceText',
+          text: endReplace,
+          start: endReplaceRange.start,
+          end: endReplaceRange.stop,
+        });
       }
       if (startDeleteRange) {
         vimState.recordedState.transformations.push({

src/cmd_line/commandLine.ts

@@ -1,14 +1,13 @@
+import * as parser from './parser';
 import * as vscode from 'vscode';
-
-import { configuration } from '../configuration/configuration';
-import { logger } from '../util/logger';
+import { CommandLineHistory } from '../history/historyFile';
 import { Message } from '../util/message';
-import { VimState } from '../state/vimState';
+import { ModeName } from './../mode/mode';
 import { StatusBar } from '../statusBar';
-import * as parser from './parser';
 import { VimError, ErrorCode } from '../error';
-import { CommandLineHistory } from '../../src/util/historyFile';
-import { ModeName } from './../mode/mode';
+import { VimState } from '../state/vimState';
+import { configuration } from '../configuration/configuration';
+import { logger } from '../util/logger';
 
 class CommandLine {
   private _history: CommandLineHistory;

src/common/matching/tagMatcher.ts

@@ -1,6 +1,5 @@
 import { TextEditor } from '../../textEditor';
 import { VimState } from '../../state/vimState';
-import { Position } from 'vscode';
 
 type Tag = { name: string; type: 'close' | 'open'; startPos: number; endPos: number };
 type MatchedTag = {

src/util/historyFile.ts → src/history/historyFile.ts

@@ -2,7 +2,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import * as util from 'util';
 import { configuration } from '../configuration/configuration';
-import { logger } from './logger';
+import { logger } from '../util/logger';
 import { getExtensionDirPath } from '../util/util';
 
 const mkdirp = require('mkdirp');

src/neovim/neovim.ts

@@ -1,15 +1,15 @@
-import * as vscode from 'vscode';
-import { spawn, ChildProcess } from 'child_process';
-import { dirname } from 'path';
-import { exists } from 'fs';
 import * as util from 'util';
-import { attach, Nvim } from 'promised-neovim-client';
-import { configuration } from '../configuration/configuration';
+import * as vscode from 'vscode';
+import { Position } from './../common/motion/position';
 import { Register, RegisterMode } from '../register/register';
 import { TextEditor } from '../textEditor';
-import { Position } from './../common/motion/position';
 import { VimState } from './../state/vimState';
+import { attach, Nvim } from 'promised-neovim-client';
+import { configuration } from '../configuration/configuration';
+import { dirname } from 'path';
+import { exists } from 'fs';
 import { logger } from '../util/logger';
+import { spawn, ChildProcess } from 'child_process';
 
 export class Neovim implements vscode.Disposable {
   private process: ChildProcess;

src/register/register.ts

@@ -1,8 +1,8 @@
-import { BaseCommand, CommandRegister, CommandYankFullLine } from './../actions/commands/actions';
-import { BaseOperator, DeleteOperator, YankOperator } from './../actions/operator';
+import { Clipboard } from './../util/clipboard';
+import { CommandRegister, CommandYankFullLine } from './../actions/commands/actions';
+import { DeleteOperator, YankOperator } from './../actions/operator';
 import { RecordedState } from './../state/recordedState';
 import { VimState } from './../state/vimState';
-import { Clipboard } from './../util/clipboard';
 
 /**
  * There are two different modes of copy/paste in Vim - copy by character

src/state/globalState.ts

@@ -2,7 +2,7 @@ import { JumpTracker } from '../jumps/jumpTracker';
 import { RecordedState } from './../state/recordedState';
 import { SubstituteState } from './substituteState';
 import { SearchState, SearchDirection } from './searchState';
-import { SearchHistory } from '../util/historyFile';
+import { SearchHistory } from '../history/historyFile';
 import { Position } from '../common/motion/position';
 import { ModeName } from '../mode/mode';
 

src/statusBar.ts

@@ -4,16 +4,13 @@ import { configuration } from './configuration/configuration';
 
 class StatusBarImpl implements vscode.Disposable {
   private _statusBarItem: vscode.StatusBarItem;
-  private _previousModeName: ModeName | undefined;
-  private _wasRecordingMacro: boolean;
-  private _wasHighPriority: boolean;
+  private _previousModeName: ModeName | undefined = undefined;
+  private _wasRecordingMacro = false;
+  private _wasHighPriority = false;
 
   constructor() {
     this._statusBarItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left);
     this._statusBarItem.show();
-    this._previousModeName = undefined;
-    this._wasRecordingMacro = false;
-    this._wasHighPriority = false;
   }
 
   public Set(
@@ -22,22 +19,24 @@ class StatusBarImpl implements vscode.Disposable {
     isRecordingMacro: boolean,
     isHighPriority: boolean = false
   ) {
-    let hasModeChanged = mode !== this._previousModeName;
+    const hasModeChanged = mode !== this._previousModeName;
 
     // text
-    let shouldUpdateText =
+    const shouldUpdateText =
       hasModeChanged ||
       mode === ModeName.SearchInProgressMode ||
       mode === ModeName.CommandlineInProgress ||
       isRecordingMacro !== this._wasRecordingMacro ||
       configuration.showcmd;
 
+    // errors and other high-priorty messages remain displayed on the status bar
+    // until specific conditions are met (such as the mode has changed)
     if ((shouldUpdateText && !this._wasHighPriority) || isHighPriority) {
       this.UpdateText(text);
     }
 
     // color
-    let shouldUpdateColor = configuration.statusBarColorControl && hasModeChanged;
+    const shouldUpdateColor = configuration.statusBarColorControl && hasModeChanged;
     if (shouldUpdateColor) {
       this.UpdateColor(mode);
     }
@@ -73,9 +72,8 @@ class StatusBarImpl implements vscode.Disposable {
       }
     }
 
-    const currentColorCustomizations = vscode.workspace
-      .getConfiguration('workbench')
-      .get('colorCustomizations');
+    const workbenchConfiguration = vscode.workspace.getConfiguration('workbench');
+    const currentColorCustomizations = workbenchConfiguration.get('colorCustomizations');
 
     const colorCustomizations = Object.assign(currentColorCustomizations || {}, {
       'statusBar.background': `${background}`,
@@ -88,9 +86,9 @@ class StatusBarImpl implements vscode.Disposable {
       delete colorCustomizations['statusBar.foreground'];
     }
 
-    vscode.workspace
-      .getConfiguration('workbench')
-      .update('colorCustomizations', colorCustomizations, true);
+    if (currentColorCustomizations !== colorCustomizations) {
+      workbenchConfiguration.update('colorCustomizations', colorCustomizations, true);
+    }
   }
 }
 

src/textEditor.ts

@@ -1,10 +1,11 @@
 import * as vscode from 'vscode';
 
-import { Position, PositionDiff } from './common/motion/position';
+import { Position } from './common/motion/position';
 import { configuration } from './configuration/configuration';
-import { VimState } from './state/vimState';
-import { ReplaceTextTransformation } from './transformations/transformations';
 
+/**
+ * Collection of helper functions around vscode.window.activeTextEditor
+ */
 export class TextEditor {
   static readonly whitespaceRegExp = new RegExp('^ *$');
 
@@ -26,8 +27,7 @@ export class TextEditor {
   }
 
   /**
-   * Do not use this method! It has been deprecated. Use InsertTextTransformation
-   * (or possibly InsertTextVSCodeTransformation) instead.
+   * @deprecated Use InsertTextTransformation (or InsertTextVSCodeTransformation) instead.
    */
   static async insert(
     text: string,
@@ -60,6 +60,9 @@ export class TextEditor {
     return true;
   }
 
+  /**
+   * @deprecated Use InsertTextTransformation (or InsertTextVSCodeTransformation) instead.
+   */
   static async insertAt(text: string, position: vscode.Position): Promise<boolean> {
     return vscode.window.activeTextEditor!.edit(editBuilder => {
       editBuilder.insert(position, text);
@@ -81,59 +84,14 @@ export class TextEditor {
   }
 
   /**
-   * Removes all text in the entire document.
-   */
-  static async deleteDocument(): Promise<boolean> {
-    const start = new vscode.Position(0, 0);
-    const lastLine = vscode.window.activeTextEditor!.document.lineCount - 1;
-    const end = vscode.window.activeTextEditor!.document.lineAt(lastLine).range.end;
-    const range = new vscode.Range(start, end);
-
-    return vscode.window.activeTextEditor!.edit(editBuilder => {
-      editBuilder.delete(range);
-    });
-  }
-
-  /**
-   * Do not use this method! It has been deprecated. Use ReplaceTextTransformation.
-   * instead.
+   * @deprecated. Use ReplaceTextTransformation instead.
    */
   static async replace(range: vscode.Range, text: string): Promise<boolean> {
     return vscode.window.activeTextEditor!.edit(editBuilder => {
       editBuilder.replace(range, text);
     });
   }
 
-  /**
-   * This is the correct replace method to use. (Notice how it's not async? Yep)
-   */
-  static replaceText(
-    vimState: VimState,
-    text: string,
-    start: Position,
-    end: Position,
-    diff: PositionDiff | undefined = undefined
-  ): void {
-    const trans: ReplaceTextTransformation = {
-      type: 'replaceText',
-      text,
-      start,
-      end,
-    };
-
-    if (diff) {
-      trans.diff = diff;
-    }
-
-    vimState.recordedState.transformations.push(trans);
-  }
-
-  static readLine(): string {
-    const lineNo = vscode.window.activeTextEditor!.selection.active.line;
-
-    return vscode.window.activeTextEditor!.document.lineAt(lineNo).text;
-  }
-
   static readLineAt(lineNo: number): string {
     if (lineNo === null) {
       lineNo = vscode.window.activeTextEditor!.selection.active.line;
@@ -290,10 +248,6 @@ export type EditorScrollDirection = 'up' | 'down';
  */
 export type EditorScrollByUnit = 'line' | 'wrappedLine' | 'page' | 'halfPage';
 
-/**
- * Values for reveal line 'at' argument
- */
-export type RevealLineAtArgument = 'top' | 'center' | 'bottom';
 /**
  * Positions in the view for cursor move command.
  */

src/util/util.ts

@@ -12,11 +12,11 @@ const AppDirectory = require('appdirectory');
  * update the position of the cursor. So we have to wait!
  */
 export async function waitForCursorSync(
-  timeout: number = 0,
+  timeoutInMilliseconds: number = 0,
   rejectOnTimeout = false
 ): Promise<void> {
   await new Promise((resolve, reject) => {
-    let timer = setTimeout(rejectOnTimeout ? reject : resolve, timeout);
+    let timer = setTimeout(rejectOnTimeout ? reject : resolve, timeoutInMilliseconds);
 
     const disposable = vscode.window.onDidChangeTextEditorSelection(x => {
       disposable.dispose();
@@ -26,11 +26,11 @@ export async function waitForCursorSync(
   });
 }
 
-export async function getCursorsAfterSync(timeout: number = 0): Promise<Range[]> {
+export async function getCursorsAfterSync(timeoutInMilliseconds: number = 0): Promise<Range[]> {
   try {
-    await waitForCursorSync(timeout, true);
+    await waitForCursorSync(timeoutInMilliseconds, true);
   } catch (e) {
-    logger.warn(`getCursorsAfterSync: selection not updated within ${timeout}ms. error=${e}.`);
+    logger.warn(`getCursorsAfterSync: selection not updated within ${timeoutInMilliseconds}ms. error=${e}.`);
   }
 
   return vscode.window.activeTextEditor!.selections.map(

test/cmd_line/commandLineHistory.test.ts

@@ -1,8 +1,6 @@
-import { CommandLineHistory } from '../../src/util/historyFile';
+import { CommandLineHistory } from '../../src/history/historyFile';
 import { assertEqual, setupWorkspace, cleanUpWorkspace } from '../testUtils';
 import { configuration } from '../../src/configuration/configuration';
-import * as path from 'path';
-import * as os from 'os';
 import * as assert from 'assert';
 
 suite('command-line history', () => {