From 046794d4cc8bb817078e983de5a4fb0f23df5926 Mon Sep 17 00:00:00 2001 From: Alison Winters Date: Tue, 28 Jan 2020 16:57:55 +0800 Subject: [PATCH 1/8] make motion applications async --- src/Actions/Delete.ts | 62 +++++++++++++++++++++------------------ src/Actions/MoveCursor.ts | 25 ++++++++-------- src/Actions/Register.ts | 27 +++++++++-------- src/Motions/Direction.ts | 4 +-- src/Motions/Document.ts | 4 +-- src/Motions/Line.ts | 4 +-- src/Motions/Match.ts | 4 +-- src/Motions/MatchPair.ts | 6 ++-- src/Motions/Motion.ts | 2 +- src/Motions/Paragraph.ts | 4 +-- src/Motions/Word.ts | 6 ++-- 11 files changed, 78 insertions(+), 70 deletions(-) diff --git a/src/Actions/Delete.ts b/src/Actions/Delete.ts index 2c5b0088..9c76a4de 100644 --- a/src/Actions/Delete.ts +++ b/src/Actions/Delete.ts @@ -26,40 +26,44 @@ export class ActionDelete { const document = activeTextEditor.document; - let ranges = activeTextEditor.selections.map((selection) => { + const promisedRanges = activeTextEditor.selections.map(async (selection) => { const start = selection.active; - const end = args.motions.reduce((position, motion) => { - return motion.apply(position, { - isInclusive: true, - shouldCrossLines: false, - isChangeAction: args.isChangeAction, - }); - }, start); + const end = await args.motions.reduce((promisedPosition, motion) => { + return promisedPosition.then((position) => + motion.apply(position, { + isInclusive: true, + shouldCrossLines: false, + isChangeAction: args.isChangeAction, + }), + ); + }, Promise.resolve(start)); return new Range(start, end); }); - if (args.motions.some((motion) => motion.isLinewise)) { - ranges = ranges.map((range) => UtilRange.toLinewise(range, document)); - } - - ranges = UtilRange.unionOverlaps(ranges); - - // TODO: Move cursor to first non-space if needed + return Promise.all(promisedRanges).then((ranges) => { + if (args.motions.some((motion) => motion.isLinewise)) { + ranges = ranges.map((range) => UtilRange.toLinewise(range, document)); + } - return (args.shouldYank - ? ActionRegister.yankByMotions({ - motions: args.motions, - isChangeAction: args.isChangeAction, - }) - : Promise.resolve(true) - ) - .then(() => { - return activeTextEditor.edit((editBuilder) => { - ranges.forEach((range) => editBuilder.delete(range)); - }); - }) - .then(() => ActionSelection.shrinkToStarts()) - .then(() => ActionReveal.primaryCursor()); + ranges = UtilRange.unionOverlaps(ranges); + + // TODO: Move cursor to first non-space if needed + + return (args.shouldYank + ? ActionRegister.yankByMotions({ + motions: args.motions, + isChangeAction: args.isChangeAction, + }) + : Promise.resolve(true) + ) + .then(() => { + return activeTextEditor.edit((editBuilder) => { + ranges.forEach((range) => editBuilder.delete(range)); + }); + }) + .then(() => ActionSelection.shrinkToStarts()) + .then(() => ActionReveal.primaryCursor()); + }); } @StaticReflect.metadata(SymbolMetadata.Action.isChange, true) diff --git a/src/Actions/MoveCursor.ts b/src/Actions/MoveCursor.ts index 1c2b30a3..0d892dbd 100644 --- a/src/Actions/MoveCursor.ts +++ b/src/Actions/MoveCursor.ts @@ -65,19 +65,16 @@ export class ActionMoveCursor { const document = activeTextEditor.document; - activeTextEditor.selections = activeTextEditor.selections.map((selection, i) => { + const promisedSelections = activeTextEditor.selections.map(async (selection, i) => { let anchor: Position; - let active = args.motions.reduce( - (position, motion) => { - return motion.apply(position, { + let active = await args.motions.reduce((promisedPosition, motion) => { + return promisedPosition.then((position) => + motion.apply(position, { preferredColumn: ActionMoveCursor.preferredColumnBySelectionIndex[i], - }); - }, - args.isVisualMode - ? UtilSelection.getActiveInVisualMode(selection) - : selection.active, - ); + }), + ); + }, Promise.resolve(args.isVisualMode ? UtilSelection.getActiveInVisualMode(selection) : selection.active)); if (args.isVisualMode) { anchor = selection.anchor; @@ -129,7 +126,11 @@ export class ActionMoveCursor { return new Selection(anchor, active); }); - - return ActionReveal.primaryCursor(); + return Promise.all(promisedSelections) + .then((selections) => { + activeTextEditor.selections = selections; + return Promise.resolve(true); + }) + .then(() => ActionReveal.primaryCursor()); } } diff --git a/src/Actions/Register.ts b/src/Actions/Register.ts index d0725b1b..bf71795e 100644 --- a/src/Actions/Register.ts +++ b/src/Actions/Register.ts @@ -83,21 +83,24 @@ export class ActionRegister { const isLinewise = args.motions.some((motion) => motion.isLinewise); - const ranges = activeTextEditor.selections.map((selection) => { + const promisedRanges = activeTextEditor.selections.map(async (selection) => { const start = selection.active; - const end = args.motions.reduce((position, motion) => { - return motion.apply(position, { - isInclusive: true, - shouldCrossLines: false, - isChangeAction: args.isChangeAction, - }); - }, start); + const end = await args.motions.reduce((promisedPosition, motion) => { + return promisedPosition.then((position) => + motion.apply(position, { + isInclusive: true, + shouldCrossLines: false, + isChangeAction: args.isChangeAction, + }), + ); + }, Promise.resolve(start)); return new Range(start, end); }); - - return ActionRegister.yankRanges({ - ranges: ranges, - isLinewise: isLinewise, + return Promise.all(promisedRanges).then((ranges) => { + return ActionRegister.yankRanges({ + ranges: ranges, + isLinewise: isLinewise, + }); }); } diff --git a/src/Motions/Direction.ts b/src/Motions/Direction.ts index 02e74f0e..2d7de9e0 100644 --- a/src/Motions/Direction.ts +++ b/src/Motions/Direction.ts @@ -33,8 +33,8 @@ export class MotionDirection extends Motion { }); } - apply(from: Position, option?: any): Position { - from = super.apply(from); + async apply(from: Position, option?: any): Promise { + from = await super.apply(from); const activeTextEditor = window.activeTextEditor; diff --git a/src/Motions/Document.ts b/src/Motions/Document.ts index 23dc4890..63039b33 100644 --- a/src/Motions/Document.ts +++ b/src/Motions/Document.ts @@ -28,8 +28,8 @@ export class MotionDocument extends Motion { return obj; } - apply(from: Position): Position { - from = super.apply(from); + async apply(from: Position): Promise { + from = await super.apply(from); const activeTextEditor = window.activeTextEditor; diff --git a/src/Motions/Line.ts b/src/Motions/Line.ts index 8466e7c4..42ed3c30 100644 --- a/src/Motions/Line.ts +++ b/src/Motions/Line.ts @@ -22,8 +22,8 @@ export class MotionLine extends Motion { return obj; } - apply(from: Position): Position { - from = super.apply(from); + async apply(from: Position): Promise { + from = await super.apply(from); const activeTextEditor = window.activeTextEditor; diff --git a/src/Motions/Match.ts b/src/Motions/Match.ts index e727b4b6..b8d9bda8 100644 --- a/src/Motions/Match.ts +++ b/src/Motions/Match.ts @@ -66,10 +66,10 @@ export class MotionMatch extends Motion { return obj; } - apply(from: Position, option: { isInclusive?: boolean } = {}): Position { + async apply(from: Position, option: { isInclusive?: boolean } = {}): Promise { option.isInclusive = option.isInclusive === undefined ? false : option.isInclusive; - from = super.apply(from); + from = await super.apply(from); const activeTextEditor = window.activeTextEditor; diff --git a/src/Motions/MatchPair.ts b/src/Motions/MatchPair.ts index 614359ce..ce3e80d3 100644 --- a/src/Motions/MatchPair.ts +++ b/src/Motions/MatchPair.ts @@ -36,15 +36,15 @@ export class MotionMatchPair extends Motion { return new MotionMatchPair(); } - apply( + async apply( from: Position, option: { isInclusive?: boolean; } = {}, - ): Position { + ): Promise { option.isInclusive = option.isInclusive === undefined ? false : option.isInclusive; - from = super.apply(from); + from = await super.apply(from); const activeTextEditor = window.activeTextEditor; diff --git a/src/Motions/Motion.ts b/src/Motions/Motion.ts index becc83a7..0c1c43d2 100644 --- a/src/Motions/Motion.ts +++ b/src/Motions/Motion.ts @@ -23,7 +23,7 @@ export abstract class Motion { this.characterDelta += characterDelta; } - apply(from: Position, option?: any): Position { + async apply(from: Position, option?: any): Promise { const activeTextEditor = window.activeTextEditor; if (!activeTextEditor) { diff --git a/src/Motions/Paragraph.ts b/src/Motions/Paragraph.ts index 55db5ddf..978f5f8a 100644 --- a/src/Motions/Paragraph.ts +++ b/src/Motions/Paragraph.ts @@ -33,8 +33,8 @@ export class MotionParagraph extends Motion { }); } - apply(from: Position): Position { - from = super.apply(from); + async apply(from: Position): Promise { + from = await super.apply(from); const activeTextEditor = window.activeTextEditor; diff --git a/src/Motions/Word.ts b/src/Motions/Word.ts index b94c4666..cae2684b 100644 --- a/src/Motions/Word.ts +++ b/src/Motions/Word.ts @@ -79,20 +79,20 @@ export class MotionWord extends Motion { return obj; } - apply( + async apply( from: Position, option: { isInclusive?: boolean; isChangeAction?: boolean; shouldCrossLines?: boolean; } = {}, - ): Position { + ): Promise { option.isInclusive = option.isInclusive === undefined ? false : option.isInclusive; option.isChangeAction = option.isChangeAction === undefined ? false : option.isChangeAction; option.shouldCrossLines = option.shouldCrossLines === undefined ? true : option.shouldCrossLines; - from = super.apply(from); + from = await super.apply(from); const activeTextEditor = window.activeTextEditor; From 49c4c069a90a08cc3238ef006848326c7db4d9f8 Mon Sep 17 00:00:00 2001 From: Alison Winters Date: Tue, 28 Jan 2020 16:58:34 +0800 Subject: [PATCH 2/8] add `gd` go to definition motions --- src/Mappers/SpecialKeys/Motion.ts | 3 +++ src/Motions/Navigation.ts | 32 +++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 src/Motions/Navigation.ts diff --git a/src/Mappers/SpecialKeys/Motion.ts b/src/Mappers/SpecialKeys/Motion.ts index 86e4b3cb..e55413b2 100644 --- a/src/Mappers/SpecialKeys/Motion.ts +++ b/src/Mappers/SpecialKeys/Motion.ts @@ -11,6 +11,7 @@ import { MotionMatchPair } from '../../Motions/MatchPair'; import { MotionLine } from '../../Motions/Line'; import { MotionParagraph } from '../../Motions/Paragraph'; import { MotionDocument } from '../../Motions/Document'; +import { MotionNavigation } from '../../Motions/Navigation'; interface MotionGenerator { (args?: {}): Motion; @@ -120,6 +121,8 @@ export class SpecialKeyMotion extends GenericMapper implements SpecialKeyCommon { keys: 'space', motionGenerators: [MotionDirection.next] }, { keys: 'backspace', motionGenerators: [MotionDirection.prev] }, + { keys: 'g d', motionGenerators: [MotionNavigation.toDeclaration] }, + { keys: 'g D', motionGenerators: [MotionNavigation.toTypeDefinition] }, ]; constructor() { diff --git a/src/Motions/Navigation.ts b/src/Motions/Navigation.ts new file mode 100644 index 00000000..23f6f206 --- /dev/null +++ b/src/Motions/Navigation.ts @@ -0,0 +1,32 @@ +import { commands, window, Position } from 'vscode'; +import { Motion } from './Motion'; + +export class MotionNavigation extends Motion { + private command: string; + + static toDeclaration(): Motion { + const obj = new MotionNavigation({ isLinewise: true }); + obj.command = 'editor.action.goToDeclaration'; + return obj; + } + + static toTypeDefinition(): Motion { + const obj = new MotionNavigation({ isLinewise: true }); + obj.command = 'editor.action.goToTypeDefinition'; + return obj; + } + + async apply(from: Position): Promise { + from = await super.apply(from); + + const activeTextEditor = window.activeTextEditor; + + if (!activeTextEditor) { + return from; + } + + await commands.executeCommand(this.command); + + return activeTextEditor.selection.active; + } +} From 914fe303d424179b360a3de494ead8e4a9b06b53 Mon Sep 17 00:00:00 2001 From: Alison Winters Date: Tue, 28 Jan 2020 18:33:59 +0800 Subject: [PATCH 3/8] minor indentation tweak for consistency --- src/Actions/Delete.ts | 8 ++++---- src/Actions/Register.ts | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Actions/Delete.ts b/src/Actions/Delete.ts index 9c76a4de..69e76a29 100644 --- a/src/Actions/Delete.ts +++ b/src/Actions/Delete.ts @@ -56,11 +56,11 @@ export class ActionDelete { }) : Promise.resolve(true) ) - .then(() => { - return activeTextEditor.edit((editBuilder) => { + .then(() => + activeTextEditor.edit((editBuilder) => { ranges.forEach((range) => editBuilder.delete(range)); - }); - }) + }), + ) .then(() => ActionSelection.shrinkToStarts()) .then(() => ActionReveal.primaryCursor()); }); diff --git a/src/Actions/Register.ts b/src/Actions/Register.ts index bf71795e..527eae06 100644 --- a/src/Actions/Register.ts +++ b/src/Actions/Register.ts @@ -96,12 +96,12 @@ export class ActionRegister { }, Promise.resolve(start)); return new Range(start, end); }); - return Promise.all(promisedRanges).then((ranges) => { - return ActionRegister.yankRanges({ + return Promise.all(promisedRanges).then((ranges) => + ActionRegister.yankRanges({ ranges: ranges, isLinewise: isLinewise, - }); - }); + }), + ); } static async yankByTextObject(args: { textObject: TextObject }): Promise { From c003d5cc74237b0be1c8dc002e85cf507bc225d6 Mon Sep 17 00:00:00 2001 From: Alison Winters Date: Tue, 28 Jan 2020 22:15:26 +0800 Subject: [PATCH 4/8] add support for setting `language` on the testcase - setting language will add sleeps to allow for language server latency - also add some basic tests for `gd` go to definition --- test/Framework/BlackBox.ts | 7 +++++- test/Framework/Util.ts | 22 ++++++++++++------- test/ModeNormal/g d.test.ts | 43 +++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 test/ModeNormal/g d.test.ts diff --git a/test/Framework/BlackBox.ts b/test/Framework/BlackBox.ts index d5528ad4..939431b3 100644 --- a/test/Framework/BlackBox.ts +++ b/test/Framework/BlackBox.ts @@ -3,6 +3,7 @@ import * as TestUtil from './Util'; import { TextEditor, TextDocument, Selection, extensions } from 'vscode'; export interface TestCase { + language?: string; from: string; inputs: string; to: string; @@ -110,7 +111,7 @@ export const run = (testCase: TestCase, before?: (textEditor: TextEditor) => voi const toInfo = extractInfo(testCase.to); const inputs = testCase.inputs.split(' '); - TestUtil.createTempDocument(fromInfo.cleanText, reusableDocument).then( + TestUtil.createTempDocument(fromInfo.cleanText, reusableDocument, testCase.language).then( async (textEditor) => { reusableDocument = textEditor.document; @@ -127,6 +128,10 @@ export const run = (testCase: TestCase, before?: (textEditor: TextEditor) => voi await waitForMillisecond(20 * tries); } + if (testCase.language) { + await waitForMillisecond(50 * tries); + } + try { assert.equal(TestUtil.getDocument()!.getText(), toInfo.cleanText); assert.deepEqual(TestUtil.getSelections(), toInfo.selections); diff --git a/test/Framework/Util.ts b/test/Framework/Util.ts index 3f6f4a5b..0c0d2673 100644 --- a/test/Framework/Util.ts +++ b/test/Framework/Util.ts @@ -1,7 +1,6 @@ import { workspace, window, - Uri, TextDocument, TextEditor, Position, @@ -13,22 +12,29 @@ import { export function createTempDocument( content?: string, reusableDocument?: TextDocument, + language: string = 'plaintext', ): Thenable { let getTextEditor: Thenable; if ( - reusableDocument && + reusableDocument?.languageId === language && window.activeTextEditor && window.activeTextEditor.document === reusableDocument ) { getTextEditor = Promise.resolve(window.activeTextEditor); } else { - const uri = reusableDocument - ? reusableDocument.uri - : Uri.parse(`untitled:${__dirname}.${Math.random()}.tmp`); - getTextEditor = workspace - .openTextDocument(uri) - .then((document) => window.showTextDocument(document)); + // for non-plaintext files, sleep for a while to let the language server load + const getDocument = + reusableDocument?.languageId === language + ? workspace.openTextDocument(reusableDocument.uri) + : workspace.openTextDocument({ language }).then((document) => + language === 'plaintext' + ? document + : new Promise((resolve) => { + setTimeout(() => resolve(document), 1500); + }), + ); + getTextEditor = getDocument.then((document) => window.showTextDocument(document)); } return getTextEditor.then((textEditor) => { diff --git a/test/ModeNormal/g d.test.ts b/test/ModeNormal/g d.test.ts new file mode 100644 index 00000000..23f5937f --- /dev/null +++ b/test/ModeNormal/g d.test.ts @@ -0,0 +1,43 @@ +import * as BlackBox from '../Framework/BlackBox'; + +suite('Normal: g d', () => { + const testCases: BlackBox.TestCase[] = [ + { + language: 'javascript', + from: 'class C {\n x = 0;\n}\nconst c = new C();\n[]c.x = 1;', + inputs: 'g d', + to: 'class C {\n x = 0;\n}\nconst []c = new C();\nc.x = 1;', + }, + { + language: 'javascript', + from: 'class C {\n x = 0;\n}\nconst c = new C();\nc.[]x = 1;', + inputs: 'g d', + to: 'class C {\n []x = 0;\n}\nconst c = new C();\nc.x = 1;', + }, + ]; + + for (let i = 0; i < testCases.length; i++) { + BlackBox.run(testCases[i]); + } +}); + +suite('Normal: g D', () => { + const testCases: BlackBox.TestCase[] = [ + { + language: 'javascript', + from: 'class C {\n x = 0;\n}\nconst c = new C();\n[]c.x = 1;', + inputs: 'g D', + to: 'class []C {\n x = 0;\n}\nconst c = new C();\nc.x = 1;', + }, + { + language: 'javascript', + from: 'class C {\n x = 0;\n}\nconst c = new C();\nc.[]x = 1;', + inputs: 'g D', + to: 'class C {\n x = 0;\n}\nconst c = new C();\nc.[]x = 1;', + }, + ]; + + for (let i = 0; i < testCases.length; i++) { + BlackBox.run(testCases[i]); + } +}); From 230ded9b7e6c2478294b9288dedea9d166748855 Mon Sep 17 00:00:00 2001 From: Alison Winters Date: Tue, 28 Jan 2020 22:48:52 +0800 Subject: [PATCH 5/8] index reusable docs by language to avoid creating superfluous tabs --- test/Framework/BlackBox.ts | 59 ++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/test/Framework/BlackBox.ts b/test/Framework/BlackBox.ts index 939431b3..5a0bdd97 100644 --- a/test/Framework/BlackBox.ts +++ b/test/Framework/BlackBox.ts @@ -94,7 +94,7 @@ const extractInfo = (originalText: string) => { }; }; -let reusableDocument: TextDocument; +const reusableDocuments: Map = new Map(); export const run = (testCase: TestCase, before?: (textEditor: TextEditor) => void) => { const plainFrom = testCase.from.replace(/\n/g, '\\n'); @@ -107,41 +107,44 @@ export const run = (testCase: TestCase, before?: (textEditor: TextEditor) => voi test(expectation, (done) => { tries++; + const language = testCase.language || 'plaintext'; const fromInfo = extractInfo(testCase.from); const toInfo = extractInfo(testCase.to); const inputs = testCase.inputs.split(' '); - TestUtil.createTempDocument(fromInfo.cleanText, reusableDocument, testCase.language).then( - async (textEditor) => { - reusableDocument = textEditor.document; + TestUtil.createTempDocument( + fromInfo.cleanText, + reusableDocuments.get(language), + language, + ).then(async (textEditor) => { + reusableDocuments.set(textEditor.document.languageId, textEditor.document); - if (before) { - before(textEditor); - } - - TestUtil.setSelections(fromInfo.selections); + if (before) { + before(textEditor); + } - await waitForMillisecond(50 * tries); - - for (let i = 0; i < inputs.length; i++) { - getCurrentMode()!.input(inputs[i]); - await waitForMillisecond(20 * tries); - } + TestUtil.setSelections(fromInfo.selections); - if (testCase.language) { - await waitForMillisecond(50 * tries); - } + await waitForMillisecond(50 * tries); - try { - assert.equal(TestUtil.getDocument()!.getText(), toInfo.cleanText); - assert.deepEqual(TestUtil.getSelections(), toInfo.selections); - } catch (error) { - done(error); - return; - } + for (let i = 0; i < inputs.length; i++) { + getCurrentMode()!.input(inputs[i]); + await waitForMillisecond(20 * tries); + } - done(); - }, - ); + if (language !== 'plaintext') { + await waitForMillisecond(50 * tries); + } + + try { + assert.equal(TestUtil.getDocument()!.getText(), toInfo.cleanText); + assert.deepEqual(TestUtil.getSelections(), toInfo.selections); + } catch (error) { + done(error); + return; + } + + done(); + }); }); }; From 1f6152ad194eda990157b1ff9fcb7b51502e97b5 Mon Sep 17 00:00:00 2001 From: Alison Winters Date: Sun, 7 Jun 2020 21:02:53 +0800 Subject: [PATCH 6/8] use async/await instead of promise when creating temp doc --- test/Framework/Util.ts | 49 +++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/test/Framework/Util.ts b/test/Framework/Util.ts index 0c0d2673..d5c3770e 100644 --- a/test/Framework/Util.ts +++ b/test/Framework/Util.ts @@ -9,46 +9,41 @@ import { EndOfLine, } from 'vscode'; -export function createTempDocument( +export async function createTempDocument( content?: string, reusableDocument?: TextDocument, language: string = 'plaintext', -): Thenable { - let getTextEditor: Thenable; +): Promise { + let textEditor: TextEditor; if ( reusableDocument?.languageId === language && window.activeTextEditor && window.activeTextEditor.document === reusableDocument ) { - getTextEditor = Promise.resolve(window.activeTextEditor); + textEditor = window.activeTextEditor; } else { - // for non-plaintext files, sleep for a while to let the language server load - const getDocument = - reusableDocument?.languageId === language - ? workspace.openTextDocument(reusableDocument.uri) - : workspace.openTextDocument({ language }).then((document) => - language === 'plaintext' - ? document - : new Promise((resolve) => { - setTimeout(() => resolve(document), 1500); - }), - ); - getTextEditor = getDocument.then((document) => window.showTextDocument(document)); + let document: TextDocument; + if (reusableDocument?.languageId === language) { + document = await workspace.openTextDocument(reusableDocument.uri); + } else { + document = await workspace.openTextDocument({ language }); + // for non-plaintext files, sleep for a while to let the language server load + if (language !== 'plaintext') { + await new Promise((resolve) => setTimeout(resolve, 2000)); + } + } + textEditor = await window.showTextDocument(document); } - return getTextEditor.then((textEditor) => { - if (content) { - return textEditor - .edit((editBuilder) => { - editBuilder.setEndOfLine(EndOfLine.LF); - editBuilder.replace(new Range(0, 0, textEditor.document.lineCount, 0), content); - }) - .then(() => textEditor); - } + if (content) { + await textEditor.edit((editBuilder) => { + editBuilder.setEndOfLine(EndOfLine.LF); + editBuilder.replace(new Range(0, 0, textEditor.document.lineCount, 0), content); + }); + } - return textEditor; - }); + return textEditor; } export function getDocument(): TextDocument | undefined { From fad820d88ae334d6a46577088174a4f764691936 Mon Sep 17 00:00:00 2001 From: Alison Winters Date: Mon, 8 Jun 2020 22:46:30 +0800 Subject: [PATCH 7/8] use async/await instead of reducing promises when applying async motions --- src/Actions/Delete.ts | 72 +++++++++++++++++++-------------------- src/Actions/MoveCursor.ts | 42 ++++++++++++----------- src/Actions/Register.ts | 42 ++++++++++++----------- 3 files changed, 81 insertions(+), 75 deletions(-) diff --git a/src/Actions/Delete.ts b/src/Actions/Delete.ts index 69e76a29..b93eabac 100644 --- a/src/Actions/Delete.ts +++ b/src/Actions/Delete.ts @@ -10,60 +10,58 @@ import { UtilRange } from '../Utils/Range'; export class ActionDelete { @StaticReflect.metadata(SymbolMetadata.Action.isChange, true) - static byMotions(args: { + static async byMotions(args: { motions: Motion[]; isChangeAction?: boolean; shouldYank?: boolean; - }): Thenable { + }): Promise { args.isChangeAction = args.isChangeAction === undefined ? false : args.isChangeAction; args.shouldYank = args.shouldYank === undefined ? false : args.shouldYank; const activeTextEditor = window.activeTextEditor; if (!activeTextEditor) { - return Promise.resolve(false); + return false; } const document = activeTextEditor.document; - const promisedRanges = activeTextEditor.selections.map(async (selection) => { - const start = selection.active; - const end = await args.motions.reduce((promisedPosition, motion) => { - return promisedPosition.then((position) => - motion.apply(position, { - isInclusive: true, - shouldCrossLines: false, - isChangeAction: args.isChangeAction, - }), - ); - }, Promise.resolve(start)); - return new Range(start, end); - }); + let ranges: Range[] = []; - return Promise.all(promisedRanges).then((ranges) => { - if (args.motions.some((motion) => motion.isLinewise)) { - ranges = ranges.map((range) => UtilRange.toLinewise(range, document)); + for (const selection of activeTextEditor.selections) { + const start = selection.active; + let position = start; + for (const motion of args.motions) { + position = await motion.apply(position, { + isInclusive: true, + shouldCrossLines: false, + isChangeAction: args.isChangeAction, + }); } + ranges.push(new Range(start, position)); + } - ranges = UtilRange.unionOverlaps(ranges); - - // TODO: Move cursor to first non-space if needed - - return (args.shouldYank - ? ActionRegister.yankByMotions({ - motions: args.motions, - isChangeAction: args.isChangeAction, - }) - : Promise.resolve(true) - ) - .then(() => - activeTextEditor.edit((editBuilder) => { - ranges.forEach((range) => editBuilder.delete(range)); - }), - ) - .then(() => ActionSelection.shrinkToStarts()) - .then(() => ActionReveal.primaryCursor()); + if (args.motions.some((motion) => motion.isLinewise)) { + ranges = ranges.map((range) => UtilRange.toLinewise(range, document)); + } + + ranges = UtilRange.unionOverlaps(ranges); + + // TODO: Move cursor to first non-space if needed + + if (args.shouldYank) { + await ActionRegister.yankByMotions({ + motions: args.motions, + isChangeAction: args.isChangeAction, + }); + } + await activeTextEditor.edit((editBuilder) => { + ranges.forEach((range) => editBuilder.delete(range)); }); + await ActionSelection.shrinkToStarts(); + await ActionReveal.primaryCursor(); + + return true; } @StaticReflect.metadata(SymbolMetadata.Action.isChange, true) diff --git a/src/Actions/MoveCursor.ts b/src/Actions/MoveCursor.ts index 0d892dbd..52383c43 100644 --- a/src/Actions/MoveCursor.ts +++ b/src/Actions/MoveCursor.ts @@ -42,12 +42,12 @@ export class ActionMoveCursor { return Promise.resolve(true); } - static byMotions(args: { + static async byMotions(args: { motions: Motion[]; isVisualMode?: boolean; isVisualLineMode?: boolean; noEmptyAtLineEnd?: boolean; - }): Thenable { + }): Promise { args.isVisualMode = args.isVisualMode === undefined ? false : args.isVisualMode; args.isVisualLineMode = args.isVisualLineMode === undefined ? false : args.isVisualLineMode; args.noEmptyAtLineEnd = args.noEmptyAtLineEnd === undefined ? false : args.noEmptyAtLineEnd; @@ -55,7 +55,7 @@ export class ActionMoveCursor { const activeTextEditor = window.activeTextEditor; if (!activeTextEditor) { - return Promise.resolve(false); + return false; } // Prevent preferred character update if no motion updates character. @@ -65,16 +65,21 @@ export class ActionMoveCursor { const document = activeTextEditor.document; - const promisedSelections = activeTextEditor.selections.map(async (selection, i) => { + const selections: Selection[] = []; + + for (let i = 0; i < activeTextEditor.selections.length; i++) { + const selection = activeTextEditor.selections[i]; let anchor: Position; - let active = await args.motions.reduce((promisedPosition, motion) => { - return promisedPosition.then((position) => - motion.apply(position, { - preferredColumn: ActionMoveCursor.preferredColumnBySelectionIndex[i], - }), - ); - }, Promise.resolve(args.isVisualMode ? UtilSelection.getActiveInVisualMode(selection) : selection.active)); + let active = args.isVisualMode + ? UtilSelection.getActiveInVisualMode(selection) + : selection.active; + + for (const motion of args.motions) { + active = await motion.apply(active, { + preferredColumn: ActionMoveCursor.preferredColumnBySelectionIndex[i], + }); + } if (args.isVisualMode) { anchor = selection.anchor; @@ -124,13 +129,12 @@ export class ActionMoveCursor { anchor = active; } - return new Selection(anchor, active); - }); - return Promise.all(promisedSelections) - .then((selections) => { - activeTextEditor.selections = selections; - return Promise.resolve(true); - }) - .then(() => ActionReveal.primaryCursor()); + selections.push(new Selection(anchor, active)); + } + + activeTextEditor.selections = selections; + await ActionReveal.primaryCursor(); + + return true; } } diff --git a/src/Actions/Register.ts b/src/Actions/Register.ts index 527eae06..ed52895f 100644 --- a/src/Actions/Register.ts +++ b/src/Actions/Register.ts @@ -72,36 +72,40 @@ export class ActionRegister { return Promise.resolve(true); } - static yankByMotions(args: { motions: Motion[]; isChangeAction?: boolean }): Thenable { + static async yankByMotions(args: { + motions: Motion[]; + isChangeAction?: boolean; + }): Promise { args.isChangeAction = args.isChangeAction === undefined ? false : args.isChangeAction; const activeTextEditor = window.activeTextEditor; if (!activeTextEditor) { - return Promise.resolve(false); + return false; } const isLinewise = args.motions.some((motion) => motion.isLinewise); - const promisedRanges = activeTextEditor.selections.map(async (selection) => { + let ranges: Range[] = []; + + for (const selection of activeTextEditor.selections) { const start = selection.active; - const end = await args.motions.reduce((promisedPosition, motion) => { - return promisedPosition.then((position) => - motion.apply(position, { - isInclusive: true, - shouldCrossLines: false, - isChangeAction: args.isChangeAction, - }), - ); - }, Promise.resolve(start)); - return new Range(start, end); + let position = start; + for (const motion of args.motions) { + position = await motion.apply(position, { + isInclusive: true, + shouldCrossLines: false, + isChangeAction: args.isChangeAction, + }); + } + ranges.push(new Range(start, position)); + } + await ActionRegister.yankRanges({ + ranges: ranges, + isLinewise: isLinewise, }); - return Promise.all(promisedRanges).then((ranges) => - ActionRegister.yankRanges({ - ranges: ranges, - isLinewise: isLinewise, - }), - ); + + return true; } static async yankByTextObject(args: { textObject: TextObject }): Promise { From 1c9dc25a6ea20606ebbf5fbeedd37c5dfac1e4ed Mon Sep 17 00:00:00 2001 From: Alison Winters Date: Fri, 12 Jun 2020 21:08:12 +0800 Subject: [PATCH 8/8] bump mocha retries to 3 --- test/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/index.ts b/test/index.ts index 6b6dbef9..a33bfb30 100644 --- a/test/index.ts +++ b/test/index.ts @@ -7,7 +7,7 @@ export function run(): Promise { const mocha = new Mocha({ ui: 'tdd', timeout: 2500, - retries: 2, + retries: 3, }); mocha.useColors(true);