fix(copy): address PR review feedback
- commit
- 46853d3f5de28f7053abe4c83730e02f045f292c
- author
- brice · · 2026-07-03 05:07 UTC
- parent
- f38c91b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +25 -4
M README.md +1 -1
| @@ -64,7 +64,7 @@ Scrollback mode freezes a view over history while the program keeps running unde | |||
| 64 | 64 | | y or Enter | copy selection and exit | | |
| 65 | 65 | | q, Esc, Ctrl-C | exit without copying | | |
| 66 | 66 | ||
| 67 | - | Copying uses OSC 52, so the selection lands in your terminal's clipboard even across SSH. After a successful copy, the right side of the status bar reports `copied N chars to clipboard` until the next screen update. | |
| 67 | + | Copying uses OSC 52, so the selection lands in your terminal's clipboard even across SSH. After a successful copy, the right side of the status bar reports `copied N chars to clipboard` until the next screen update. If the selection exceeds the OSC 52 copy limit, the status bar reports `selection too large to copy` instead. | |
| 68 | 68 | ||
| 69 | 69 | ## Mouse | |
| 70 | 70 | ||
| @@ -64,7 +64,7 @@ Scrollback mode freezes a view over history while the program keeps running unde | |||||
| 64 | | y or Enter | copy selection and exit | | 64 | | y or Enter | copy selection and exit | | ||
| 65 | | q, Esc, Ctrl-C | exit without copying | | 65 | | q, Esc, Ctrl-C | exit without copying | | ||
| 66 | 66 | ||||
| 67 | - | Copying uses OSC 52, so the selection lands in your terminal's clipboard even across SSH. After a successful copy, the right side of the status bar reports `copied N chars to clipboard` until the next screen update. | 67 | + | Copying uses OSC 52, so the selection lands in your terminal's clipboard even across SSH. After a successful copy, the right side of the status bar reports `copied N chars to clipboard` until the next screen update. If the selection exceeds the OSC 52 copy limit, the status bar reports `selection too large to copy` instead. |
| 68 | 68 | ||||
| 69 | ## Mouse | 69 | ## Mouse | ||
| 70 | 70 | ||||
M internal/usecase/daemon/daemon.go +8 -3
| @@ -240,6 +240,11 @@ func (d *Daemon) boundedSendErr(ac *attachedClient, f ports.Frame) error { | |||
| 240 | 240 | timer.Stop() | |
| 241 | 241 | return err | |
| 242 | 242 | case <-timer.C(): | |
| 243 | + | select { | |
| 244 | + | case err := <-result: | |
| 245 | + | return err | |
| 246 | + | default: | |
| 247 | + | } | |
| 243 | 248 | _ = ac.tr.Close() | |
| 244 | 249 | return errors.New("send timed out") | |
| 245 | 250 | } | |
| @@ -1801,11 +1806,11 @@ func drawStatus(row []renderer.Cell, sess *session, rightText string) { | |||
| 1801 | 1806 | return | |
| 1802 | 1807 | } | |
| 1803 | 1808 | style := renderer.DefaultStyle() | |
| 1804 | - | start := max(len(row)-len([]rune(rightText))-1, x+1) | |
| 1805 | - | if start >= len(row) { | |
| 1809 | + | rightWidth := len([]rune(rightText)) + 1 | |
| 1810 | + | if len(row)-x-1 < rightWidth { | |
| 1806 | 1811 | return | |
| 1807 | 1812 | } | |
| 1808 | - | x = start | |
| 1813 | + | x = len(row) - rightWidth | |
| 1809 | 1814 | writeStatusText(row, &x, " "+rightText, style) | |
| 1810 | 1815 | } | |
| 1811 | 1816 | ||
| @@ -240,6 +240,11 @@ func (d *Daemon) boundedSendErr(ac *attachedClient, f ports.Frame) error { | |||||
| 240 | timer.Stop() | 240 | timer.Stop() | ||
| 241 | return err | 241 | return err | ||
| 242 | case <-timer.C(): | 242 | case <-timer.C(): | ||
| 243 | + | select { | |||
| 244 | + | case err := <-result: | |||
| 245 | + | return err | |||
| 246 | + | default: | |||
| 247 | + | } | |||
| 243 | _ = ac.tr.Close() | 248 | _ = ac.tr.Close() | ||
| 244 | return errors.New("send timed out") | 249 | return errors.New("send timed out") | ||
| 245 | } | 250 | } | ||
| @@ -1801,11 +1806,11 @@ func drawStatus(row []renderer.Cell, sess *session, rightText string) { | |||||
| 1801 | return | 1806 | return | ||
| 1802 | } | 1807 | } | ||
| 1803 | style := renderer.DefaultStyle() | 1808 | style := renderer.DefaultStyle() | ||
| 1804 | - | start := max(len(row)-len([]rune(rightText))-1, x+1) | 1809 | + | rightWidth := len([]rune(rightText)) + 1 |
| 1805 | - | if start >= len(row) { | 1810 | + | if len(row)-x-1 < rightWidth { |
| 1806 | return | 1811 | return | ||
| 1807 | } | 1812 | } | ||
| 1808 | - | x = start | 1813 | + | x = len(row) - rightWidth |
| 1809 | writeStatusText(row, &x, " "+rightText, style) | 1814 | writeStatusText(row, &x, " "+rightText, style) | ||
| 1810 | } | 1815 | } | ||
| 1811 | 1816 | ||||
M internal/usecase/daemon/daemon_test.go +16 -0
| @@ -1241,6 +1241,22 @@ func TestStatusCompositionGolden(t *testing.T) { | |||
| 1241 | 1241 | } | |
| 1242 | 1242 | } | |
| 1243 | 1243 | ||
| 1244 | + | func TestStatusCopyFeedbackRendersOnlyWhenFullyFits(t *testing.T) { | |
| 1245 | + | p, releasePTY := newBlockingPTY(t) | |
| 1246 | + | _, sess, _, _ := newManualSessionWithPTYs(t, p) | |
| 1247 | + | defer releasePTY() | |
| 1248 | + | sess.name = "work" | |
| 1249 | + | win := sess.activeTab() | |
| 1250 | + | win.screen = vt.NewScreen(30, 2) | |
| 1251 | + | win.size = domain.Size{Cols: 30, Rows: 2} | |
| 1252 | + | ||
| 1253 | + | frame, _ := composeClientFrame(sess, win, true, "ok") | |
| 1254 | + | require.Equal(t, " work 1 ok", rowText(frame.Row(2))) | |
| 1255 | + | ||
| 1256 | + | frame, _ = composeClientFrame(sess, win, true, "selection too large to copy") | |
| 1257 | + | require.Equal(t, " work 1 ", rowText(frame.Row(2))) | |
| 1258 | + | } | |
| 1259 | + | ||
| 1244 | 1260 | func TestStatusRepaintsOnCreateSwitchAndResize(t *testing.T) { | |
| 1245 | 1261 | p1, releasePTY1 := newBlockingPTY(t) | |
| 1246 | 1262 | p2, releasePTY2 := newBlockingPTY(t) | |
| @@ -1241,6 +1241,22 @@ func TestStatusCompositionGolden(t *testing.T) { | |||||
| 1241 | } | 1241 | } | ||
| 1242 | } | 1242 | } | ||
| 1243 | 1243 | ||||
| 1244 | + | func TestStatusCopyFeedbackRendersOnlyWhenFullyFits(t *testing.T) { | |||
| 1245 | + | p, releasePTY := newBlockingPTY(t) | |||
| 1246 | + | _, sess, _, _ := newManualSessionWithPTYs(t, p) | |||
| 1247 | + | defer releasePTY() | |||
| 1248 | + | sess.name = "work" | |||
| 1249 | + | win := sess.activeTab() | |||
| 1250 | + | win.screen = vt.NewScreen(30, 2) | |||
| 1251 | + | win.size = domain.Size{Cols: 30, Rows: 2} | |||
| 1252 | + | ||||
| 1253 | + | frame, _ := composeClientFrame(sess, win, true, "ok") | |||
| 1254 | + | require.Equal(t, " work 1 ok", rowText(frame.Row(2))) | |||
| 1255 | + | ||||
| 1256 | + | frame, _ = composeClientFrame(sess, win, true, "selection too large to copy") | |||
| 1257 | + | require.Equal(t, " work 1 ", rowText(frame.Row(2))) | |||
| 1258 | + | } | |||
| 1259 | + | ||||
| 1244 | func TestStatusRepaintsOnCreateSwitchAndResize(t *testing.T) { | 1260 | func TestStatusRepaintsOnCreateSwitchAndResize(t *testing.T) { | ||
| 1245 | p1, releasePTY1 := newBlockingPTY(t) | 1261 | p1, releasePTY1 := newBlockingPTY(t) | ||
| 1246 | p2, releasePTY2 := newBlockingPTY(t) | 1262 | p2, releasePTY2 := newBlockingPTY(t) | ||