Merge pull request #588 from peco/gh-455

Fix #455

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) {

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")
+}

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
 	}

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")
+	})
+}

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())

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