Repository navigation
fix: preserve live selections during pane output - #57
Conversation
Port of upstream 963f78f (herdrdev#4193), adapted to the fork's popup selection and popup copy mode: explicit selections are live buffer ranges dropped only on resize or a screen switch, and mouse copies read the live range instead of a revision the server rejects once output lands. refs #54 (cherry picked from commit 963f78f)
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 SummarySummary by CodeRabbit
Walkthroughコンテンツ更新後も、マウスやキーボードで行った選択を維持するよう変更しました。コピー時は現在の選択範囲を読み取ります。サイズ変更や通常画面・代替画面の切り替えでは選択を解除します。検索結果由来の選択には、取得時のリビジョンを使います。 Changesコピー選択の動作
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
actor User
participant CopyMode as コピーモード
participant CopyRequest as request_selection_copy
participant Buffer as ペインまたはポップアップのバッファ
User->>CopyMode: 選択を確定
CopyMode->>CopyRequest: 選択種別を渡す
CopyRequest->>Buffer: 明示選択の現在の範囲を読み取る
CopyRequest->>Buffer: 検索結果を取得時のリビジョンで読み取る
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The selection and copy behavior appears ready to merge after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change affects which text is copied, rather than granting additional access. No introduced security defect was established, but late copy completion and restart behavior remain partly unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Popup copy mode recorded the client's hit size on entry but resynced against the popup frame size. Now that output keeps copy-mode selections, a popup resized to exactly the entry hit size no longer looked like a resize, so a pre-reflow selection survived and copied the wrong range. Record the frame size on entry, which also stops the first frame after entering from dropping a selection. refs #54
何が動いていなかったか
出力が続くペイン(スピナーやログが動いているシェル、scratch popup など、herdr が選択を処理するペイン)で、マウスのドラッグ選択がすぐ消え、離してもコピーされなかった。toast も出ないため、失敗に気付けない。
原因は 2 つある。
src/client/shell/state.rsの surface 更新時の判定(selection_content_changed)が、前後どちらかのフレームの content revision が奇数(PTY 書き込み途中のキャプチャ)であるか、選択したセルの文字が変わると、選択を破棄していた。popup 側の同等の判定(ed84d04b で入れたセル比較)も同じ。finish_pane_selection→request_selection_copy(outcome, false))が content revision 付きで送られていた。間に出力が 1 回でも挟まると、サーバがstale_contentで拒否し、何も表示されずに失敗する。upstream は 963f78f「fix: preserve live selections during pane output」(herdrdev#4193)で両方を直している。フォークのベースはそれより前なので入っていなかった。
修正
963f78f を cherry-pick し、フォーク独自の popup 選択と popup の copy mode にも同じ方針を適用した。
v/ Space /V)はバッファ上の範囲として保持する。破棄するのは、サイズ変更と normal/alternate 画面の切り替えのときだけにする。selection_text_forは revision なしを live 読みとして扱い、popup.selection.readもそのまま渡す。衝突の解消
actions.rs:フォークの.target_content_revision(&pane_id)(popup にも対応)を残し、コメントだけ upstream の文言にした。copy_mode.rs:target_content_state(id) -> Option<(u64, bool)>を足した。popup はメトリクスから、ペインは surface から revision と alternate 画面フラグを取る。target_content_revisionはこれを使う形にし、enter_copy_modeはalternate_screen_activeも記録する。exit_copy_modeは upstream のまま当たった。mouse.rs:フォークは離したときの処理をfinish_pane_selection(ペインと popup で共通)に切り出しているので、そこでrequest_selection_copy(outcome, true)にした。state.rs:pane_surface_row/selection_cells_unchangedに加え、使われなくなるフォーク独自のpopup_surface_row/popup_selection_cells_unchangedも消した。selection_content_changedをselection_invalidatedに改名した。ペイン側は幅・高さ・alternate 画面だけを見る。upstream の word-gesture 条件は、フォークに a0474ae(word_selection_gesture)が無いので取り込んでいない。resized、screen_switched、「このフレームと対になるメトリクスが無ければ破棄」を残し、content_movedを消した。alternate_screen_activeを足した。サイズか画面が変わったときだけcopy_mode.selectionを消す。revision が変わっただけなら、revision・サイズ・画面フラグの更新と検索状態のクリアだけを行う。upstream 側の末尾(pane.alternate_screen_activeを参照する行)は衝突マーカー無しで入るので、手で書き直した。tests/mouse_selection.rs:衝突範囲は upstream の word-drag テスト(a0474ae7、フォークには無い)なので、フォーク側を残してテスト名の変更だけを取り込んだ。本文の変更はそのまま当たった(upstream のselection_repaint_deadlineはフォークに無いので、assert!(drag.repaint)のまま)。docs/next/website/src/content/docs/{,ja/,zh-cn/}keyboard.mdx)はそのまま当たった。popup の copy mode のサイズ(追加コミット c07d8e1)
レビューで、この PR が持ち込んだ popup だけの退行が見つかったので直した。
yが折り返し直す前の範囲を revision なしで読んでいた。enter_copy_modeは、popup については popup フレームの寸法を記録するようにした。再同期が比べるのと同じ値になる。ペインはこれまでどおりペインの内側の矩形を使う(再同期も同じ値と比べる)。テスト
popup_output_inside_the_selection_drops_itをpopup_output_inside_the_selection_keeps_the_live_rangeに反転した。偶数・奇数の revision の両方で範囲が残り、離すと revision なしで読みに行くことを確かめる。popup_selection_copies_through_endpoint_extractionにcontent_revision.is_none()を加えた。popup_keyboard_selections_survive_output_and_copy_live_ranges/popup_empty_keyboard_anchor_keeps_search_fallback_revision_guard/popup_keyboard_selection_does_not_return_after_resize_or_screen_switch)。a_popup_frame_without_its_report_drops_the_selectionを足した。surface_with_popup()を使う。この fixture のフレーム(10 列)は、クライアントが計算する領域(枠 2 と予約列 1 を除いた 9 列)より 1 列広く、フレームが追いつく前の状態にあたる。popup_copy_selection_drops_when_a_lagging_frame_catches_up:追いついたフレームで copy mode の選択も消え、lで復元されず、yが何も読まないことを確かめる。keyboard_selection_on_the_alternate_screen_survives_output/popup_keyboard_selection_on_the_alternate_screen_survives_output)。client_double_click_selects_and_copies_endpoint_row_wordで確かめるようにした。popup_output_outside_the_selection_keeps_the_dragのコメントを、セル比較の説明から live 範囲の方針に直した。検証
worktree の
vendor/libghostty-vt/zig-outは main のビルド済みを複製し、ZIG=/usr/bin/trueで cargo を回した(ローカルの zig/SDK が壊れているため)。cargo fmt --check:OKcargo clippy --all-targets --locked -- -D warnings:OKcargo nextest run --locked --no-fail-fast:3354 件中 3353 件成功。失敗はlive_handoff::live_handoff_preserves_pane_process_ioの 1 件で、main でも落ちる既知の環境依存テスト。uv run python -m unittest scripts.test_ui_hot_path_architecture scripts.test_docs_translation_parity:OKcargo build --release --locked:OKfmtとclippyは OK。client::shell327/327、client::568/568、全体は 3361 件中 3360 件成功(失敗は上と同じlive_handoff_preserves_pane_process_io)。リリースビルドでハーネスのcopy --rounds 5 --text-mode fast-spinner --drag-step 0.04は 15/15、popup --rounds 2 --drag --drag-step 0.03 --click-delay 0.2 --timeout 2はbcdefghiをコピー、クリック失敗 0。diag_mouse.py)。before は導入済みの 0.9.0-fork.9、after はこのブランチの最初のコミットのリリースビルド。copy --rounds 5 --text-mode fast-spinner --drag-step 0.04selhold --rounds 4 --text-mode fast-spinnercopy --rounds 3 --text-mode staticcopy-click --rounds 6popup --rounds 2 --drag --drag-step 0.03 --click-delay 0.2 --timeout 2bcdefghiをコピー、クリック失敗 0consistency --rounds 3 --panes 3 --dragwidecopy --rounds 3 --text-mode append --drag-step 0.04参考の append は、この行数では実行中にビューがスクロールしなかった。
--rows 30 --rounds 2でスクロールさせた回は before 5/6、after 6/6。before の 1 件は、ドラッグ中に 1 行スクロールしてハーネスの固定座標が次の行を指したもので、選択の消失ではない。どちらも 1 回だけの試行で、ドラッグとスクロールが重なるかはタイミング次第。残課題・注意点
stale_contentで失敗しうる。mouse.rsのselection_scroll_metricsとtick_selection_autoscroll)。出力でドラッグが打ち切られなくなったので、履歴を遡った状態で追記が続くペインでは、ビューが最新側へ飛ぶことがある。upstream(現在の master)も同じコードなので、upstream に報告する候補。self.selectionだけで、copy mode の anchor は残る。その間にyを押すと、検索結果があればそれにフォールバックし、メトリクスが無いのでtarget_content_revisionがNoneになり、revision の確認なしに読む。既存の挙動で、起きるのはまれ。Some(revision)付きで送る(actions.rsのPluginAction)。copy mode の選択が出力で消えなくなったため、出力が続くペインで copy mode からプラグインのアクションを呼ぶとstale_contentで拒否される。upstream から引き継いだ挙動。pane_scroll_targetsは offset がちょうど一致したときにしか消えない(state.rsのset_pane_surface)。履歴を遡った状態で追記が速いペインでは、copy mode の offset が同期しなくなり、残っている選択が古い投影として描かれなくなる。yではその選択をコピーできる。既存の挙動。refs #54
🤖 Generated with Claude Code