From a35076da6f94a0973b2edae63d8ae8ac7c03f4fc Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Sun, 15 Feb 2026 18:13:54 +0900 Subject: [PATCH] Fix #455 --- action.go | 2 +- action_test.go | 40 ++++++++++++++++++++++++- layout.go | 9 ++++++ layout_test.go | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ peco_test.go | 13 ++++++++ screen.go | 12 ++++++++ 6 files changed, 155 insertions(+), 2 deletions(-) diff --git a/action.go b/action.go index bb4713d..259b078 100644 --- a/action.go +++ b/action.go @@ -742,7 +742,7 @@ func doDeleteBackwardChar(ctx context.Context, state *Peco, e Event) { } func doRefreshScreen(ctx context.Context, state *Peco, _ Event) { - state.Hub().SendDraw(ctx, &DrawOptions{DisableCache: true}) + state.Hub().SendDraw(ctx, &DrawOptions{DisableCache: true, ForceSync: true}) } func doToggleQuery(ctx context.Context, state *Peco, _ Event) { diff --git a/action_test.go b/action_test.go index d433cfc..f399808 100644 --- a/action_test.go +++ b/action_test.go @@ -15,12 +15,13 @@ import ( "github.com/stretchr/testify/require" ) -// recordingHub wraps nullHub but records SendPaging and SendStatusMsg calls. +// recordingHub wraps nullHub but records SendPaging, SendStatusMsg, and SendDraw calls. type recordingHub struct { nullHub mu sync.Mutex pagingArgs []interface{} statusMsgs []string + drawArgs []interface{} } func (h *recordingHub) SendPaging(_ context.Context, v interface{}) { @@ -35,6 +36,20 @@ func (h *recordingHub) SendStatusMsg(_ context.Context, msg string) { h.statusMsgs = append(h.statusMsgs, msg) } +func (h *recordingHub) SendDraw(_ context.Context, v interface{}) { + h.mu.Lock() + defer h.mu.Unlock() + h.drawArgs = append(h.drawArgs, v) +} + +func (h *recordingHub) getDrawArgs() []interface{} { + h.mu.Lock() + defer h.mu.Unlock() + dst := make([]interface{}, len(h.drawArgs)) + copy(dst, h.drawArgs) + return dst +} + func (h *recordingHub) getPagingArgs() []interface{} { h.mu.Lock() defer h.mu.Unlock() @@ -56,6 +71,7 @@ func (h *recordingHub) reset() { defer h.mu.Unlock() h.pagingArgs = nil h.statusMsgs = nil + h.drawArgs = nil } func TestActionFunc(t *testing.T) { @@ -572,3 +588,25 @@ func TestGHIssue428_PgUpPgDnDefaultBindings(t *testing.T) { "PgUp should trigger ScrollPageUp") }) } + +// TestGHIssue455_RefreshScreenSendsForceSync verifies that doRefreshScreen +// sends DrawOptions with both DisableCache and ForceSync set to true. +func TestGHIssue455_RefreshScreenSendsForceSync(t *testing.T) { + ctx := context.Background() + rHub := &recordingHub{} + + state := New() + state.hub = rHub + state.selection = NewSelection() + state.currentLineBuffer = NewMemoryBuffer() + + doRefreshScreen(ctx, state, Event{}) + + drawArgs := rHub.getDrawArgs() + require.Len(t, drawArgs, 1, "expected exactly 1 SendDraw call") + + opts, ok := drawArgs[0].(*DrawOptions) + require.True(t, ok, "SendDraw argument should be *DrawOptions") + require.True(t, opts.DisableCache, "DisableCache should be true") + require.True(t, opts.ForceSync, "ForceSync should be true for screen refresh") +} diff --git a/layout.go b/layout.go index 39caa64..6c06459 100644 --- a/layout.go +++ b/layout.go @@ -333,6 +333,7 @@ func selectionContains(state *Peco, n int) bool { type DrawOptions struct { RunningQuery bool DisableCache bool + ForceSync bool } // Draw displays the ListArea on the screen @@ -710,6 +711,14 @@ func (l *BasicLayout) DrawScreen(state *Peco, options *DrawOptions) { l.DrawPrompt(state) l.list.Draw(state, l, perPage, options) + if options != nil && options.ForceSync { + type syncer interface{ Sync() } + if s, ok := l.screen.(syncer); ok { + s.Sync() + return + } + } + if err := l.screen.Flush(); err != nil { return } diff --git a/layout_test.go b/layout_test.go index 70d00f9..a8a3df8 100644 --- a/layout_test.go +++ b/layout_test.go @@ -285,3 +285,84 @@ func TestGHIssue460_MatchedStyleDoesNotBleedToEndOfLine(t *testing.T) { } }) } + +// TestGHIssue455_DrawScreenForceSync verifies that BasicLayout.DrawScreen +// calls Sync() (full redraw) instead of Flush() (differential) when +// DrawOptions.ForceSync is true. +func TestGHIssue455_DrawScreenForceSync(t *testing.T) { + setupState := func(t *testing.T) (*Peco, *SimScreen) { + t.Helper() + + screen := NewDummyScreen() + state := New() + state.screen = screen + state.skipReadConfig = true + state.Filters().Add(filter.NewIgnoreCase()) + + mb := NewMemoryBuffer() + mb.lines = append(mb.lines, line.NewRaw(0, "line one", false)) + state.currentLineBuffer = mb + + loc := state.Location() + loc.SetPage(1) + loc.SetPerPage(10) + loc.SetLineNumber(0) + + return state, screen + } + + t.Run("ForceSync true calls Sync instead of final Flush", func(t *testing.T) { + state, screen := setupState(t) + layout := NewDefaultLayout(state) + + screen.interceptor.reset() + layout.DrawScreen(state, &DrawOptions{DisableCache: true, ForceSync: true}) + + syncEvents := screen.interceptor.events["Sync"] + flushEvents := screen.interceptor.events["Flush"] + + require.Len(t, syncEvents, 1, "expected exactly 1 Sync call") + // DrawPrompt internally calls Flush, but the final DrawScreen + // Flush should be replaced by Sync. + for i, ev := range screen.interceptor.events["Flush"] { + t.Logf("Flush event %d: %v", i, ev) + } + for i, ev := range screen.interceptor.events["Sync"] { + t.Logf("Sync event %d: %v", i, ev) + } + // The prompt's Flush still fires, but the final screen Flush + // is replaced by Sync. So Flush count should be 1 less than + // the non-ForceSync case. + flushCountWithSync := len(flushEvents) + + // Compare against the non-ForceSync case + screen.interceptor.reset() + layout.DrawScreen(state, &DrawOptions{DisableCache: true, ForceSync: false}) + flushCountWithout := len(screen.interceptor.events["Flush"]) + + require.Equal(t, flushCountWithout-1, flushCountWithSync, + "ForceSync should replace exactly one Flush call with Sync") + }) + + t.Run("ForceSync false does not call Sync", func(t *testing.T) { + state, screen := setupState(t) + layout := NewDefaultLayout(state) + + screen.interceptor.reset() + layout.DrawScreen(state, &DrawOptions{DisableCache: true, ForceSync: false}) + + syncEvents := screen.interceptor.events["Sync"] + require.Empty(t, syncEvents, "expected no Sync calls when ForceSync is false") + }) + + t.Run("nil options does not call Sync", func(t *testing.T) { + state, screen := setupState(t) + layout := NewDefaultLayout(state) + + screen.interceptor.reset() + layout.DrawScreen(state, nil) + + syncEvents := screen.interceptor.events["Sync"] + require.Empty(t, syncEvents, "expected no Sync calls with nil options") + }) +} diff --git a/peco_test.go b/peco_test.go index ffd6ba8..d150f91 100644 --- a/peco_test.go +++ b/peco_test.go @@ -246,6 +246,19 @@ func (s *SimScreen) Size() (int, int) { func (s *SimScreen) Resume() {} func (s *SimScreen) Suspend() {} +// Sync records a "Sync" event via the interceptor. This satisfies the +// optional syncer interface used by BasicLayout.DrawScreen when +// ForceSync is requested. +func (s *SimScreen) Sync() { + s.mu.Lock() + defer s.mu.Unlock() + if s.closed { + return + } + s.record("Sync", interceptorArgs{}) + s.screen.Sync() +} + func TestIDGen(t *testing.T) { idgen := newIDGen() ctx, cancel := context.WithCancel(context.Background()) diff --git a/screen.go b/screen.go index 97a6b53..3efdb30 100644 --- a/screen.go +++ b/screen.go @@ -219,6 +219,18 @@ func (t *Termbox) Flush() error { return nil } +// Sync forces a complete redraw of every cell on the physical display. +// This recovers from screen corruption caused by external output (e.g., +// STDERR messages written directly to the terminal). +func (t *Termbox) Sync() { + t.mutex.Lock() + defer t.mutex.Unlock() + if t.screen == nil { + return + } + t.screen.Sync() +} + // PollEvent returns a channel that you can listen to for // terminal events. The actual polling is done in a // separate goroutine