From c9b405a49de6aa4741fd3c5a55ff9f3247d26bc8 Mon Sep 17 00:00:00 2001 From: Captain Caius <241342+captaincaius@users.noreply.github.com> Date: Sat, 22 Sep 2018 16:51:17 +0300 Subject: [PATCH 1/3] feature - substitute state and :s - closes #3067 --- src/actions/commands/actions.ts | 2 +- src/cmd_line/commands/substitute.ts | 35 +++++++-- src/cmd_line/subparsers/substitute.ts | 67 +++++++++++------ src/state/globalState.ts | 14 ++++ src/state/substituteState.ts | 22 ++++++ test/cmd_line/substitute.test.ts | 100 ++++++++++++++++++++++++++ 6 files changed, 213 insertions(+), 27 deletions(-) create mode 100644 src/state/substituteState.ts diff --git a/src/actions/commands/actions.ts b/src/actions/commands/actions.ts index c14c8496dd6..c998580df32 100644 --- a/src/actions/commands/actions.ts +++ b/src/actions/commands/actions.ts @@ -1872,7 +1872,7 @@ class CommandInsertInCommandline extends BaseCommand { @RegisterAction class CommandEscInCommandline extends BaseCommand { modes = [ModeName.CommandlineInProgress]; - keys = [[''], [''], ['']];; + keys = [[''], [''], ['']]; runsOnceForEveryCursor() { return this.keysPressed[0] === '\n'; } diff --git a/src/cmd_line/commands/substitute.ts b/src/cmd_line/commands/substitute.ts index 5f463aa671a..189e2f09fd9 100644 --- a/src/cmd_line/commands/substitute.ts +++ b/src/cmd_line/commands/substitute.ts @@ -10,9 +10,11 @@ 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'; export interface ISubstituteCommandArguments extends node.ICommandArgs { - pattern: string; + pattern: string | undefined; replace: string; flags: number; count?: number; @@ -84,14 +86,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); } diff --git a/src/cmd_line/subparsers/substitute.ts b/src/cmd_line/subparsers/substitute.ts index b50b0cd73ba..c4999cc0f96 100644 --- a/src/cmd_line/subparsers/substitute.ts +++ b/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); diff --git a/src/state/globalState.ts b/src/state/globalState.ts index a42797c740f..ea1ec17c657 100644 --- a/src/state/globalState.ts +++ b/src/state/globalState.ts @@ -1,5 +1,6 @@ import { JumpTracker } from '../jumps/jumpTracker'; import { RecordedState } from './../state/recordedState'; +import { SubstituteState } from './substituteState'; import { SearchState } from './searchState'; /** @@ -17,6 +18,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 */ @@ -56,6 +62,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; } diff --git a/src/state/substituteState.ts b/src/state/substituteState.ts new file mode 100644 index 00000000000..6b0807e3d38 --- /dev/null +++ b/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; + } +} diff --git a/test/cmd_line/substitute.test.ts b/test/cmd_line/substitute.test.ts index 070df112ddd..e98e15b9a37 100644 --- a/test/cmd_line/substitute.test.ts +++ b/test/cmd_line/substitute.test.ts @@ -303,6 +303,106 @@ suite('Basic substitute', () => { assertEqualLines(['fighters', 'bar', 'fighters', 'bar']); }); + test('Substitute with parameters should update search state', async () => { + await modeHandler.handleMultipleKeyEvents([ + 'i', // foo bar foo bar + 'f', + 'o', + 'o', + '', + 'o', + 'b', + 'a', + 'r', + '', + 'o', + 'f', + 'o', + 'o', + '', + 'o', + 'b', + 'a', + 'r', + '', + '/', // search for bar + 'b', + 'a', + 'r', + '\n', + ]); + await commandLine.Run('s/ar/ite', modeHandler.vimState); // first bar to bite + await modeHandler.handleMultipleKeyEvents([ + 'n', // repeat search for ar, not bar + 'r', // replace bar with brr + 'r', + ]); + + assertEqualLines(['foo', 'bite', 'foo', 'brr']); + }); + test('Substitute with no pattern should repeat previous substitution and not update search state', async () => { + await modeHandler.handleMultipleKeyEvents([ + 'i', // link zelda link zelda link + 'l', + 'i', + 'n', + 'k', + '', + 'o', + 'z', + 'e', + 'l', + 'd', + 'a', + '', + 'o', + 'l', + 'i', + 'n', + 'k', + '', + 'o', + 'z', + 'e', + 'l', + 'd', + 'a', + '', + 'o', + 'l', + 'i', + 'n', + 'k', + '', + '1', // go to the first line + 'g', + 'g', + ]); + await commandLine.Run('s/ink/egend', modeHandler.vimState); // replace first ink with egend + await modeHandler.handleMultipleKeyEvents([ + '/', + 'l', + 'i', + 'n', + 'k', // search for full link + '\n', + ]); + await commandLine.Run('s', modeHandler.vimState); // repeat previous replacement of ink, not link! + await modeHandler.handleMultipleKeyEvents([ + 'n', // search for link, not ink + 'r', + 'p', // should be pink now + ]); + + assertEqualLines(['legend', 'zelda', 'legend', 'zelda', 'pink']); + }); + test('Substitute repeat previous should accept flags', async () => { + await modeHandler.handleMultipleKeyEvents(['i', 'f', 'o', 'o', 'o', '']); + await commandLine.Run('s/o/un', modeHandler.vimState); // replace first o with un + await commandLine.Run('s g', modeHandler.vimState); // repeat replacement on all following o's + + assertEqualLines(['fununun']); + }); test('Substitute with empty search string should use last searched pattern', async () => { await modeHandler.handleMultipleKeyEvents([ 'i', From 2f834649094f6f83082c489903867f2384d005a7 Mon Sep 17 00:00:00 2001 From: Captain Caius <241342+captaincaius@users.noreply.github.com> Date: Fri, 28 Sep 2018 17:37:10 +0300 Subject: [PATCH 2/3] Better tests using newTest and added missing empty-replacement coverage --- test/cmd_line/substitute.test.ts | 166 +++++++++++++------------------ 1 file changed, 68 insertions(+), 98 deletions(-) diff --git a/test/cmd_line/substitute.test.ts b/test/cmd_line/substitute.test.ts index 042f830452d..2214f722665 100644 --- a/test/cmd_line/substitute.test.ts +++ b/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,105 +336,73 @@ suite('Basic substitute', () => { assertEqualLines(['fighters', 'bar', 'fighters', 'bar']); }); - test('Substitute with parameters should update search state', async () => { - await modeHandler.handleMultipleKeyEvents([ - 'i', // foo bar foo bar - 'f', - 'o', - 'o', - '', - 'o', - 'b', - 'a', - 'r', - '', - 'o', - 'f', - 'o', - 'o', - '', - 'o', - 'b', - 'a', - 'r', - '', - '/', // search for bar - 'b', - 'a', - 'r', - '\n', - ]); - await commandLine.Run('s/ar/ite', modeHandler.vimState); // first bar to bite - await modeHandler.handleMultipleKeyEvents([ - 'n', // repeat search for ar, not bar - 'r', // replace bar with brr - 'r', - ]); - - assertEqualLines(['foo', 'bite', 'foo', 'brr']); + 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'], }); - test('Substitute with no pattern should repeat previous substitution and not update search state', async () => { - await modeHandler.handleMultipleKeyEvents([ - 'i', // link zelda link zelda link - 'l', - 'i', - 'n', - 'k', - '', - 'o', - 'z', - 'e', - 'l', - 'd', - 'a', - '', - 'o', - 'l', - 'i', - 'n', - 'k', - '', - 'o', - 'z', - 'e', - 'l', - 'd', - 'a', - '', - 'o', - 'l', - 'i', - 'n', - 'k', - '', - '1', // go to the first line - 'g', - 'g', - ]); - await commandLine.Run('s/ink/egend', modeHandler.vimState); // replace first ink with egend - await modeHandler.handleMultipleKeyEvents([ - '/', - 'l', - 'i', - 'n', - 'k', // search for full link - '\n', - ]); - await commandLine.Run('s', modeHandler.vimState); // repeat previous replacement of ink, not link! - await modeHandler.handleMultipleKeyEvents([ - 'n', // search for link, not ink - 'r', - 'p', // should be pink now - ]); - - assertEqualLines(['legend', 'zelda', 'legend', 'zelda', 'pink']); + 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', + ], }); - test('Substitute repeat previous should accept flags', async () => { - await modeHandler.handleMultipleKeyEvents(['i', 'f', 'o', 'o', 'o', '']); - await commandLine.Run('s/o/un', modeHandler.vimState); // replace first o with un - await commandLine.Run('s g', modeHandler.vimState); // repeat replacement on all following o's - - assertEqualLines(['fununun']); + 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([ From e3bbfd0d0acfa004c0b30869840042395adb576a Mon Sep 17 00:00:00 2001 From: Captain Caius <241342+captaincaius@users.noreply.github.com> Date: Fri, 28 Sep 2018 17:39:26 +0300 Subject: [PATCH 3/3] Added comments to explain all the corner cases involving find/sub states --- src/cmd_line/commands/substitute.ts | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/cmd_line/commands/substitute.ts b/src/cmd_line/commands/substitute.ts index 6ccf76e0099..0da6778b760 100644 --- a/src/cmd_line/commands/substitute.ts +++ b/src/cmd_line/commands/substitute.ts @@ -13,6 +13,12 @@ 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 | undefined; replace: string; @@ -52,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;