diff --git a/src/cmd_line/commands/substitute.ts b/src/cmd_line/commands/substitute.ts index 7b8b9c81e..be60c2904 100644 --- a/src/cmd_line/commands/substitute.ts +++ b/src/cmd_line/commands/substitute.ts @@ -215,7 +215,7 @@ export class SubstituteCommand extends ExCommand { return !this.arguments.flags.confirmEach; } - private getRegex(args: ISubstituteCommandArguments, vimState: VimState) { + private getRegex(args: ISubstituteCommandArguments, vimState: VimState): RegExp | undefined { let jsRegexFlags = ''; if (configuration.gdefault || configuration.substituteGlobalFlag) { // the gdefault flag is on, then /g if on by default and /g negates that @@ -238,7 +238,7 @@ export class SubstituteCommand extends ExCommand { // i.e. :s const prevSubstituteState = globalState.substituteState; if ( - prevSubstituteState === undefined || + prevSubstituteState?.searchPattern === undefined || prevSubstituteState.searchPattern.patternString === '' ) { throw VimError.fromCode(ErrorCode.NoPreviousSubstituteRegularExpression); @@ -266,7 +266,7 @@ export class SubstituteCommand extends ExCommand { vimState.currentMode ); } - return new RegExp(args.pattern.regex.source, jsRegexFlags); + return args.pattern ? new RegExp(args.pattern.regex.source, jsRegexFlags) : undefined; } /** @@ -396,6 +396,10 @@ export class SubstituteCommand extends ExCommand { async execute(vimState: VimState): Promise { const regex = this.getRegex(this.arguments, vimState); + if (regex === undefined) { + return; + } + const selection = vimState.editor.selection; const line = selection.start.isBefore(selection.end) ? selection.start.line @@ -418,6 +422,10 @@ export class SubstituteCommand extends ExCommand { // TODO: Global Setting. // TODO: There are differencies between Vim Regex and JS Regex. const regex = this.getRegex(this.arguments, vimState); + if (regex === undefined) { + return; + } + let lines = 0; let substitutions = 0; for ( diff --git a/src/state/searchState.ts b/src/state/searchState.ts index e9f8a5a4d..6d90171f8 100644 --- a/src/state/searchState.ts +++ b/src/state/searchState.ts @@ -17,9 +17,10 @@ export class SearchState { ) { this._searchString = searchString; - const { pattern, offset } = searchStringParser({ direction, ignoreSmartcase }).tryParse( - searchString - ); + const result = searchStringParser({ direction, ignoreSmartcase }).parse(searchString); + const { pattern, offset } = result.status + ? result.value + : { pattern: undefined, offset: undefined }; this.pattern = pattern; this.offset = offset; @@ -29,7 +30,7 @@ export class SearchState { } private _searchString: string; - public pattern: Pattern; + public pattern?: Pattern; private offset?: SearchOffset; public readonly previousMode: Mode; @@ -40,11 +41,14 @@ export class SearchState { } public set searchString(str: string) { this._searchString = str; - const { pattern, offset } = searchStringParser({ - direction: this.pattern.direction, + const result = searchStringParser({ + direction: this.direction, ignoreSmartcase: this.ignoreSmartcase, - }).tryParse(str); - if (pattern.patternString !== this.pattern.patternString) { + }).parse(str); + const { pattern, offset } = result.status + ? result.value + : { pattern: undefined, offset: undefined }; + if (pattern?.patternString !== this.pattern?.patternString) { this.pattern = pattern; this.matchRanges.clear(); } @@ -52,7 +56,8 @@ export class SearchState { } public get direction(): SearchDirection { - return this.pattern.direction; + // TODO: Defaulting to forward is wrong - I think storing the direction in the pattern is a mistake + return this.pattern?.direction ?? SearchDirection.Forward; } /** @@ -72,7 +77,7 @@ export class SearchState { private readonly ignoreSmartcase: boolean; private recalculateSearchRanges(editor: TextEditor): Range[] { - if (this.searchString === '') { + if (this.searchString === '' || this.pattern === undefined) { return []; } @@ -129,7 +134,7 @@ export class SearchState { return undefined; } - const effectiveDirection = (direction * this.pattern.direction) as SearchDirection; + const effectiveDirection = (direction * this.direction) as SearchDirection; if (effectiveDirection === SearchDirection.Forward) { for (const [index, range] of matchRanges.entries()) { diff --git a/src/state/substituteState.ts b/src/state/substituteState.ts index 76a6d767e..2df9a1bb0 100644 --- a/src/state/substituteState.ts +++ b/src/state/substituteState.ts @@ -7,14 +7,14 @@ export class SubstituteState { /** * The last pattern searched for in the substitution */ - public searchPattern: Pattern; + public searchPattern: Pattern | undefined; /** * The last replacement string in the substitution */ public replaceString: string; - constructor(searchPattern: Pattern, replaceString: string) { + constructor(searchPattern: Pattern | undefined, replaceString: string) { this.searchPattern = searchPattern; this.replaceString = replaceString; } diff --git a/src/vimscript/lineRange.ts b/src/vimscript/lineRange.ts index 2102806b3..e8015160b 100644 --- a/src/vimscript/lineRange.ts +++ b/src/vimscript/lineRange.ts @@ -206,20 +206,22 @@ export class Address { if (!globalState.substituteState) { throw VimError.fromCode(ErrorCode.NoPreviousSubstituteRegularExpression); } - const searchState = new SearchState( - SearchDirection.Forward, - vimState.cursorStopPosition, - globalState.substituteState.searchPattern.patternString, - {}, - vimState.currentMode - ); - const match = searchState.getNextSearchMatchPosition( + const searchState = globalState.substituteState.searchPattern + ? new SearchState( + SearchDirection.Forward, + vimState.cursorStopPosition, + globalState.substituteState.searchPattern.patternString, + {}, + vimState.currentMode + ) + : undefined; + const match = searchState?.getNextSearchMatchPosition( vimState.editor, vimState.cursorStopPosition ); if (match === undefined) { // TODO: throw proper errors for nowrapscan - throw VimError.fromCode(ErrorCode.PatternNotFound, searchState.searchString); + throw VimError.fromCode(ErrorCode.PatternNotFound, searchState?.searchString); } return match.pos.line; default: diff --git a/src/vimscript/pattern.ts b/src/vimscript/pattern.ts index 3e78403b2..18a378479 100644 --- a/src/vimscript/pattern.ts +++ b/src/vimscript/pattern.ts @@ -38,6 +38,7 @@ export class Pattern { public readonly ignorecase: boolean | undefined; private static readonly MAX_SEARCH_RANGES = 1000; + private static readonly SPECIAL_CHARS_REGEX = /[\-\[\]{}()*+?.,\\\^$|#\s]/g; public nextMatch(document: TextDocument, fromPosition: Position): Range | undefined { const haystack = document.getText(); @@ -103,6 +104,16 @@ export class Pattern { return matchRanges.afterWrapping.concat(matchRanges.beforeWrapping); } + private static compileRegex(regexString: string, ignoreCase?: boolean): RegExp { + const flags = ignoreCase ?? configuration.ignorecase ? 'gim' : 'gm'; + try { + return new RegExp(regexString, flags); + } catch (err) { + // Couldn't compile the regexp, try again with special characters escaped + return new RegExp(regexString.replace(Pattern.SPECIAL_CHARS_REGEX, '\\$&'), flags); + } + } + public static fromLiteralString( input: string, direction: SearchDirection, @@ -110,17 +121,9 @@ export class Pattern { ): Pattern { const patternString = input.replace(escapeRegExp(input), '\\$&'); if (wordBoundaries) { - return new Pattern( - `\\<${patternString}\\>`, - direction, - new RegExp(`\b${patternString}\b`, configuration.ignorecase ? 'gim' : 'gm') - ); + return new Pattern(`\\<${patternString}\\>`, direction, Pattern.compileRegex(patternString)); } else { - return new Pattern( - patternString, - direction, - new RegExp(patternString, configuration.ignorecase ? 'gim' : 'gm') - ); + return new Pattern(patternString, direction, Pattern.compileRegex(patternString)); } } @@ -177,13 +180,15 @@ export class Pattern { }; }) .map(({ patternString, caseOverride }) => { - const flags = Pattern.getIgnoreCase(patternString, { + const ignoreCase = Pattern.getIgnoreCase(patternString, { caseOverride, ignoreSmartcase: args.ignoreSmartcase ?? false, - }) - ? 'gim' - : 'gm'; - return new Pattern(patternString, args.direction, RegExp(patternString, flags)); + }); + return new Pattern( + patternString, + args.direction, + Pattern.compileRegex(patternString, ignoreCase) + ); }); } diff --git a/test/mode/modeNormal.test.ts b/test/mode/modeNormal.test.ts index 9b59c0cb4..34cfb193c 100755 --- a/test/mode/modeNormal.test.ts +++ b/test/mode/modeNormal.test.ts @@ -2321,6 +2321,13 @@ suite('Mode Normal', () => { end: ['|x end', 'x', 'x', 'start'], }); + newTest({ + title: 'Search for `(`', + start: ['|one (two) three'], + keysPressed: '/(\n', + end: ['one |(two) three'], + }); + /** * The escaped `/` and `?` the next tests are necessary because otherwise they denote a search offset. */