Skip to content

Parse cronic's options in any order, and report usage errors - #130

Merged
mernst merged 3 commits into
plume-lib:masterfrom
mernst:cronic-options
Oct 9, 2026
Merged

mernst merged 3 commits into
plume-lib:masterfrom
mernst:cronic-options

Conversation

@mernst

@mernst mernst commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Split out of #129, which now handles only the shell options that the wrapped command runs with.

  • Accept --expected-status N, --expected-status=N, --permit-stderr, and --duplicate-stderr in any order, and -- to end the options.
  • Require --expected-status to be an integer from 0 to 255.
  • Report a missing command or an unknown option (including a single-dash one such as -v) as a usage error, with exit status 64.
  • Add -h and --help.
  • Parse the options before creating the temporary directory.
  • Prefix cronic's function names with cronic_, so that they do not hide a wrapped command named, for example, usage or cleanup.

* Accept `--expected-status N`, `--expected-status=N`, `--permit-stderr`, and
  `--duplicate-stderr` in any order, and `--` to end the options.
* Require `--expected-status` to be an integer from 0 to 255.
* Report a missing command or an unknown option (including a single-dash one
  such as `-v`) as a usage error, with exit status 64.
* Add `-h` and `--help`.
* Parse the options before creating the temporary directory.
* Prefix `cronic`'s function names with `cronic_`, so that they do not hide a
  wrapped command named, for example, `usage` or `cleanup`.
mernst added a commit to mernst/plume-scripts that referenced this pull request Oct 8, 2026
…r's shell options

* Option parsing and usage errors are now in plume-lib#130.
* Run the command with all of the caller's shell options, as `set +o` reports
  them, rather than only `-e`, `-u`, and `-x`.
* Turn off tracing of `cronic` only when xtrace came from an exported
  SHELLOPTS, so that `bash -x cronic` still traces `cronic`.
* Prefix `cronic`'s variables with `CRONIC_`, so that `cronic` does not change
  a variable such as `DEBUG` or `OUT` that the caller exported, and unset them
  before running the command, so that a builtin or shell function does not see
  them.
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 48 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: plume-lib/plume-scripts/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 2efc1bf6-0c78-4540-9fb9-e06bb414bb0a
📥 Commits

Reviewing files that changed from the base of the PR and between a9235f1 and 2986987.

📒 Files selected for processing (3)
  • README.md
  • cronic
  • tests/cronic-test/cronic-test.sh

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: plume-lib/plume-scripts/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 3f4f65b9-d641-4bc1-8664-6d12e83248b3
📥 Commits

Reviewing files that changed from the base of the PR and between 21f6ccc and a9235f1.

📒 Files selected for processing (2)
  • cronic
  • tests/cronic-test/cronic-test.sh

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Cronic now documents and parses --expected-status, --permit-stderr, and --duplicate-stderr before creating its temporary directory. It accepts options in any order, supports --expected-status in separate and = forms, and handles help and --. Invalid options, missing values, and missing commands produce usage errors. The cleanup function was renamed, and the exit and signal traps now call the renamed function. The tests cover these behaviors and add output-matching expectations that do not require a report marker.

Priority: ⬇️ Low

Change: Feature

Merge Risk: ⚪ Minimal · up to a9235

No actionable merge-blocking issue remains after normal checks.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

mernst and others added 2 commits October 8, 2026 09:37
…alue

Running the wrapped command as `command -- "$@"` keeps a program from being
shadowed by one of cronic's shell functions, whatever its name.  The tests now
check every function name, read from the script itself.

Also: report "--expected-status requires a value" when the value is missing,
point the README to `cronic --help`, and simplify the test's output checks.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@mernst

mernst commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mernst
mernst merged commit d37c363 into plume-lib:master Oct 9, 2026
7 checks passed
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