refactor(cli/load,tests) Migrate display-message to typed wrapper#1039
Open
refactor(cli/load,tests) Migrate display-message to typed wrapper#1039
Conversation
why: The reattach loop reached for libtmux's `Server.cmd()` escape
hatch to invoke `tmux display-message -p '#S'` for the session
name, even though libtmux has exposed a typed
`Pane.display_message()` wrapper for this exact subcommand for
several releases. With libtmux 0.56.0 now in the dependency tree
(it broadens `display_message`'s flag coverage on top of the
existing get_text path), there's no remaining reason to keep this
particular cmd() escape.
what:
- Replace `builder.session.cmd("display-message", "-p", "'#S'")`
in `_reattach()` with
`active_pane.display_message("'#S'", get_text=True)` against
`builder.session.active_pane`. Same tmux invocation under the
hood; typed call site instead of stringly-typed positional args.
- Add a narrowing assert on `active_pane` (returns `Pane | None`
in libtmux's typing) so mypy can prove the call site is
well-typed. Mirrors the existing `assert builder.session is not
None` pattern at the top of `_reattach()`.
- Iterate the returned `list[str]` directly. Previously the loop
consumed `proc.stdout` (a list), so the iteration shape is
identical and the per-line `tmuxp_echo` / `logger.debug` body
is unchanged.
No behavior change: the active pane is what tmux's display-message
defaults to anyway when no `-t` is passed, so the underlying tmux
invocation is byte-for-byte identical.
why: Six tests in the plugin-system suite were querying tmux for
session/window names via the same `session.cmd("display-message",
"-p", "'#S'")` escape hatch that production just migrated away
from in `cli/load.py:_reattach`. Keeping the tests on the typed
wrapper that production now uses keeps the call shape consistent
across the codebase and removes the last `cmd("display-message",
...)` site from the repo.
what:
- `tests/cli/test_cli.py::test_reattach_plugins`: replace the
`cmd()` call with `active_pane.display_message("'#S'",
get_text=True)` so the post-reattach assertion uses the typed
wrapper.
- `tests/workspace/test_builder.py`: migrate four
`test_plugin_system_*` checks (`before_workspace_builder`,
`on_window_create`, `after_window_finished`, and the
multi-window variant covering both `before_script` and
`after_window_finished`) onto `active_pane.display_message()`.
The multi-window variant hoists the active_pane lookup once and
reuses it for both the `'#S'` and `'#W'` queries.
- Each call site adds a narrowing assert on `active_pane`
(libtmux types it as `Pane | None`) so mypy can prove the call
is well-typed. Mirrors the production assert added in
`_reattach()`.
No behavior change: tmux's `display-message` defaults to the
session's active pane when no `-t` is passed, so the underlying
tmux invocations are byte-for-byte identical to the prior
`session.cmd(...)` calls.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1039 +/- ##
==========================================
+ Coverage 81.96% 81.97% +0.01%
==========================================
Files 28 28
Lines 2545 2547 +2
Branches 485 485
==========================================
+ Hits 2086 2088 +2
Misses 328 328
Partials 131 131 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
cmd("display-message", "-p", ...)escape hatches in the codebase: one in production (cli/load._reattach), six in test code (the plugin-system suite).Pane.display_message(cmd, get_text=True)wrapper. No behavior change.Why now
PR #1038 broadened the
Pane.display_messageflag coverage in libtmux 0.56.0 (format_string=,verbose=,delay=,notify=,target_client=). While auditing tmuxp for spots that could move to the new typed surface, the only remainingcmd()escape in production turned out to be the post-reattach session-name echo loop incli/load._reattach. Five of the six other usages were in tests asserting on session/window names via the same idiom — leaving them oncmd()while production migrated would split the call shape across the codebase.The migration itself doesn't depend on a 0.56.0-only API; this is consistency cleanup that 0.56.0's broader display_message surface made an obvious moment to do.
Changes in this PR
c54893e5—cli/load(refactor[_reattach])UsePane.display_messagewrapperProduction change. Replaces:
with:
The narrowing assert mirrors the existing
assert builder.session is not Nonealready at the top of_reattach(). tmux'sdisplay-messagewithout-tdefaults to the session's active pane, so the underlying invocation is byte-for-byte identical.46b52f16—tests(refactor[plugin_system])UsePane.display_messagewrapperMigrates six test sites in
tests/cli/test_cli.py::test_reattach_pluginsand fourtest_plugin_system_*checks intests/workspace/test_builder.pyto the same typed wrapper. Hoistsactive_paneonce in the multi-query test that asserts on both'#S'and'#W'(replaces two separatecmd()roundtrips with two calls on the same pane reference).Test plan
uv run ruff check . --fix --show-fixesclean before each commituv run ruff format .clean before each commituv run mypyclean before each commit (the narrowing assert was load-bearing — without it,Pane | Nonetyping rejects the call)uv run py.test --reruns 0 -vvv— 797 passed, 2 skipped, before each commitjust build-docssucceeds before each commitcmd("display-message", ...)calls anywhere insrc/ortests/(verified viagrep -rn 'cmd("display-message"' src/ tests/)