Fix a race condition that existed between clear status and print status

For whatever reason I was thinking at the time, I had split the channels
to receive request to print to and clear the status message. This
basically means that, even if you meant to print X and then clear it
in 500 milliseconds, the clear request could actually arrive BEFORE
the print message.

This change basically puts all the status related request into one
channel, so that all requests can come sequentially

hub.go

@@ -141,18 +141,11 @@ func (h *Hub) StatusMsgCh() chan HubReq {
 
 // SendStatusMsg sends a string to be displayed in the status message
 func (h *Hub) SendStatusMsg(q string) {
-	send(h.StatusMsgCh(), HubReq{q, nil}, h.isSync)
+	h.SendStatusMsgAndClear(q, 0)
 }
 
-func (h *Hub) ClearStatusCh() chan HubReq {
-	return h.clearStatusCh
-}
-
-// SendClearStatus sends a request to clear the status message in
-// `d` duration. If a new status message is sent before the clear
-// request is executed, the clear instruction will be canceled
-func (h *Hub) SendClearStatus(d time.Duration) {
-	send(h.ClearStatusCh(), HubReq{d, nil}, h.isSync)
+func (h *Hub) SendStatusMsgAndClear(q string, clearDelay time.Duration) {
+	send(h.StatusMsgCh(), HubReq{StatusMsgRequest{q, clearDelay}, nil}, h.isSync)
 }
 
 // PagingCh returns the channel to page through the results

hub_test.go

@@ -27,19 +27,14 @@ func TestHub(t *testing.T) {
 	}()
 	go func() {
 		hr := <-h.StatusMsgCh()
-		if hr.DataString() != "Hello, World!" {
-			t.Errorf("Expected data to be 'Hello World!', got '%s'", hr.DataString())
+		r := hr.DataInterface().(StatusMsgRequest)
+		if r.message != "Hello, World!" {
+			t.Errorf("Expected data to be 'Hello World!', got '%s'", r.message)
 		}
 		time.Sleep(100 * time.Millisecond)
 		done["status"] = time.Now()
 		hr.Done()
 	}()
-	go func() {
-		hr := <-h.ClearStatusCh()
-		time.Sleep(100 * time.Millisecond)
-		done["clearStatus"] = time.Now()
-		hr.Done()
-	}()
 	go func() {
 		hr := <-h.PagingCh()
 		time.Sleep(100 * time.Millisecond)
@@ -51,7 +46,6 @@ func TestHub(t *testing.T) {
 		h.SendQuery("Hello World!")
 		h.SendDraw(nil)
 		h.SendStatusMsg("Hello, World!")
-		h.SendClearStatus(time.Second)
 		h.SendPaging(ToLineAbove)
 	})
 
@@ -59,7 +53,6 @@ func TestHub(t *testing.T) {
 		"query",
 		"draw",
 		"status",
-		"clearStatus",
 		"paging",
 	}
 

keymap.go

@@ -63,11 +63,10 @@ func wrapClearSequence(a Action) Action {
 		}
 
 		if len(i.currentKeySeq) > 0 {
-			i.SendStatusMsg(strings.Join(i.currentKeySeq, " "))
+			i.SendStatusMsgAndClear(strings.Join(i.currentKeySeq, " "), 500 * time.Millisecond)
 			i.currentKeySeq = []string{}
 		}
 
-		i.SendClearStatus(500 * time.Millisecond)
 		a.Execute(i, ev)
 	})
 }

layout.go

