GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||
GeezerVison is showing github.com — fix(workspace/builder): Wait for shell prompt before layout and commands by tony · Pull Request #1018 · tmux-python/tmuxp Conversation
Member
SummaryFixes #365 (open since March 2018). WorkspaceBuilder sends Root cause traced through tmux and zsh source:
Fix: Poll pane cursor position until it moves from origin Changes
Test plan
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions…layout and commands why: WorkspaceBuilder sends SIGWINCH (via select_layout) and commands (via send_keys) to panes before their shell finishes initializing (#365, open since 2018). With zsh, this triggers the PROMPT_SP partial-line marker — an inverse+bold "%" — because: tmux: screen_reinit() sets cx=cy=0 on new panes (screen.c:107-108). select_layout → layout_fix_panes → window_pane_resize enqueues a resize, and window_pane_send_resize delivers SIGWINCH via ioctl(TIOCSWINSZ) on the pane's PTY (window.c:449). zsh: preprompt() (utils.c:1530) runs the PROMPT_SP heuristic (utils.c:1545-1566) before every prompt. It outputs the PROMPT_EOL_MARK (default: "%B%S%#%s%b") to detect partial lines, then pads with spaces and returns the cursor. When SIGWINCH arrives mid-init, the interrupted redraw leaves the marker visible. what: - Add _wait_for_pane_ready() that polls cursor position until it moves from origin (0,0), indicating the shell has drawn its prompt - Call readiness check before select_layout in iter_create_panes() so SIGWINCH only arrives after the shell can handle resize gracefully - Wait for all default-shell panes, not just those with commands (blank panes via "- pane" shorthand expand to shell_command: []) - Remove redundant select_layout call in build() that doubled SIGWINCH signals after each pane yield - Add tests for readiness detection, timeout, call count, and layout deduplication
|
There was an error while loading. Please reload this page.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## master #1018 +/- ## ========================================== + Coverage 80.11% 80.32% +0.21% ========================================== Files 28 28 Lines 2409 2430 +21 Branches 457 462 +5 ========================================== + Hits 1930 1952 +22 - Misses 356 357 +1 + Partials 123 121 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| if "layout" in window_config: | ||
| window.select_layout(window_config["layout"]) | ||
|
|
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I did some code archaeology on this because I had the same concern about reintroducing old layout/space bugs.
This specific select_layout() call looks like the redundant one, not the essential one. The builder still applies select_layout() per-pane in iter_create_panes(), which is the part that actually preserves
the incremental rebalance during pane creation. The call removed here was the second pass in build(), so it was effectively doubling the layout application after each yielded pane.
The historical “no space for new pane” regressions were more closely tied to session geometry than to this duplicate call. In 2022, #793 changed both layout timing and default-size, and the fallout in #800
was mainly from the server starting at 80x24 before a client attached. The later fix path was to size new_session() correctly (8a59203, then 0777c3b/63ac676d), not to keep a second select_layout()
here.
I also checked the current regressions: test_issue_800_default_size_many_windows still passes, and this branch adds test_select_layout_not_called_after_yield specifically to lock in the intended behavior:
call select_layout() once per pane, not twice. So I don’t think removing this duplicate call should reintroduce the lack-of-space issue.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…iness check why: The except block in _wait_for_pane_ready discarded the exception traceback, making it impossible to diagnose pane refresh failures. what: - Add exc_info=True to logger.debug call in the except block
…ness skip coverage
why: The window_config.get("window_shell") fallback path at builder.py:566
was untested. Only pane-level shell: had fixture coverage.
what:
- Add skips_all_panes_with_window_shell fixture to PANE_READINESS_FIXTURES
- Verifies expected_wait_count=0 when window_shell is set
…c scope why: The readiness skip for panes with shell/window_shell was undocumented, making it unclear to future maintainers why pane_shell is None is the check. what: - Add comment explaining that shell/window_shell runs a command launcher, not an interactive shell, so there is no prompt to wait for
…o 5s why: The 2s timeout was too tight for slow environments, causing flaky doctest failures when the shell hadn't drawn its prompt yet. what: - Bump _wait_for_pane_ready doctest timeout from 2.0 to 5.0 seconds
why: Tmux doctests are inherently timing-sensitive — shell startup races can cause spurious failures even with generous timeouts. what: - Add pytest_collection_modifyitems hook to apply flaky(reruns=2) marker to DoctestItems from DOCTEST_NEEDS_TMUX modules
There was an error while loading. Please reload this page.
why: PR #1018 waits for each pane's shell to draw its prompt before the next pane is created, so workspace load time grew linearly with pane count (issue #1053: 12 windows / 18 panes took ~25s). The shells already initialize concurrently once tmux spawns them; only the observation was serial. what: - Add _wait_for_panes_ready(), a concurrent readiness barrier that polls every pane under one shared timeout instead of one per pane. - Restructure build() into phases: create all windows and panes, wait for every default-shell pane once, then lay out and send commands. - Create panes by splitting the newest pane only and defer the layout until after the barrier, so no pane is resized mid-prompt — keeping the zsh partial-line '%' fix (issue #365) intact. - Add _split_pane_reclaiming_space(): on a no-space split failure, wait for existing panes then select_layout and retry. - Keep iter_create_panes() as a per-window back-compat wrapper. - Reduce _wait_for_pane_ready() to a single-pane delegate. - Update readiness/layout tests for the barrier; add tests for the shared barrier, whole-session wait, and layout-after-barrier order.
why: PR #1018 waits for each pane's shell to draw its prompt before the next pane is created, so workspace load time grew linearly with pane count (issue #1053: 12 windows / 18 panes took ~25s). The shells already initialize concurrently once tmux spawns them; only the observation was serial. what: - Add _wait_for_panes_ready(), a concurrent readiness barrier that polls every pane under one shared timeout instead of one per pane. - Restructure build() into phases: create all windows and panes, wait for every default-shell pane once, then lay out and send commands. - Create panes by splitting the newest pane only and defer the layout until after the barrier, so no pane is resized mid-prompt — keeping the zsh partial-line '%' fix (issue #365) intact. - Add _split_pane_reclaiming_space(): on a no-space split failure, wait for existing panes then select_layout and retry. - Keep iter_create_panes() as a per-window back-compat wrapper. - Reduce _wait_for_pane_ready() to a single-pane delegate. - Update readiness/layout tests for the barrier; add tests for the shared barrier, whole-session wait, and layout-after-barrier order.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Outputs % / percent sign after every command in some panes
There was an error while loading. Please reload this page.