fix HelpError handling #36
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a HelpError is thrown (e.g., unknown command, no command specified), `parse()` and `parseAsync()` now catch it and handle gracefully: - Error message is printed to stderr - Help text is displayed to stderr - `process.exitCode` is set to 1 (no `process.exit()` call) - Returns result with `helpShown: true` flag This prevents HelpError from bubbling up to global exception handlers while still providing useful feedback to the user. Closes #31
Replace `process.exit()` with `process.exitCode` throughout `parseCore()`:
- `--help` now sets `process.exitCode = 0` and returns `{ helpShown: true }`
- `--version` now sets `process.exitCode = 0` and returns `{ helpShown: true }`
- `--completion-script` now sets appropriate exit code and returns
- `--get-bargs-completions` now sets exit code and returns
- `showNestedCommandHelp()` now returns a result instead of calling exit
This allows the process to terminate naturally, enabling proper cleanup
handlers and making the code more testable and composable.
There was a problem hiding this comment.
Pull request overview
This PR updates bargs to handle HelpError internally (showing help and setting process.exitCode instead of throwing) and removes process.exit()-based early termination paths.
Changes:
- Catch
HelpErrorinparse()/parseAsync()and return a result containinghelpShown: trueafter emitting help text and settingprocess.exitCode. - Replace
process.exit()usage in help/version/completion paths with anearlyExit()result. - Update tests to assert help output +
process.exitCoderather than expecting rejected promises.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/parser-commands.test.ts | Updates unknown-command test to assert help output + exitCode and helpShown. |
| test/bargs.test.ts | Adjusts unknown-command and HelpError tests to validate non-throwing behavior and stderr output capture. |
| src/types.ts | Extends CliBuilder.parse() / parseAsync() return types to optionally include helpShown. |
| src/bargs.ts | Implements HelpError catching, removes process.exit() usage, and adds early-exit/help-error handling helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Rename `helpShown` to `earlyExit` for clarity (issue #4) The flag is now accurately named since it's set for help, version, and completion output, not just help display. - Extract shared test helper `withCapturedStderr()` (issues #2, #3) Reduces code duplication in tests that capture stderr and exitCode. - Handle HelpError in nested command delegation (issue #1) `__parseWithParentGlobals()` now catches HelpError so nested builders can render their own help instead of bubbling up to the parent.
Restore standard CLI behavior where --help, --version, completion flags, and error conditions (unknown/missing commands) terminate the process. Changes: - `exitProcess()` calls `process.exit()` for help/version/completions - `handleHelpError()` calls `process.exit(1)` after displaying help - Remove `earlyExit` flag from return types (no longer needed) - Update tests to mock `process.exit` instead of checking return values - Document process termination behavior in README.md This is what users expect from a CLI - these flags print output and exit.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review feedback: - Extract `MockExitError` and `withMockedExit` to `test/helpers/mock-exit.ts` - Use `Promise.resolve().then(fn)` to handle any thenable, not just Promise - Consolidate duplicate JSDoc blocks for `handleHelpError` - Update README example to use sentinel error and `finally` block
boneskull
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a HelpError is thrown (e.g., unknown command, no command specified),
parse()andparseAsync()now catch it and handle gracefully:process.exitCodeis set to 1 (noprocess.exit()call)helpShown: trueflagThis prevents HelpError from bubbling up to global exception handlers
while still providing useful feedback to the user.
Refactors use of
process.exit()and documents exiting behavior.