From d16d44f2eab85d11ff9764e88171c67baaa4b532 Mon Sep 17 00:00:00 2001 From: Captain Caius <241342+captaincaius@users.noreply.github.com> Date: Sun, 23 Sep 2018 19:34:02 +0300 Subject: [PATCH 1/5] feature: relative, plus/minus ranges. closes #2384 --- src/actions/commands/actions.ts | 2 +- src/cmd_line/commands/substitute.ts | 6 +- src/cmd_line/node.ts | 137 ++++++++++++++++++++++++---- src/cmd_line/parser.ts | 2 + test/cmd_line/substitute.test.ts | 62 ++++++++++++- 5 files changed, 186 insertions(+), 23 deletions(-) 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..36d1f456e4a 100644 --- a/src/cmd_line/commands/substitute.ts +++ b/src/cmd_line/commands/substitute.ts @@ -221,7 +221,11 @@ export class SubstituteCommand extends node.CommandBase { endLine = new vscode.Position(TextEditor.getLineCount() - 1, 0); } else { startLine = range.lineRefToPosition(vimState.editor, range.left, vimState); - endLine = range.lineRefToPosition(vimState.editor, range.right, vimState); + if (range.right.length === 0) { + endLine = startLine; + } else { + endLine = range.lineRefToPosition(vimState.editor, range.right, vimState); + } } if (this._arguments.count && this._arguments.count >= 0) { diff --git a/src/cmd_line/node.ts b/src/cmd_line/node.ts index 1aadeab21ea..7e2e48c396c 100644 --- a/src/cmd_line/node.ts +++ b/src/cmd_line/node.ts @@ -3,6 +3,8 @@ import * as vscode from 'vscode'; import { VimState } from '../state/vimState'; import * as token from './token'; +type LineRefOperation = token.TokenType.Plus | token.TokenType.Minus | undefined; + export class LineRange { left: token.Token[]; separator: token.Token; @@ -20,15 +22,29 @@ export class LineRange { } if (!this.separator) { - if (this.left.length > 0 && tok.type !== token.TokenType.Offset) { - // XXX: is this always this error? - throw Error('not a Vim command'); + if (this.left.length > 0) { + switch (tok.type) { + case token.TokenType.Offset: + case token.TokenType.LineNumber: + case token.TokenType.Plus: + case token.TokenType.Minus: + break; + default: + throw Error('Trailing characters'); + } } this.left.push(tok); } else { - if (this.right.length > 0 && tok.type !== token.TokenType.Offset) { - // XXX: is this always this error? - throw Error('not a Vim command'); + if (this.right.length > 0) { + switch (tok.type) { + case token.TokenType.Offset: + case token.TokenType.LineNumber: + case token.TokenType.Plus: + case token.TokenType.Minus: + break; + default: + throw Error('Trailing characters'); + } } this.right.push(tok); } @@ -57,20 +73,30 @@ export class LineRange { toks: token.Token[], vimState: VimState ): vscode.Position { - var first = toks[0]; - switch (first.type) { - case token.TokenType.Dollar: + let currentLineNum: number; + let currentColumn = 0; // only mark does this differently + let currentOperation: LineRefOperation = undefined; + + var firstToken = toks[0]; + // handle first-token special cases (e.g. %, inital line number is "." by default) + switch (firstToken.type) { case token.TokenType.Percent: return new vscode.Position(doc.document.lineCount - 1, 0); + case token.TokenType.Dollar: + currentLineNum = doc.document.lineCount - 1; + break; + case token.TokenType.Plus: + case token.TokenType.Minus: case token.TokenType.Dot: - return new vscode.Position(doc.selection.active.line, 0); + currentLineNum = doc.selection.active.line; + // undocumented: if the first token is plus or minus, vim seems to behave as though there was a "." + currentOperation = firstToken.type === token.TokenType.Dot ? undefined : firstToken.type; + break; case token.TokenType.LineNumber: - var line = Number.parseInt(first.content, 10); - line = Math.max(0, line - 1); - line = Math.min(doc.document.lineCount, line); - return new vscode.Position(line, 0); + currentLineNum = Number.parseInt(firstToken.content, 10) - 1; // user sees 1-based - everything else is 0-based + break; case token.TokenType.SelectionFirstLine: - let startLine = Math.min.apply( + currentLineNum = Math.min.apply( null, doc.selections.map( selection => @@ -79,21 +105,94 @@ export class LineRange { : selection.end.line ) ); - return new vscode.Position(startLine, 0); + break; case token.TokenType.SelectionLastLine: - let endLine = Math.max.apply( + currentLineNum = Math.max.apply( null, doc.selections.map( selection => selection.start.isAfter(selection.end) ? selection.start.line : selection.end.line ) ); - return new vscode.Position(endLine, 0); + break; case token.TokenType.Mark: - return vimState.historyTracker.getMark(first.content).position; + currentLineNum = vimState.historyTracker.getMark(firstToken.content).position.line; + currentColumn = vimState.historyTracker.getMark(firstToken.content).position.character; + break; default: throw new Error('Not Implemented'); } + + // now handle subsequent tokens, offsetting the current candidate line number + for (let tokenIndex = 1; tokenIndex < toks.length; ++tokenIndex) { + let currentToken = toks[tokenIndex]; + + switch (currentOperation) { + case token.TokenType.Plus: + switch (currentToken.type) { + case token.TokenType.Minus: + case token.TokenType.Plus: + // undocumented: when there's two operators in a row, vim behaves as though there's a "1" between them + currentLineNum += 1; + currentColumn = 0; + currentOperation = currentToken.type; + break; + case token.TokenType.LineNumber: + currentLineNum += Number.parseInt(currentToken.content, 10); + currentColumn = 0; + currentOperation = undefined; + break; + default: + throw Error('Trailing characters'); + } + break; + case token.TokenType.Minus: + switch (currentToken.type) { + case token.TokenType.Minus: + case token.TokenType.Plus: + // undocumented: when there's two operators in a row, vim behaves as though there's a "1" between them + currentLineNum -= 1; + currentColumn = 0; + currentOperation = currentToken.type; + break; + case token.TokenType.LineNumber: + currentLineNum -= Number.parseInt(currentToken.content, 10); + currentColumn = 0; + currentOperation = undefined; + break; + default: + throw Error('Trailing characters'); + } + break; + case undefined: + switch (currentToken.type) { + case token.TokenType.Minus: + case token.TokenType.Plus: + currentOperation = currentToken.type; + break; + default: + throw Error('Trailing characters'); + } + break; + } + } + + // undocumented: when there's a trailing operation in the tank without an RHS, vim uses "1" + switch (currentOperation) { + case token.TokenType.Plus: + currentLineNum += 1; + currentColumn = 0; + break; + case token.TokenType.Minus: + currentLineNum -= 1; + currentColumn = 0; + break; + } + + // finally, make sure current position is in bounds :) + currentLineNum = Math.max(0, currentLineNum); + currentLineNum = Math.min(doc.document.lineCount - 1, currentLineNum); + return new vscode.Position(currentLineNum, currentColumn); } } diff --git a/src/cmd_line/parser.ts b/src/cmd_line/parser.ts index 9cd400783d3..6577a86c8c7 100644 --- a/src/cmd_line/parser.ts +++ b/src/cmd_line/parser.ts @@ -32,6 +32,8 @@ function parseLineRange(state: ParserState, commandLine: node.CommandLine): IPar case token.TokenType.SelectionFirstLine: case token.TokenType.SelectionLastLine: case token.TokenType.Mark: + case token.TokenType.Plus: + case token.TokenType.Minus: commandLine.range.addToken(tok); continue; case token.TokenType.CommandName: diff --git a/test/cmd_line/substitute.test.ts b/test/cmd_line/substitute.test.ts index 070df112ddd..4165b9393b5 100644 --- a/test/cmd_line/substitute.test.ts +++ b/test/cmd_line/substitute.test.ts @@ -8,8 +8,10 @@ import { reloadConfiguration, setupWorkspace, } from './../testUtils'; +import { getTestingFunctions } from '../testSimplifier'; suite('Basic substitute', () => { + let { newTest, newTestOnly } = getTestingFunctions(); let modeHandler: ModeHandler; setup(async () => { @@ -33,14 +35,70 @@ suite('Basic substitute', () => { assertEqualLines(['dbd']); }); - test('Replace multiple lines', async () => { + test('Replace across all lines', async () => { await modeHandler.handleMultipleKeyEvents(['i', 'a', 'b', 'a', '', 'o', 'a', 'b']); await commandLine.Run('%s/a/d/g', modeHandler.vimState); assertEqualLines(['dbd', 'db']); }); - test('Replace across specific lines', async () => { + newTest({ + title: 'Replace on specific single line', + start: ['blah blah', 'bla|h', 'blah blah', 'blah blah'], + keysPressed: ':3s/blah/yay\n', + end: ['blah blah', 'bla|h', 'yay blah', 'blah blah'], + }); + + newTest({ + title: 'Replace on current line using dot', + start: ['blah blah', '|blah', 'blah blah', 'blah blah'], + keysPressed: ':.s/blah/yay\n', + end: ['blah blah', '|yay', 'blah blah', 'blah blah'], + }); + + newTest({ + title: 'Replace single relative line using dot and plus', + start: ['blah blah', 'bla|h', 'blah blah', 'blah blah'], + keysPressed: ':.+2s/blah/yay\n', + end: ['blah blah', 'bla|h', 'blah blah', 'yay blah'], + }); + + newTest({ + title: 'Replace across specific line range', + start: ['blah blah', 'bla|h', 'blah blah', 'blah blah'], + keysPressed: ':3,4s/blah/yay\n', + end: ['blah blah', 'bla|h', 'yay blah', 'yay blah'], + }); + + newTest({ + title: 'Replace across relative line range using dot, plus, and minus', + start: ['blah blah', '|blah', 'blah blah', 'blah blah'], + keysPressed: ':.-1,.+1s/blah/yay\n', + end: ['yay blah', '|yay', 'yay blah', 'blah blah'], + }); + + newTest({ + title: 'Undocumented: operator without LHS assumes dot as LHS', + start: ['blah blah', 'bla|h', 'blah blah', 'blah blah'], + keysPressed: ':+2s/blah/yay\n', + end: ['blah blah', 'bla|h', 'blah blah', 'yay blah'], + }); + + newTest({ + title: 'Undocumented: multiple consecutive operators use 1 as RHS', + start: ['blah blah', 'bla|h', 'blah blah', 'blah blah'], + keysPressed: ':.++1s/blah/yay\n', + end: ['blah blah', 'bla|h', 'blah blah', 'yay blah'], + }); + + newTest({ + title: 'Undocumented: trailing operators use 1 as RHS', + start: ['blah blah', 'bla|h', 'blah blah', 'blah blah'], + keysPressed: ':.+1+s/blah/yay\n', + end: ['blah blah', 'bla|h', 'blah blah', 'yay blah'], + }); + + test('Replace specific single equal lines', async () => { await modeHandler.handleMultipleKeyEvents(['i', 'a', 'b', 'a', '', 'o', 'a', 'b']); await commandLine.Run('1,1s/a/d/g', modeHandler.vimState); From 1bf8bb14e3f39ee6bfed53e9fa01f4fe0cea4bc0 Mon Sep 17 00:00:00 2001 From: Captain Caius <241342+captaincaius@users.noreply.github.com> Date: Tue, 2 Oct 2018 13:44:32 +0300 Subject: [PATCH 2/5] command lexer refactor so LineNumber and Offset are distinct It seems more in-line with the original direction the code was going It's a little more code, but I think easier to understand for a first-time reader this way. --- src/cmd_line/lexer.ts | 71 ++++++++++++++++++++++++++---------------- src/cmd_line/node.ts | 6 ++-- src/cmd_line/parser.ts | 1 + 3 files changed, 48 insertions(+), 30 deletions(-) diff --git a/src/cmd_line/lexer.ts b/src/cmd_line/lexer.ts index fb6c5527022..fcfdba9375f 100644 --- a/src/cmd_line/lexer.ts +++ b/src/cmd_line/lexer.ts @@ -60,7 +60,18 @@ namespace LexerFunctions { case '7': case '8': case '9': - return lexLineRef; + if (tokens.length < 1) { + // special case - first digitey token is always a line number + return lexDigits(TokenType.LineNumber); + } else { + // otherwise, use previous token to determine which flavor of digit lexer should be used + let previousTokenType = tokens[tokens.length - 1].type; + if (previousTokenType === TokenType.Plus || previousTokenType === TokenType.Minus) { + return lexDigits(TokenType.Offset); + } else { + return lexDigits(TokenType.LineNumber); + } + } case '+': tokens.push(emitToken(TokenType.Plus, state)!); continue; @@ -110,33 +121,41 @@ namespace LexerFunctions { return lexRange; } - function lexLineRef(state: Scanner, tokens: Token[]): ILexFunction | null { - // The first digit has already been lexed. - while (true) { - if (state.isAtEof) { - tokens.push(emitToken(TokenType.LineNumber, state)!); - return null; - } + /** + * when we're lexing digits, it could either be a line number or an offset, depending on whether + * our previous token was a + or a - + * + * so it's lexRange's job to specify which token to emit. + */ + function lexDigits(tokenType: TokenType) { + return function(state: Scanner, tokens: Token[]): ILexFunction | null { + // The first digit has already been lexed. + while (true) { + if (state.isAtEof) { + tokens.push(emitToken(tokenType, state)!); + return null; + } - var c = state.next(); - switch (c) { - case '0': - case '1': - case '2': - case '3': - case '4': - case '5': - case '6': - case '7': - case '8': - case '9': - continue; - default: - state.backup(); - tokens.push(emitToken(TokenType.LineNumber, state)!); - return lexRange; + var c = state.next(); + switch (c) { + case '0': + case '1': + case '2': + case '3': + case '4': + case '5': + case '6': + case '7': + case '8': + case '9': + continue; + default: + state.backup(); + tokens.push(emitToken(tokenType, state)!); + return lexRange; + } } - } + }; } function lexCommand(state: Scanner, tokens: Token[]): ILexFunction | null { diff --git a/src/cmd_line/node.ts b/src/cmd_line/node.ts index 7e2e48c396c..f7978adeba0 100644 --- a/src/cmd_line/node.ts +++ b/src/cmd_line/node.ts @@ -25,7 +25,6 @@ export class LineRange { if (this.left.length > 0) { switch (tok.type) { case token.TokenType.Offset: - case token.TokenType.LineNumber: case token.TokenType.Plus: case token.TokenType.Minus: break; @@ -38,7 +37,6 @@ export class LineRange { if (this.right.length > 0) { switch (tok.type) { case token.TokenType.Offset: - case token.TokenType.LineNumber: case token.TokenType.Plus: case token.TokenType.Minus: break; @@ -137,7 +135,7 @@ export class LineRange { currentColumn = 0; currentOperation = currentToken.type; break; - case token.TokenType.LineNumber: + case token.TokenType.Offset: currentLineNum += Number.parseInt(currentToken.content, 10); currentColumn = 0; currentOperation = undefined; @@ -155,7 +153,7 @@ export class LineRange { currentColumn = 0; currentOperation = currentToken.type; break; - case token.TokenType.LineNumber: + case token.TokenType.Offset: currentLineNum -= Number.parseInt(currentToken.content, 10); currentColumn = 0; currentOperation = undefined; diff --git a/src/cmd_line/parser.ts b/src/cmd_line/parser.ts index 6577a86c8c7..e7b1fa68a38 100644 --- a/src/cmd_line/parser.ts +++ b/src/cmd_line/parser.ts @@ -32,6 +32,7 @@ function parseLineRange(state: ParserState, commandLine: node.CommandLine): IPar case token.TokenType.SelectionFirstLine: case token.TokenType.SelectionLastLine: case token.TokenType.Mark: + case token.TokenType.Offset: case token.TokenType.Plus: case token.TokenType.Minus: commandLine.range.addToken(tok); From e1bc34aefd05f21d8dcde336209a0f2562571246 Mon Sep 17 00:00:00 2001 From: Captain Caius <241342+captaincaius@users.noreply.github.com> Date: Tue, 2 Oct 2018 14:59:54 +0300 Subject: [PATCH 3/5] Dependent feature added: numLines-colon shorthand --- src/actions/commands/actions.ts | 6 +++++- test/cmd_line/substitute.test.ts | 7 +++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/actions/commands/actions.ts b/src/actions/commands/actions.ts index c998580df32..b8e4ff7b038 100644 --- a/src/actions/commands/actions.ts +++ b/src/actions/commands/actions.ts @@ -1772,7 +1772,11 @@ class CommandShowCommandLine extends BaseCommand { public async exec(position: Position, vimState: VimState): Promise { if (vimState.currentMode === ModeName.Normal) { - vimState.currentCommandlineText = ''; + if (vimState.recordedState.count) { + vimState.currentCommandlineText = `.,.+${vimState.recordedState.count - 1}`; + } else { + vimState.currentCommandlineText = ''; + } } else { vimState.currentCommandlineText = "'<,'>"; } diff --git a/test/cmd_line/substitute.test.ts b/test/cmd_line/substitute.test.ts index 72c321acd00..7977bc8df28 100644 --- a/test/cmd_line/substitute.test.ts +++ b/test/cmd_line/substitute.test.ts @@ -97,6 +97,13 @@ suite('Basic substitute', () => { end: ['yay blah', '|yay', 'yay blah', 'blah blah'], }); + newTest({ + title: 'Replace across relative line range using numLines+colon shorthand', + start: ['blah blah', '|blah', 'blah blah', 'blah blah'], + keysPressed: '3:s/blah/yay\n', + end: ['blah blah', '|yay', 'yay blah', 'yay blah'], + }); + newTest({ title: 'Undocumented: operator without LHS assumes dot as LHS', start: ['blah blah', 'bla|h', 'blah blah', 'blah blah'], From e07f0b20266d5888198ce696ffe7d5d6de64184d Mon Sep 17 00:00:00 2001 From: Captain Caius <241342+captaincaius@users.noreply.github.com> Date: Thu, 11 Oct 2018 12:39:30 +0300 Subject: [PATCH 4/5] lets to consts --- src/cmd_line/lexer.ts | 2 +- src/cmd_line/node.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmd_line/lexer.ts b/src/cmd_line/lexer.ts index fcfdba9375f..5c8d281d255 100644 --- a/src/cmd_line/lexer.ts +++ b/src/cmd_line/lexer.ts @@ -65,7 +65,7 @@ namespace LexerFunctions { return lexDigits(TokenType.LineNumber); } else { // otherwise, use previous token to determine which flavor of digit lexer should be used - let previousTokenType = tokens[tokens.length - 1].type; + const previousTokenType = tokens[tokens.length - 1].type; if (previousTokenType === TokenType.Plus || previousTokenType === TokenType.Minus) { return lexDigits(TokenType.Offset); } else { diff --git a/src/cmd_line/node.ts b/src/cmd_line/node.ts index f7978adeba0..b9956e7f65d 100644 --- a/src/cmd_line/node.ts +++ b/src/cmd_line/node.ts @@ -75,7 +75,7 @@ export class LineRange { let currentColumn = 0; // only mark does this differently let currentOperation: LineRefOperation = undefined; - var firstToken = toks[0]; + const firstToken = toks[0]; // handle first-token special cases (e.g. %, inital line number is "." by default) switch (firstToken.type) { case token.TokenType.Percent: From e742c57f01b5814ce9cc11d19255ae2a70850a07 Mon Sep 17 00:00:00 2001 From: Captain Caius <241342+captaincaius@users.noreply.github.com> Date: Thu, 11 Oct 2018 12:40:38 +0300 Subject: [PATCH 5/5] fix - exotic count-related bugs along with tests --- src/cmd_line/subparsers/substitute.ts | 6 ++---- test/cmd_line/substitute.test.ts | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/cmd_line/subparsers/substitute.ts b/src/cmd_line/subparsers/substitute.ts index c4999cc0f96..e8901fad128 100644 --- a/src/cmd_line/subparsers/substitute.ts +++ b/src/cmd_line/subparsers/substitute.ts @@ -96,7 +96,8 @@ function parseSubstituteFlags(scanner: Scanner): number { flags = flags | node.SubstituteFlags.UsePreviousPattern; break; default: - return node.SubstituteFlags.None; + scanner.backup(); + return flags; } index++; @@ -139,7 +140,6 @@ export function parseSubstituteCommandArgs(args: string): node.SubstituteCommand pattern: undefined, replace: '', // ignored in this context flags: node.SubstituteFlags.None, - count: 1, }); } let scanner: Scanner; @@ -153,7 +153,6 @@ export function parseSubstituteCommandArgs(args: string): node.SubstituteCommand pattern: '', replace: '', flags: node.SubstituteFlags.None, - count: 1, }); } @@ -168,7 +167,6 @@ export function parseSubstituteCommandArgs(args: string): node.SubstituteCommand pattern: searchPattern, replace: '', flags: node.SubstituteFlags.None, - count: 1, }); } replaceString = parsePattern('', scanner, delimiter)[0]; diff --git a/test/cmd_line/substitute.test.ts b/test/cmd_line/substitute.test.ts index 3dff22bbe26..bbafea2d7cd 100644 --- a/test/cmd_line/substitute.test.ts +++ b/test/cmd_line/substitute.test.ts @@ -37,6 +37,13 @@ suite('Basic substitute', () => { assertEqualLines(['dbd']); }); + newTest({ + title: 'Replace with flags AND count', + start: ['|blah blah', 'blah', 'blah blah', 'blah blah'], + keysPressed: ':.s/blah/yay/g 2\n', + end: ['|yay yay', 'yay', 'blah blah', 'blah blah'], + }); + test('Replace with `c` flag', async () => { const confirmStub = sinon.stub(SubstituteCommand.prototype, 'confirmReplacement').returns(true); await modeHandler.handleMultipleKeyEvents(['i', 'a', 'b', 'a', '']); @@ -104,6 +111,20 @@ suite('Basic substitute', () => { end: ['blah blah', '|yay', 'yay blah', 'yay blah'], }); + newTest({ + title: 'Repeat replacement across relative line range', + start: ['|blah blah', 'blah', 'blah blah', 'blah blah'], + keysPressed: ':s/blah/yay\nj3:s\n', + end: ['yay blah', '|yay', 'yay blah', 'yay blah'], + }); + + newTest({ + title: 'Replace with range AND count but no flags', + start: ['|blah blah', 'blah', 'blah blah', 'blah blah'], + keysPressed: '3:s/blah/yay/ 2\n', + end: ['|blah blah', 'blah', 'yay blah', 'yay blah'], + }); + newTest({ title: 'Undocumented: operator without LHS assumes dot as LHS', start: ['blah blah', 'bla|h', 'blah blah', 'blah blah'],