Fix CLI declarations that disagree with runtime behaviour - #1082
Open
tony wants to merge 3 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## class-variables #1082 +/- ##
===================================================
- Coverage 82.55% 82.53% -0.02%
===================================================
Files 31 31
Lines 2768 2766 -2
Branches 518 518
===================================================
- Hits 2285 2283 -2
Misses 346 346
Partials 137 137 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
why: Three statements freeze makes about itself were wrong. --quiet advertised skipping confirmation, but it only gates informational output -- confirmations still run, and --yes is what skips them. --force advertised overwriting the workspace file, but it only relaxes the existence check inside the interactive destination prompt, so it does nothing when -o is given. save_to was typed str | None while -o parses with type=pathlib.Path. what: - Reword the --quiet and --force help to what each one does, and point --quiet at --yes for the behaviour it used to claim - Type save_to as pathlib.Path | None and convert at the one place the destination becomes a string - Pin all three with tests built from create_freeze_subparser
why: CLIColorsLiteral admitted 56, a value nothing produces. The -2 flag stores 256, so the type excluded the very value its own flag assigns and admitted one that cannot occur. what: - Declare CLIColorsLiteral as Literal[256, 88]
why: Three namespace fields were declared for flags that do not exist. The shell subparser defines no -2/-8 and no --log-file, and --version uses argparse's version action, which prints and exits with dest suppressed. Reading any of them raises AttributeError, so the annotations described a surface no caller could use. what: - Remove colors and log_file from CLIShellNamespace, and the colour literal left unused by their removal - Remove version from CLINamespace
tony
force-pushed
the
cli-annotation-fixes
branch
from
July 27, 2026 01:43
64cc513 to
6338d84
Compare
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
--quietand--forcehelp strings intmuxp freeze, both of which described behaviour the flags do not haveCLIFreezeNamespace.save_to, typedstr | Nonewhile-o/--save-toparses withtype=pathlib.PathCLIColorsLiteral, which admitted56(a value nothing produces) and excluded256(the value-2assigns)Each was found while documenting these classes: writing an accurate description of a field is what exposed that the field's declaration and its runtime behaviour disagreed.
Based on
class-variables, which supplies the docstrings these declarations belong to.Changes
src/tmuxp/cli/freeze.py--quiet: help said "don't prompt for confirmation". The flag only guards informational output — everyif not args.quietwraps atmuxp_echocall. Confirmation prompts still run;--yesis the flag that skips them. A user reaching for-qto get an unattended freeze would still be sitting at a prompt.--force: help said "overwrite the workspace file". It only relaxes the existence check inside the interactive destination prompt (if not args.force and os.path.exists(dest_prompt)). When-ois given there is no existence check at all, so--forceis a no-op on the path most users pair it with.save_to: declaredstr | None, but argparse stores aPosixPath. Correcting the annotation surfaced three genuine type errors that the wrong declaration had been masking — the destination is funnelled through a local that becomes astr. That conversion is now explicit at the boundary.src/tmuxp/cli/load.pyCLIColorsLiteralwasLiteral[56, 88].-2storesconst=256and-8storesconst=88, so the type admitted a value nothing produces and rejected one of the two the flags actually assign.56reads as a dropped digit from256.src/tmuxp/cli/shell.py,src/tmuxp/cli/__init__.pyCLIShellNamespace.colorsand.log_fileare declared, but theshellsubparser defines no-2/-8and no--log-file, andcommand_shellreads neither.CLINamespace.versionis declared, but--versionuses argparse's version action, which prints and exits withdestsuppressed. All three are absent from the parsed namespace.Verification
Confirm
-2yields the value the corrected literal admits:$ python -c "from tmuxp.cli import create_parser; print(create_parser().parse_args(['load','x','-2']).colors)"Confirm the removed annotations never had a value:
$ python -c "from tmuxp.cli import create_parser; n=create_parser().parse_args(['shell']); print(hasattr(n,'colors'), hasattr(n,'log_file'))"Test plan
test_freeze_save_to_parses_as_a_path— the parsed value is apathlib.Path, matching the declarationtest_freeze_quiet_help_describes_what_it_silences— help names output suppression and points at--yestest_freeze_force_help_scopes_itself_to_the_prompt— help scopes itself to the prompted destinationuv run ruff format/ruff checkcleanuv run mypy src/ tests/clean, including the three errors the correctedsave_totype exposed