feat(cli)!: rename -s to -sc for --max-supercluster-size (#233) - #243
Merged
Conversation
TimD1-bot
force-pushed
the
232_td_error-unsorted-vcf
branch
2 times, most recently
from
August 11, 2026 16:58
c9814f3 to
42041e3
Compare
BREAKING CHANGE: --max-supercluster-size moves from the short form -s to -sc, matching the existing multi-character convention of -sv, -mq, -ct, -md and -ci. This frees the mnemonic -s for the stratification flag added under #47. No -s alias is retained. A silent alias would let an existing '-s 15000' invocation be reinterpreted as a file path once -s means stratification, so bare -s is rejected outright and an old command line fails loudly. Only the short form changes, so the long form and every prose reference to --max-supercluster-size still read correctly.
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.
Note
Authorship: the content below was drafted by Claude Opus 5 (an AI coding agent) and
filed via
ghunder @TimD1-bot, a bot account operated by @TimD1. It reflects theagent's analysis, not a statement authored by @TimD1.
Resolves #233. Part of #47.
Warning
Breaking, user-facing CLI change. Any existing command line passing
-sto vcfdist nowfails; see below.
Targets
devdirectly — the branch is rebased ontodevand carries a single commit.Change
--max-supercluster-sizemoves from the short form-sto-sc.-s-sc--max-supercluster-size-scmatches the multi-character convention already used by-sv,-mq,-ctand-ci. Thepoint of the move is to free the mnemonic
-sfor the stratification flag added under #47 — ashort form cannot mean two things at once.
Only the short form changes, so the long form still works, and the prose references to
--max-supercluster-sizeinsrc/cluster.cpp:580,977,1010and the validation message atsrc/globals.cpp:409all still read correctly and are untouched.No
-salias is retainedDeliberately. A silent alias would let an existing
-s 15000invocation be reinterpreted as afile path once
-smeans stratification — a confusing failure at best, a wrong run at worst.Bare
-snow falls through toUnexpected option '-s'and exits 1, so an old command linefails loudly instead of being silently reinterpreted.
Testing
ParseArgs.SuperclusterOkandParseArgs.SuperclusterTooSmallErrorsretargeted to-sc, aswas the
-l 200 -sc 100case inParseArgs.SuperclusterLtMaxSizePlus2Errors.ParseArgs.SuperclusterBareSDashRejectedpins bare-sas rejected, so the no-aliasdecision cannot be silently undone.
ParseArgs.SuperclusterLongFormOkpins the long form as unaffected by the rename.PrintUsage.ListsDocumentedFlagsnow requires the usage text to advertise-sc, --max-supercluster-size.test_max-supercluster-size_sc-short-form-acceptedruns theswallowed_snpsfixtures with
-sc 20000and pins counts identical to the default-flag run;test_max-supercluster-size_bare-s-rejectedruns the same invocation with-s 20000andrequires exit 1 with
Unexpected option '-s'.Verified locally on a clean rebuild of the rebased tree: 777/777 unit tests pass, 124/124
pytest cases pass (122 before, plus the two new integration cases).
-Wall -Wextrabuilds ofsrc/andtests/unit/build/produce no warnings, anddoxygen src/Doxyfilereports none.Blast radius
No in-repo caller passes vcfdist's
-s; the only occurrences were the unit tests updated here.No documentation under
docs/orREADME.mdmentions the flag.