fix(moarstats): catch clustered -o in --stats-options output guard#3882
Merged
Conversation
The joined-inputs guard rejected a caller-supplied -o/--output in
--stats-options with a naive `token.starts_with("-o")` check. That
missed docopt-style clustered short options: `-Eo joined.csv` expands
to `-E -o joined.csv`, so the joined `qsv stats` CSV could still be
redirected away from the captured stdout, reintroducing the empty-
capture "missing 'field' column" failure.
Add `stats_options_redirect_output()`, which expands clustered short
options using the no-argument short flags from `qsv stats` USAGE
(-E/-h/-n) and stops scanning at the first argument-taking option (so
`-so` — select a column named `o` — is not misread as `-s -o`). Add a
unit test covering plain, clustered, and false-positive cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens moarstats --join-inputs against caller-supplied output redirection in --stats-options by correctly detecting docopt-style clustered short options (e.g. -Eo file) that previously bypassed the -o/--output guard and could cause the joined qsv stats subprocess stdout capture to be empty.
Changes:
- Add
stats_options_redirect_output()to detect-o/--outputin--stats-options, including clustered short options while avoiding-sofalse-positives. - Replace the prior naive
starts_with("-o")guard with the new helper in the--join-inputspath. - Add unit tests covering plain, clustered, and false-positive cases.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Follow-up to #3881 (joined-inputs subprocess race fix). The joined-inputs guard rejects a caller-supplied
-o/--outputin--stats-optionsbecausemoarstatscaptures theqsv statssubprocess stdout itself. The guard used a naivetoken.starts_with("-o")check, which missed docopt-style clustered short options:-Eo joined.csvexpands to-E -o joined.csv, so the joined stats CSV could still be silently redirected away from the captured stdout — reintroducing the empty-capture "missing 'field' column" parse failure.Changes
stats_options_redirect_output(), which expands clustered short-option tokens. It treats the no-argument short flags fromqsv statsUSAGE (-E/-h/-n) as cluster-continuable and stops scanning at the first argument-taking option — so-so(select a column literally namedo) is correctly not misread as-s -o.starts_with("-o")guard in the--join-inputspath with the new helper.-o,--output,--output=), clustered (-Eo,-Eno), and false-positive (-so,--select output) cases.Testing
cargo t moarstats -F all_features— 88 passedcargo clippy -F all_features— cleanAddresses a roborev review finding (job 2332).
🤖 Generated with Claude Code