Merge pull request #32 from tshino/wrap-queing

Make 'wrap' command queueable
tshino · Jan 11, 2022 · 131e8a6 · 131e8a6
2 parents fb92adb + 647d8bd
commit 131e8a6
Show file tree

Hide file tree

Showing 5 changed files with 354 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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,41 +21,76 @@ 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;
             }
         };
     };
 
     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
@@ -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(); // <--