@@ -2,6 +2,7 @@ package peco
 
 import (
 	"fmt"
+	"sync"
 	"time"
 	"unicode/utf8"
 
@@ -42,8 +43,7 @@ func IsValidVerticalAnchor(anchor VerticalAnchor) bool {
 
 // Layout represents the component that controls where elements are placed on screen
 type Layout interface {
-	ClearStatus(time.Duration)
-	PrintStatus(string)
+	PrintStatus(string, time.Duration)
 	DrawPrompt()
 	DrawScreen([]Match)
 	MovePage(PagingRequest)
@@ -188,6 +188,7 @@ type StatusBar struct {
 	*Ctx
 	*AnchorSettings
 	clearTimer *time.Timer
+	timerMutex *sync.Mutex
 }
 
 // NewStatusBar creates a new StatusBar struct
@@ -196,29 +197,26 @@ func NewStatusBar(ctx *Ctx, anchor VerticalAnchor, anchorOffset int) *StatusBar
 		ctx,
 		NewAnchorSettings(anchor, anchorOffset),
 		nil,
+		&sync.Mutex{},
 	}
 }
 
 func (s *StatusBar) stopTimer() {
+	s.timerMutex.Lock()
+	defer s.timerMutex.Unlock()
 	if t := s.clearTimer; t != nil {
 		t.Stop()
+		s.clearTimer = nil
 	}
 }
 
-// ClearStatus clears the string displayed in the status bar area
-// after `d time.Duration`.
-func (s *StatusBar) ClearStatus(d time.Duration) {
-	s.stopTimer()
-	s.clearTimer = time.AfterFunc(d, func() {
-		s.PrintStatus("")
-	})
-}
-
 // PrintStatus prints a new status message. This also resets the
 // timer created by ClearStatus()
-func (s *StatusBar) PrintStatus(msg string) {
+func (s *StatusBar) PrintStatus(msg string, clearDelay time.Duration) {
 	s.stopTimer()
 
+	s.timerMutex.Lock()
+
 	location := s.AnchorPosition()
 
 	w, _ := screen.Size()
@@ -248,6 +246,16 @@ func (s *StatusBar) PrintStatus(msg string) {
 		printScreen(w-width, location, fgAttr|termbox.AttrReverse|termbox.AttrBold, bgAttr|termbox.AttrReverse, msg, false)
 	}
 	screen.Flush()
+
+	s.timerMutex.Unlock()
+
+	// if everything is successful AND the clearDelay timer is specified,
+	// then set a timer to clear the status
+	if clearDelay != 0 {
+		s.clearTimer = time.AfterFunc(clearDelay, func() {
+			s.PrintStatus("", 0)
+		})
+	}
 }
 
 // ListArea represents the area where the actual line buffer is

layout_test.go

@@ -2,7 +2,6 @@ package peco
 
 import (
 	"testing"
-	"time"
 	"unicode/utf8"
 
 	"github.com/mattn/go-runewidth"
@@ -111,16 +110,11 @@ func TestStatusBar(t *testing.T) {
 	defer guard()
 
 	st := NewStatusBar(NewCtx(nil), AnchorBottom, 0)
-	st.PrintStatus("Hello, World!")
+	st.PrintStatus("Hello, World!", 0)
 
 	events := i.events
 	if l := len(events["Flush"]); l != 1 {
 		t.Errorf("Expected 1 Flush event, got %d", l)
 		return
 	}
-
-	// XXX This is not a test... :/
-	// need to think of a way to verify that we're actually doing something
-	st.ClearStatus(500 * time.Millisecond)
-	<-time.After(time.Second)
 }

view.go

@@ -22,6 +22,14 @@ const (
 	ToScrollPageUp
 )
 
+// StatusMsgRequest specifies the string to be drawn
+// on the status message bar and an optional delay that tells
+// the view to clear that message
+type StatusMsgRequest struct {
+	message string
+	clearDelay time.Duration
+}
+
 // Loop receives requests to update the screen
 func (v *View) Loop() {
 	defer v.ReleaseWaitGroup()
@@ -30,10 +38,7 @@ func (v *View) Loop() {
 		case <-v.LoopCh():
 			return
 		case m := <-v.StatusMsgCh():
-			v.printStatus(m.DataString())
-			m.Done()
-		case m := <-v.ClearStatusCh():
-			v.clearStatus(m.DataInterface().(time.Duration))
+			v.printStatus(m.DataInterface().(StatusMsgRequest))
 			m.Done()
 		case r := <-v.PagingCh():
 			v.movePage(r.DataInterface().(PagingRequest))
@@ -54,12 +59,8 @@ func (v *View) Loop() {
 	}
 }
 
-func (v *View) printStatus(m string) {
-	v.layout.PrintStatus(m)
-}
-
-func (v *View) clearStatus(d time.Duration) {
-	v.layout.ClearStatus(d)
+func (v *View) printStatus(r StatusMsgRequest) {
+	v.layout.PrintStatus(r.message, r.clearDelay)
 }
 
 func (v *View) drawScreenNoLock(targets []Match) {