Merge pull request #3068 from captaincaius/add-substitution-state

Adding state to substitution command

src/cmd_line/commands/substitute.ts

@@ -10,9 +10,17 @@ import { configuration } from '../../configuration/configuration';
 import { Decoration } from '../../configuration/decoration';
 import { Jump } from '../../jumps/jump';
 import { Position } from '../../common/motion/position';
+import { SubstituteState } from '../../state/substituteState';
+import { SearchState, SearchDirection } from '../../state/searchState';
 
+/**
+ * NOTE: for "pattern", undefined is different from an empty string.
+ * when it's undefined, it means to repeat the previous REPLACEMENT and ignore "replace".
+ * when it's an empty string, it means to use the previous SEARCH (not replacement) state,
+ * and replace with whatever's set by "replace" (even an empty string).
+ */
 export interface ISubstituteCommandArguments extends node.ICommandArgs {
-  pattern: string;
+  pattern: string | undefined;
   replace: string;
   flags: number;
   count?: number;
@@ -50,6 +58,31 @@ export enum SubstituteFlags {
   UsePreviousPattern = 0x400,
 }
 
+/**
+ * vim has a distinctly different state for previous search and for previous substitute.  However, in SOME
+ * cases a substitution will also update the search state along with the substitute state.
+ *
+ * Also, the substitute command itself will sometimes use the search state, and other times it will use the
+ * substitute state.
+ *
+ * These are the following cases and how vim handles them:
+ * 1. :s/this/that
+ *   - standard search/replace
+ *   - update substitution state
+ *   - update search state too!
+ * 2. :s or :s [flags]
+ *   - use previous SUBSTITUTION state, and repeat previous substitution pattern and replace.
+ *   - do not touch search state!
+ *   - changing substitution state is dont-care cause we're repeating it ;)
+ * 3. :s/ or :s// or :s///
+ *   - use previous SEARCH state (not substitution), and DELETE the string matching the pattern (replace with nothing)
+ *   - update substitution state
+ *   - updating search state is dont-care cause we're repeating it ;)
+ * 4. :s/this or :s/this/ or :s/this//
+ *   - input is pattern - replacement is empty (delete)
+ *   - update replacement state
+ *   - update search state too!
+ */
 export class SubstituteCommand extends node.CommandBase {
   neovimCapable = true;
   protected _arguments: ISubstituteCommandArguments;
@@ -84,14 +117,35 @@ export class SubstituteCommand extends node.CommandBase {
       jsRegexFlags += 'i';
     }
 
-    // If no pattern is entered, use previous search state (including search with * and #)
-    if (args.pattern === '') {
-      const prevSearchState = vimState.globalState.searchState;
-      if (prevSearchState === undefined || prevSearchState.searchString === '') {
+    if (args.pattern === undefined) {
+      // If no pattern is entered, use previous SUBSTITUTION state and don't update search state
+      // i.e. :s
+      const prevSubstiteState = vimState.globalState.substituteState;
+      if (prevSubstiteState === undefined || prevSubstiteState.searchPattern === '') {
         throw VimError.fromCode(ErrorCode.E35);
       } else {
-        args.pattern = prevSearchState.searchString;
+        args.pattern = prevSubstiteState.searchPattern;
+        args.replace = prevSubstiteState.replaceString;
       }
+    } else {
+      if (args.pattern === '') {
+        // If an explicitly empty pattern is entered, use previous search state (including search with * and #) and update both states
+        // e.g :s/ or :s///
+        const prevSearchState = vimState.globalState.searchState;
+        if (prevSearchState === undefined || prevSearchState.searchString === '') {
+          throw VimError.fromCode(ErrorCode.E35);
+        } else {
+          args.pattern = prevSearchState.searchString;
+        }
+      }
+      vimState.globalState.substituteState = new SubstituteState(args.pattern, args.replace);
+      vimState.globalState.searchState = new SearchState(
+        SearchDirection.Forward,
+        vimState.cursorPosition,
+        args.pattern,
+        { isRegex: true },
+        vimState.currentMode
+      );
     }
     return new RegExp(args.pattern, jsRegexFlags);
   }

src/cmd_line/subparsers/substitute.ts

@@ -128,40 +128,67 @@ function parseCount(scanner: Scanner): number {
  */
 export function parseSubstituteCommandArgs(args: string): node.SubstituteCommand {
   try {
+    let searchPattern: string | undefined;
+    let replaceString: string;
+    let flags: number;
+    let count: number;
+
+    if (!args) {
+      // special case for :s
+      return new node.SubstituteCommand({
+        pattern: undefined,
+        replace: '', // ignored in this context
+        flags: node.SubstituteFlags.None,
+        count: 1,
+      });
+    }
+    let scanner: Scanner;
+
     let delimiter = args[0];
 
     if (isValidDelimiter(delimiter)) {
-      let scanner = new Scanner(args.substr(1, args.length - 1));
-      let [searchPattern, searchDelimiter] = parsePattern('', scanner, delimiter);
-
-      if (searchDelimiter) {
-        let replaceString = parsePattern('', scanner, delimiter)[0];
-
-        scanner.skipWhiteSpace();
-        let flags = parseSubstituteFlags(scanner);
-        scanner.skipWhiteSpace();
-        let count = parseCount(scanner);
+      if (args.length === 1) {
+        // special case for :s/ or other delimiters
         return new node.SubstituteCommand({
-          pattern: searchPattern,
-          replace: replaceString,
-          flags: flags,
-          count: count,
+          pattern: '',
+          replace: '',
+          flags: node.SubstituteFlags.None,
+          count: 1,
         });
-      } else {
+      }
+
+      let secondDelimiterFound: boolean;
+
+      scanner = new Scanner(args.substr(1, args.length - 1));
+      [searchPattern, secondDelimiterFound] = parsePattern('', scanner, delimiter);
+
+      if (!secondDelimiterFound) {
+        // special case for :s/search
         return new node.SubstituteCommand({
           pattern: searchPattern,
           replace: '',
           flags: node.SubstituteFlags.None,
-          count: 0,
+          count: 1,
         });
       }
+      replaceString = parsePattern('', scanner, delimiter)[0];
+    } else {
+      // if it's not a valid delimiter, it must be flags, so start parsing from here
+      searchPattern = undefined;
+      replaceString = '';
+      scanner = new Scanner(args);
     }
 
-    // TODO(rebornix): Can this ever happen?
+    scanner.skipWhiteSpace();
+    flags = parseSubstituteFlags(scanner);
+    scanner.skipWhiteSpace();
+    count = parseCount(scanner);
+
     return new node.SubstituteCommand({
-      pattern: '',
-      replace: '',
-      flags: node.SubstituteFlags.None,
+      pattern: searchPattern,
+      replace: replaceString,
+      flags: flags,
+      count: count,
     });
   } catch (e) {
     throw error.VimError.fromCode(error.ErrorCode.E486);

src/state/globalState.ts

@@ -1,5 +1,6 @@
 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 { Position } from '../common/motion/position';
@@ -20,6 +21,11 @@ export class GlobalState {
    */
   private static _searchStatePrevious: SearchState[] = [];
 
+  /**
+   * Last substitute state for running :s by itself
+   */
+  private static _substituteState: SubstituteState | undefined = undefined;
+
   /**
    * Last search state for running n and N commands
    */
@@ -85,6 +91,14 @@ export class GlobalState {
     GlobalState._previousFullAction = state;
   }
 
+  public get substituteState(): SubstituteState | undefined {
+    return GlobalState._substituteState;
+  }
+
+  public set substituteState(state: SubstituteState | undefined) {
+    GlobalState._substituteState = state;
+  }
+
   public get searchState(): SearchState | undefined {
     return GlobalState._searchState;
   }

src/state/substituteState.ts

@@ -0,0 +1,22 @@
+import { Position } from './../common/motion/position';
+import { TextEditor } from './../textEditor';
+
+/**
+ * State involved with Substitution commands (:s).
+ */
+export class SubstituteState {
+  /**
+   * The last pattern searched for in the substitution
+   */
+  public searchPattern: string;
+
+  /**
+   * The last replacement string in the substitution
+   */
+  public replaceString: string;
+
+  constructor(searchPattern: string, replaceString: string) {
+    this.searchPattern = searchPattern;
+    this.replaceString = replaceString;
+  }
+}

test/cmd_line/substitute.test.ts

@@ -10,8 +10,10 @@ import {
   reloadConfiguration,
   setupWorkspace,
 } from './../testUtils';
+import { getTestingFunctions } from '../testSimplifier';
 
 suite('Basic substitute', () => {
+  let { newTest, newTestOnly } = getTestingFunctions();
   let modeHandler: ModeHandler;
 
   setup(async () => {
@@ -243,7 +245,7 @@ suite('Basic substitute', () => {
       assertEqualLines(['bz']);
     });
   });
-  suite('Substitute with empty search string should use previous search', () => {
+  suite('Substitute should use various previous search/substitute states', () => {
     test('Substitute with previous search using *', async () => {
       await modeHandler.handleMultipleKeyEvents([
         'i',
@@ -334,6 +336,74 @@ suite('Basic substitute', () => {
 
       assertEqualLines(['fighters', 'bar', 'fighters', 'bar']);
     });
+    newTest({
+      title: 'Substitute with parameters should update search state',
+      start: ['foo', 'bar', 'foo', 'bar|'],
+      keysPressed:
+        '/bar\n' + // search for bar (search state now = bar)
+        ':s/ar/ite\n' + // change first bar to bite (search state now = ar, not bar)
+        'n' + // repeat search (ar, not bar)
+        'rr', // and replace a with r
+      end: ['foo', 'bite', 'foo', 'b|rr'],
+    });
+    newTest({
+      title:
+        'Substitute with empty replacement should delete previous substitution (all variants) and accepts flags',
+      start: [
+        'link',
+        '|ganon is here',
+        'link',
+        'ganon is here',
+        'link',
+        'ganon is here',
+        'link',
+        'ganon is here',
+        'link',
+        'ganon ganon is here',
+      ],
+      keysPressed:
+        ':s/ganon/zelda\n' + // replace ganon with zelda (ensuring we have a prior replacement state)
+        'n' + // find next ganon
+        ':s/\n' + // replace ganon with nothing (using prior state)
+        ':s/ganon/zelda\n' + // does nothing (just ensuring we have a prior replacement state)
+        'n' + // find next ganon
+        ':s//\n' + // replace ganon with nothing (using prior state)
+        'n' + // find next ganon
+        ':s/ganon\n' + // replace ganon with nothing (using single input)
+        ':s/ganon/zelda\n' + // does nothing (just ensuring we have a prior replacement state)
+        'n' + // find next ganon
+        ':s///g\n', // replace ganon with nothing
+      end: [
+        'link',
+        'zelda is here',
+        'link',
+        ' is here',
+        'link',
+        ' is here',
+        'link',
+        ' is here',
+        'link',
+        '|  is here',
+      ],
+    });
+    newTest({
+      title:
+        'Substitute with no pattern should repeat previous substitution and not alter search state',
+      start: ['|link', 'zelda', 'link', 'zelda', 'link'],
+      keysPressed:
+        ':s/ink/egend\n' + // replace link with legend (search state now = egend, and substitute state set)
+        '/link\n' + // search for link (search state now = link, not ink)
+        ':s\n' + // repeat replacement (using substitute state, so ink, not link - note: search state should NOT change)
+        'n' + // repeat search for link, not ink
+        'rp', // and replace l with p (confirming search state was unaltered)
+      end: ['legend', 'zelda', 'legend', 'zelda', '|pink'],
+    });
+    newTest({
+      title: 'Substitute repeat previous should accept flags',
+      start: ['|fooo'],
+      keysPressed: ':s/o/un\n:s g\n', // repeated replacement accepts g flag, replacing all other occurrences
+      end: ['|fununun'],
+    });
     test('Substitute with empty search string should use last searched pattern', async () => {
       await modeHandler.handleMultipleKeyEvents([
         'i',