diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a227ccb..f5d30edc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ All notable changes to the Keyboard Macro Bata extension will be documented in this file. ### [Unreleased] +- Feature + - Made the `kb-macro.wrap` command queueable to reduce input misses during recording. [#32](https://github.com/tshino/vscode-kb-macro/pull/32) - Documentation - Added 'Tips' section to the README. diff --git a/src/keyboard_macro.js b/src/keyboard_macro.js index 2ff7904d..7a722631 100644 --- a/src/keyboard_macro.js +++ b/src/keyboard_macro.js @@ -184,7 +184,14 @@ const KeyboardMacro = function({ awaitController }) { return spec; }; - const wrap = reentrantGuard.makeGuardedCommand(async function(args) { + // WrapQueueSize + // independently adjustable value. + // min value is 1. + // greater value reduces input rejection. 2 or 3 is enough. + // greater value leads to too many queued and delayed command execution. + // See: https://github.com/tshino/vscode-kb-macro/pull/32 + const WrapQueueSize = 2; + const wrap = reentrantGuard.makeQueueableCommand(async function(args) { if (recording) { const spec = makeCommandSpec(args); if (!spec) { @@ -204,7 +211,7 @@ const KeyboardMacro = function({ awaitController }) { } } } - }); + }, { queueSize: WrapQueueSize }); return { RecordingStateReason, @@ -228,7 +235,8 @@ const KeyboardMacro = function({ awaitController }) { isRecording: () => { return recording; }, isPlaying: () => { return playing; }, getCurrentSequence: () => { return sequence.get(); }, - setShowInputBox // testing purpose only + setShowInputBox, + WrapQueueSize }; }; diff --git a/src/reentrant_guard.js b/src/reentrant_guard.js index f54bcd06..70a8aea5 100644 --- a/src/reentrant_guard.js +++ b/src/reentrant_guard.js @@ -2,7 +2,11 @@ const reentrantGuard = (function() { - let locked = false; + const state = { + locked: false, + queueable: false + }; + const queue = []; let printError = defaultPrintError; function defaultPrintError(error) { @@ -17,31 +21,64 @@ const reentrantGuard = (function() { const makeGuardedCommand = function(body) { return async function(args) { - if (locked) { + if (state.locked) { return; } - locked = true; + state.locked = true; try { await body(args); } catch (error) { printError(error); } finally { - locked = false; + state.locked = false; } }; }; const makeGuardedCommandSync = function(func) { return function(args) { - if (locked) { + if (state.locked) { return; } - locked = true; + state.locked = true; try { func(args); } catch (error) { printError(error); } finally { - locked = false; + state.locked = false; + } + }; + }; + const makeQueueableCommand = function(body, { queueSize = 0 } = {}) { + return async function(args) { + if (state.locked) { + if (state.queueable) { + if (!queueSize || queue.length < queueSize) { + queue.push([body, args]); + } + } + return; + } + state.locked = true; + state.queueable = true; + try { + try { + await body(args); + } catch (error) { + printError(error); + } + while (0 < queue.length) { + try { + const [body, args] = queue[0]; + queue.splice(0, 1); + await body(args); + } catch (error) { + printError(error); + } + } + } finally { + state.locked = false; + state.queueable = false; } }; }; @@ -49,9 +86,11 @@ const reentrantGuard = (function() { return { makeGuardedCommand, makeGuardedCommandSync, + makeQueueableCommand, // testing purpose only - setPrintError + setPrintError, + getQueueLength: function() { return queue.length; } }; })(); diff --git a/test/suite/keyboard_macro.test.js b/test/suite/keyboard_macro.test.js index eac74b97..35a00154 100644 --- a/test/suite/keyboard_macro.test.js +++ b/test/suite/keyboard_macro.test.js @@ -551,19 +551,55 @@ describe('KeybaordMacro', () => { { command: 'internal:log', args: { test: '1' } } ]); }); - it('should prevent reentry', async () => { + it('should enqueue and serialize concurrent calls', async () => { keyboardMacro.startRecording(); const promise1 = keyboardMacro.wrap({ command: 'internal:log' }); const promise2 = keyboardMacro.wrap({ command: 'internal:log' }); await Promise.all([promise1, promise2]); keyboardMacro.finishRecording(); - assert.deepStrictEqual(logs, [ 'begin', 'end' ]); + assert.deepStrictEqual(logs, [ + 'begin', 'end', + 'begin', 'end' + ]); assert.deepStrictEqual(keyboardMacro.getCurrentSequence(), [ + { command: 'internal:log' }, { command: 'internal:log' } ]); }); - it('should prevent other commands to preempt (cancelRecording)', async () => { + it('should be able to enqueue and serialize concurrent call up to WrapQueueSize', async () => { + keyboardMacro.startRecording(); + const promises = []; + promises.push(keyboardMacro.wrap({ command: 'internal:log' })); // (1) + for (let i = 0; i < keyboardMacro.WrapQueueSize; i++) { + promises.push(keyboardMacro.wrap({ command: 'internal:log' })); // (2) to (WrapQueueSize + 1) + } + await Promise.all(promises); + await keyboardMacro.wrap({ command: 'internal:log' }); // (WrapQueueSize + 2) + keyboardMacro.finishRecording(); + + const expectedLog = Array(keyboardMacro.WrapQueueSize + 2).fill(['begin', 'end']).flat(); + const expectedSequence = Array(keyboardMacro.WrapQueueSize + 2).fill({ command: 'internal:log' }); + assert.deepStrictEqual(logs, expectedLog); + assert.deepStrictEqual(keyboardMacro.getCurrentSequence(), expectedSequence); + }); + it('should overflow when over WrapQueueSize concurrent calls made', async () => { + keyboardMacro.startRecording(); + const promises = []; + promises.push(keyboardMacro.wrap({ command: 'internal:log' })); // (1) + for (let i = 0; i < keyboardMacro.WrapQueueSize + 1; i++) { // <-- PLUS ONE! + promises.push(keyboardMacro.wrap({ command: 'internal:log' })); // (2) to (WrapQueueSize + 2) + } + await Promise.all(promises); + await keyboardMacro.wrap({ command: 'internal:log' }); // (WrapQueueSize + 3) + keyboardMacro.finishRecording(); + + const expectedLog = Array(keyboardMacro.WrapQueueSize + 2).fill(['begin', 'end']).flat(); + const expectedSequence = Array(keyboardMacro.WrapQueueSize + 2).fill({ command: 'internal:log' }); + assert.deepStrictEqual(logs, expectedLog); + assert.deepStrictEqual(keyboardMacro.getCurrentSequence(), expectedSequence); + }); + it('should prevent other commands from interrupting wrap command (cancelRecording)', async () => { keyboardMacro.startRecording(); const promise1 = keyboardMacro.wrap({ command: 'internal:log' }); keyboardMacro.cancelRecording(); // <-- @@ -575,7 +611,7 @@ describe('KeybaordMacro', () => { ]); assert.strictEqual(keyboardMacro.isRecording(), true); }); - it('should prevent other commands to preempt (finishRecording)', async () => { + it('should prevent other commands from interrupting wrap command (finishRecording)', async () => { keyboardMacro.startRecording(); const promise1 = keyboardMacro.wrap({ command: 'internal:log' }); keyboardMacro.finishRecording(); // <-- diff --git a/test/suite/reentrant_guard.test.js b/test/suite/reentrant_guard.test.js index 0f220d76..c2ae7ede 100644 --- a/test/suite/reentrant_guard.test.js +++ b/test/suite/reentrant_guard.test.js @@ -252,4 +252,258 @@ describe('reentrantGuard', () => { } }); }); + describe('makeQueueableCommand', () => { + const makeQueueableCommand = reentrantGuard.makeQueueableCommand; + const logs = []; + beforeEach(() => { + logs.length = 0; + }); + it('should return an async function', async () => { + const func = makeQueueableCommand(() => {}); + assert.strictEqual(typeof func, 'function'); + assert.strictEqual(func.constructor.name, 'AsyncFunction'); + }); + it('should return a function that invokes the given function', async () => { + const target = function() { + logs.push('hello'); + }; + const func = makeQueueableCommand(target); + assert.deepStrictEqual(logs, []); + await func(); + assert.deepStrictEqual(logs, [ 'hello' ]); + await func(); + assert.deepStrictEqual(logs, [ 'hello', 'hello' ]); + }); + it('should return a function that passes arguments to the given function', async () => { + const target = function(arg) { + logs.push('hello ' + arg); + }; + const func = makeQueueableCommand(target); + await func('world'); + assert.deepStrictEqual(logs, [ 'hello world' ]); + }); + it('should return a function that invokes the given async function', async () => { + const asyncTarget = async function() { + logs.push('hello'); + await TestUtil.sleep(10); + logs.push('bye'); + }; + const func = makeQueueableCommand(asyncTarget); + logs.push('before call'); + await func(); + logs.push('after call'); + assert.deepStrictEqual(logs, [ + 'before call', + 'hello', + 'bye', + 'after call' + ]); + }); + it('should enqueue and serialize invocation of the function', async () => { + const asyncTarget = async function(arg) { + logs.push('hello ' + arg); + await TestUtil.sleep(10); + logs.push('bye'); + }; + const func = makeQueueableCommand(asyncTarget); + logs.push('before concurrent call'); + const promise1 = func('1'); + const promise2 = func('2'); + assert.strictEqual(reentrantGuard.getQueueLength(), 1); + await Promise.all([promise1, promise2]); + logs.push('after concurrent call'); + assert.strictEqual(reentrantGuard.getQueueLength(), 0); + await func('3'); + logs.push('after third call'); + assert.strictEqual(reentrantGuard.getQueueLength(), 0); + assert.deepStrictEqual(logs, [ + 'before concurrent call', + 'hello 1', + 'bye', + 'hello 2', + 'bye', + 'after concurrent call', + 'hello 3', + 'bye', + 'after third call' + ]); + }); + it('should not enqueue over queueSize if specified', async () => { + const asyncTarget = async function(arg) { + logs.push('hello ' + arg); + await TestUtil.sleep(10); + logs.push('bye'); + }; + const func = makeQueueableCommand(asyncTarget, { queueSize: 3 }); + logs.push('before concurrent call'); + const promise1 = func('1'); + const promise2 = func('2'); + const promise3 = func('3'); + const promise4 = func('4'); + const promise5 = func('5'); + assert.strictEqual(reentrantGuard.getQueueLength(), 3); + await Promise.all([promise1, promise2, promise3, promise4, promise5]); + logs.push('after concurrent call'); + await func('6'); + logs.push('after last call'); + assert.strictEqual(reentrantGuard.getQueueLength(), 0); + assert.deepStrictEqual(logs, [ + 'before concurrent call', + 'hello 1', + 'bye', + 'hello 2', + 'bye', + 'hello 3', + 'bye', + 'hello 4', + 'bye', + 'after concurrent call', + 'hello 6', + 'bye', + 'after last call' + ]); + }); + it('should prevent queueable function from interrupting guarded functions', async () => { + const asyncTarget = async function(arg) { + logs.push('hello ' + arg); + await TestUtil.sleep(10); + logs.push('bye'); + }; + const guarded = reentrantGuard.makeGuardedCommand(asyncTarget); + const queueable = makeQueueableCommand(asyncTarget); + logs.push('before concurrent call'); + const promise1 = guarded('guarded'); + const promise2 = queueable('queueable'); + assert.strictEqual(reentrantGuard.getQueueLength(), 0); + await Promise.all([promise1, promise2]); + logs.push('after concurrent call'); + assert.strictEqual(reentrantGuard.getQueueLength(), 0); + assert.deepStrictEqual(logs, [ + 'before concurrent call', + 'hello guarded', + 'bye', + 'after concurrent call' + ]); + }); + it('should prevent guarded function from interrupting queueable functions', async () => { + const asyncTarget = async function(arg) { + logs.push('hello ' + arg); + await TestUtil.sleep(10); + logs.push('bye'); + }; + const guarded = reentrantGuard.makeGuardedCommand(asyncTarget); + const queueable = makeQueueableCommand(asyncTarget); + logs.push('before concurrent call'); + const promise1 = queueable('queueable'); + const promise2 = guarded('guarded'); + assert.strictEqual(reentrantGuard.getQueueLength(), 0); + await Promise.all([promise1, promise2]); + logs.push('after concurrent call'); + assert.strictEqual(reentrantGuard.getQueueLength(), 0); + assert.deepStrictEqual(logs, [ + 'before concurrent call', + 'hello queueable', + 'bye', + 'after concurrent call' + ]); + }); + it('should prevent the function from leaking exceptions', async () => { + const old = reentrantGuard.setPrintError(error => { + logs.push('error: ' + error); + }); + try { + const willThrow = makeQueueableCommand(function() { + logs.push('will throw'); + throw 'error'; + }); + const wontThrow = makeQueueableCommand(function() { + logs.push('will not throw'); + }); + logs.push('before call'); + await willThrow(); + await wontThrow(); + logs.push('after call'); + assert.deepStrictEqual(logs, [ + 'before call', + 'will throw', + 'error: error', + 'will not throw', + 'after call' + ]); + } finally { + reentrantGuard.setPrintError(old); + } + }); + it('should invoke queued function even if another queued function threw exception (1)', async () => { + const old = reentrantGuard.setPrintError(error => { + logs.push('error: ' + error); + }); + try { + const willThrow = makeQueueableCommand(async function() { + logs.push('will throw...'); + await TestUtil.sleep(10); + logs.push('will throw now'); + throw 'error'; + }); + const wontThrow = makeQueueableCommand(async function() { + logs.push('will not throw'); + }); + logs.push('before concurrent call'); + const promise1 = willThrow(); + const promise2 = wontThrow(); + await Promise.all([promise1, promise2]); + logs.push('after concurrent call'); + assert.deepStrictEqual(logs, [ + 'before concurrent call', + 'will throw...', + 'will throw now', + 'error: error', + 'will not throw', + 'after concurrent call' + ]); + } finally { + reentrantGuard.setPrintError(old); + } + }) + it('should invoke queued function even if another queued function threw exception (2)', async () => { + const old = reentrantGuard.setPrintError(error => { + logs.push('error: ' + error); + }); + try { + const wontThrow1 = makeQueueableCommand(async function() { + logs.push('will not throw (1) begin'); + await TestUtil.sleep(10); + logs.push('will not throw (1) end'); + }); + const willThrow = makeQueueableCommand(async function() { + logs.push('will throw...'); + await TestUtil.sleep(10); + logs.push('will throw now'); + throw 'error'; + }); + const wontThrow2 = makeQueueableCommand(async function() { + logs.push('will not throw (2)'); + }); + logs.push('before concurrent call'); + const promise1 = wontThrow1(); + const promise2 = willThrow(); + await promise1; + const promise3 = wontThrow2(); + await Promise.all([promise2, promise3]); + logs.push('after concurrent call'); + assert.deepStrictEqual(logs, [ + 'before concurrent call', + 'will not throw (1) begin', + 'will not throw (1) end', + 'will throw...', + 'will throw now', + 'error: error', + 'will not throw (2)', + 'after concurrent call' + ]); + } finally { + reentrantGuard.setPrintError(old); + } + }) + }); });