Skip to content

fix: avoid retaining argument placeholder regex state - #258

Open
JunaidAhamed-7777 wants to merge 1 commit into
bcomnes:masterfrom
JunaidAhamed-7777:fix-empty-unpack-placeholders
Open

JunaidAhamed-7777 wants to merge 1 commit into
bcomnes:masterfrom
JunaidAhamed-7777:fix-empty-unpack-placeholders

Conversation

@JunaidAhamed-7777

Copy link
Copy Markdown

{%} should produce one command per argument, so an empty argument list should run no commands.
The shared global regular expressions retain lastIndex when empty expansion skips replacement, which can retain a later command or break placeholder expansion in a subsequent API call.

Use stateless String.search() checks for placeholder detection while retaining global replacement.
Add eight regressions covering empty expansion through the API and all three CLI entry points, repeated API calls, and later positional, unpacked, and mixed placeholders.
All eight new tests fail against the original implementation; the patched focused suite passes all 47 tests.

Validation on Windows with Node 26.8.1:

  • npm run check passes dependency compatibility, ESLint, unused-code, and TypeScript checks.
  • npm test exercises 276 tests: 272 pass, and four existing issue14 cases fail because rm is missing from the default Windows PATH.
  • Those same four cases also fail on the original code and pass on both original and patched code when the existing Git Unix tools are included in the process PATH.

Human review follows draft submission.

@JunaidAhamed-7777
JunaidAhamed-7777 marked this pull request as ready for review October 13, 2026 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant