fix: --no-cmux double-render + rebind preview scroll keys - #4
Merged
Conversation
Two issues reported after the in-process-preview ship: ## 1. `--no-cmux` opened *both* panels Steps that triggered the bug: $ cmux # running cmux daemon $ mdmux --no-cmux ~/notes > enter on README.md The user expected one panel (in-process). They got two: cmux opened a sibling surface *and* mdmux's own preview pane rendered the same file. Cause: `App::open_selected` always called `self.cmux.open_markdown`, even with `render_mode == InProcess`. The CLI flag swapped the render mode but the client stayed as the live `CliCmux`, so every Enter shelled out to a working cmux daemon in addition to loading the preview. Fix: branch on `render_mode` in `open_selected` and `close_markdown_panel`. The cmux path runs only when we own a cmux surface; the in-process path runs only when we own the preview pane. Mutually exclusive. ## 2. Preview scroll keys were the wrong way around Requested binding: j / k — page scroll (full page) J / K — half-page scroll Was: J / K — one line at a time Ctrl-D / U — half page New mapping: j / k — full page scroll *when a preview is showing* J / K — half-page scroll Ctrl-D / U — half-page scroll (kept as vim-style alias) ↑ / ↓ — tree navigation (unchanged) j / k — tree navigation when no preview is open (unchanged) The `j` / `k` guards are conditional on `app.preview.is_some()` so cmux users (no in-process preview) and in-process users who haven't opened a file yet still get the original tree-navigation behaviour. With a preview open, the arrow keys take over tree movement. ## Tests - New: `close_markdown_panel_in_inprocess_does_not_touch_cmux` — sets a bogus surface id, calls close, asserts the surface is *not* taken (cmux client untouched) but the preview *is* cleared. - Tightened: `open_selected_populates_preview_in_inprocess_mode` now also asserts `current_md_surface.is_none()`, which is the direct regression assertion for issue 1. - Tightened: `open_selected_skips_preview_in_cmux_mode` symmetrically asserts `current_md_surface.is_some()`. `cargo test --lib`: **49 passed**, clippy + fmt clean. ## Docs - README key table: `j`/`k` listed for page scroll (in-process, preview open), `J`/`K` for half-page, `Ctrl-D`/`U` as alias. - README "Without cmux" section: spells out that with a preview open `j`/`k` switch to scrolling and tree navigation falls back to arrows. - In-app help screen mirrors the same. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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
1 participant
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.
Two reported bugs from the in-process-preview ship
1.
--no-cmuxopened both panelsSteps that triggered it:
Expected: one panel (in-process). Got: two. cmux opened a sibling surface and mdmux's own preview pane rendered the same file.
Cause.
App::open_selectedalways calledself.cmux.open_markdown, regardless ofrender_mode. The CLI flag swapped the render mode but the client stayed as the liveCliCmux, so every Enter shelled out to the real cmux daemon in addition to loading the preview.Fix. Branch on
render_modeinopen_selectedandclose_markdown_panel. The cmux path runs only when we own a cmux surface; the in-process path runs only when we own the preview pane. Mutually exclusive.2. Preview scroll keys were the wrong way around
J/K→ one lineJ/K→ half pageCtrl-D/U→ half pageCtrl-D/U→ half page (kept as alias)j/k→ full page when preview is openThe
j/karms are guarded onapp.preview.is_some(), so:j/kkeep navigating the tree, identical to today.j/kmove the tree.j/kscroll the preview; tree navigation falls back to arrow keys.Tests
open_selected_populates_preview_in_inprocess_mode(tightened)current_md_surface.is_none()— direct regression for issue 1open_selected_skips_preview_in_cmux_mode(tightened)current_md_surface.is_some()in cmux modeclose_markdown_panel_in_inprocess_does_not_touch_cmux(new)cargo test --lib→ 49 passed. Clippy + fmt clean.Docs
j/k(page),J/K(half-page),Ctrl-D/U(alias)?) updated to matchTest plan
cmuxrunning,mdmux --no-cmux ~/notes, press Enter → only the in-process preview opens, no cmux pane spawnedj/kscroll the preview a full pageJ/Kscroll half a page↑/↓move the tree selectionj/kstill move the tree (unchanged)🤖 Generated with Claude Code