parseNamedArgs re-scanned argv with indexOf/includes once per flag — O(flags * argv) — on the command-dispatch hot path (24 call sites across gsd-tools + init/state/validate routers). Build a first-index Map of argv tokens in a single pass and use it for the flag lookups, dropping it to O(argv + flags). Semantics are identical: firstIndex.get(t)??-1 === indexOf(t), firstIndex.has(t) === includes(t); first-occurrence-wins and the value-token rejection are preserved. Adds the first behavior-lock tests for the module. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
5
.changeset/312-arg-projection-single-pass.md
Normal file
5
.changeset/312-arg-projection-single-pass.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 312
|
||||
---
|
||||
Index argv once in parseNamedArgs instead of re-scanning per flag (O(flags*argv) -> O(argv+flags)) (#312).
|
||||
@@ -18,15 +18,22 @@
|
||||
* @returns {Record<string, string|boolean|null>}
|
||||
*/
|
||||
function parseNamedArgs(args, valueFlags = [], booleanFlags = []) {
|
||||
// Index each token's first position once (firstIndex.get(t) ?? -1 === args.indexOf(t),
|
||||
// firstIndex.has(t) === args.includes(t)) so the flag loops below don't each re-scan
|
||||
// argv — O(argv + flags) instead of O(flags * argv). Semantics are unchanged. (#312)
|
||||
const firstIndex = new Map();
|
||||
for (let i = 0; i < args.length; i++) {
|
||||
if (!firstIndex.has(args[i])) firstIndex.set(args[i], i);
|
||||
}
|
||||
const result = {};
|
||||
for (const flag of valueFlags) {
|
||||
const idx = args.indexOf(`--${flag}`);
|
||||
const idx = firstIndex.has(`--${flag}`) ? firstIndex.get(`--${flag}`) : -1;
|
||||
result[flag] = idx !== -1 && args[idx + 1] !== undefined && !args[idx + 1].startsWith('--')
|
||||
? args[idx + 1]
|
||||
: null;
|
||||
}
|
||||
for (const flag of booleanFlags) {
|
||||
result[flag] = args.includes(`--${flag}`);
|
||||
result[flag] = firstIndex.has(`--${flag}`);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
123
tests/command-arg-projection.test.cjs
Normal file
123
tests/command-arg-projection.test.cjs
Normal file
@@ -0,0 +1,123 @@
|
||||
'use strict';
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const {
|
||||
parseNamedArgs,
|
||||
parseMultiwordArg,
|
||||
} = require('../get-shit-done/bin/lib/command-arg-projection.cjs');
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// parseNamedArgs — behavior-lock tests (green before AND after the #312 fix)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('value flag with valid value', () => {
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs(['--name', 'foo'], ['name']),
|
||||
{ name: 'foo' }
|
||||
);
|
||||
});
|
||||
|
||||
test('value flag followed by another flag (value rejected)', () => {
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs(['--name', '--other'], ['name']),
|
||||
{ name: null }
|
||||
);
|
||||
});
|
||||
|
||||
test('value flag at end of array (no following token)', () => {
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs(['--name'], ['name']),
|
||||
{ name: null }
|
||||
);
|
||||
});
|
||||
|
||||
test('value flag absent from args', () => {
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs(['--x', 'y'], ['name']),
|
||||
{ name: null }
|
||||
);
|
||||
});
|
||||
|
||||
test('boolean flag present', () => {
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs(['--write'], [], ['write']),
|
||||
{ write: true }
|
||||
);
|
||||
});
|
||||
|
||||
test('boolean flag absent', () => {
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs([], [], ['write']),
|
||||
{ write: false }
|
||||
);
|
||||
});
|
||||
|
||||
test('first-occurrence-wins: duplicate value flag uses first index', () => {
|
||||
// Locks the indexOf-first semantics that the Map must preserve (#312)
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs(['--name', 'a', '--name', 'b'], ['name']),
|
||||
{ name: 'a' }
|
||||
);
|
||||
});
|
||||
|
||||
test('mixed multiple flags (the O(flags*argv) case)', () => {
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs(['--a', '1', '--flag', '--b', '2'], ['a', 'b'], ['flag']),
|
||||
{ a: '1', b: '2', flag: true }
|
||||
);
|
||||
});
|
||||
|
||||
test('empty args with multiple declared flags', () => {
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs([], ['name', 'path'], ['verbose', 'dry-run']),
|
||||
{ name: null, path: null, verbose: false, 'dry-run': false }
|
||||
);
|
||||
});
|
||||
|
||||
test('value flag value undefined via array boundary', () => {
|
||||
// --count is last token; args[idx+1] is undefined — must return null
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs(['--other', 'x', '--count'], ['count']),
|
||||
{ count: null }
|
||||
);
|
||||
});
|
||||
|
||||
test('boolean flag does not clobber an already-set value-flag key when names differ', () => {
|
||||
assert.deepStrictEqual(
|
||||
parseNamedArgs(['--msg', 'hello', '--verbose'], ['msg'], ['verbose']),
|
||||
{ msg: 'hello', verbose: true }
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// parseMultiwordArg — spot coverage for module completeness
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
test('parseMultiwordArg: collects tokens until next flag', () => {
|
||||
assert.strictEqual(
|
||||
parseMultiwordArg(['--msg', 'hello', 'world', '--x'], 'msg'),
|
||||
'hello world'
|
||||
);
|
||||
});
|
||||
|
||||
test('parseMultiwordArg: absent flag returns null', () => {
|
||||
assert.strictEqual(
|
||||
parseMultiwordArg(['--other', 'val'], 'msg'),
|
||||
null
|
||||
);
|
||||
});
|
||||
|
||||
test('parseMultiwordArg: flag present but no tokens returns null', () => {
|
||||
assert.strictEqual(
|
||||
parseMultiwordArg(['--msg', '--next'], 'msg'),
|
||||
null
|
||||
);
|
||||
});
|
||||
|
||||
test('parseMultiwordArg: flag at end of array with no tokens returns null', () => {
|
||||
assert.strictEqual(
|
||||
parseMultiwordArg(['--msg'], 'msg'),
|
||||
null
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user