Merge pull request #348 from peco/topic/issue-345

Add notion of "top-level" actions

action.go

@@ -324,6 +324,10 @@ func doSelectDown(ctx context.Context, state *Peco, e termbox.Event) {
 }
 
 func doSelectUp(ctx context.Context, state *Peco, e termbox.Event) {
+	if pdebug.Enabled {
+		g := pdebug.Marker("doSelectUp")
+		defer g.End()
+	}
 	state.Hub().SendPaging(ToLineAbove)
 }
 
@@ -344,7 +348,9 @@ func doScrollRight(ctx context.Context, state *Peco, e termbox.Event) {
 }
 
 func doToggleSelectionAndSelectNext(ctx context.Context, state *Peco, e termbox.Event) {
+	toplevel, _ := ctx.Value(isTopLevelActionCall).(bool)
 	state.Hub().Batch(func() {
+		ctx = context.WithValue(ctx, isTopLevelActionCall, false)
 		doToggleSelection(ctx, state, e)
 		// XXX This is sucky. Fix later
 		if state.LayoutType() == "top-down" {
@@ -352,7 +358,7 @@ func doToggleSelectionAndSelectNext(ctx context.Context, state *Peco, e termbox.
 		} else {
 			doSelectUp(ctx, state, e)
 		}
-	})
+	}, toplevel)
 }
 
 func doInvertSelection(ctx context.Context, state *Peco, _ termbox.Event) {
@@ -652,6 +658,11 @@ func doRefreshScreen(ctx context.Context, state *Peco, _ termbox.Event) {
 }
 
 func doToggleQuery(ctx context.Context, state *Peco, _ termbox.Event) {
+	if pdebug.Enabled {
+		g := pdebug.Marker("doToggleQuery")
+		defer g.End()
+	}
+
 	q := state.Query()
 	if q.Len() == 0 {
 		q.RestoreSavedQuery()
@@ -688,19 +699,23 @@ func doSingleKeyJump(ctx context.Context, state *Peco, e termbox.Event) {
 		return
 	}
 
+	toplevel, _ := ctx.Value(isTopLevelActionCall).(bool)
 	state.Hub().Batch(func() {
+		ctx = context.WithValue(ctx, isTopLevelActionCall, false)
 		state.Hub().SendPaging(JumpToLineRequest(index))
 		doFinish(ctx, state, e)
-	})
+	}, toplevel)
 }
 
 func makeCombinedAction(actions ...Action) ActionFunc {
 	return ActionFunc(func(ctx context.Context, state *Peco, e termbox.Event) {
+		toplevel, _ := ctx.Value(isTopLevelActionCall).(bool)
 		state.Hub().Batch(func() {
+			ctx = context.WithValue(ctx, isTopLevelActionCall, false)
 			for _, a := range actions {
 				a.Execute(ctx, state, e)
 			}
-		})
+		}, toplevel)
 	})
 }
 

hub/hub.go

@@ -39,10 +39,16 @@ func New(bufsiz int) *Hub {
 
 // Batch allows you to synchronously send messages during the
 // scope of f() being executed.
-func (h *Hub) Batch(f func()) {
-	// lock during this operation
-	h.mutex.Lock()
-	defer h.mutex.Unlock()
+func (h *Hub) Batch(f func(), shouldLock bool) {
+	if pdebug.Enabled {
+		g := pdebug.Marker("Batch %t", shouldLock)
+		defer g.End()
+	}
+	if shouldLock {
+		// lock during this operation
+		h.mutex.Lock()
+		defer h.mutex.Unlock()
+	}
 
 	// temporarily set isSync = true
 	o := h.isSync

hub/hub_test.go

@@ -50,7 +50,7 @@ func TestHub(t *testing.T) {
 		h.SendDraw(true)
 		h.SendStatusMsg("Hello, World!")
 		h.SendPaging(1)
-	})
+	}, true)
 
 	phases := []string{
 		"query",

issues_test.go

@@ -5,7 +5,9 @@ import (
 	"io/ioutil"
 	"os"
 	"testing"
+	"time"
 
+	termbox "github.com/nsf/termbox-go"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 )
@@ -68,3 +70,45 @@ func TestIssue212_SanityCheck(t *testing.T) {
 		return
 	}
 }
+
+func TestIssue345(t *testing.T) {
+	cfg, err := newConfig(`{
+	"Keymap": {
+    "C-t": "my.ToggleSelectionInAboveLine"
+	},
+	"Action": {
+		"my.ToggleSelectionInAboveLine": [
+			"peco.SelectUp",
+			"peco.ToggleSelectionAndSelectNext"
+		]
+	}
+}`)
+	if !assert.NoError(t, err, "newConfig should succeed") {
+		return
+	}
+	defer os.Remove(cfg)
+
+	state := newPeco()
+	if !assert.NoError(t, state.config.Init(), "Config.Init should succeed") {
+		return
+	}
+
+	state.Argv = append(state.Argv, []string{"--rcfile", cfg}...)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	go state.Run(ctx)
+	defer cancel()
+
+	<-state.Ready()
+
+	ev := termbox.Event{
+		Type: termbox.EventKey,
+		Key:  termbox.KeyCtrlT,
+	}
+	if !assert.NoError(t, state.Keymap().ExecuteAction(ctx, state, ev), "ExecuteAction should succeed") {
+		return
+	}
+
+	time.Sleep(time.Second)
+
+}

keymap.go

@@ -25,13 +25,21 @@ func (km Keymap) Sequence() Keyseq {
 	return km.seq
 }
 
-func (km Keymap) ExecuteAction(ctx context.Context, state *Peco, ev termbox.Event) error {
+const isTopLevelActionCall = "peco.isTopLevelActionCall"
+
+func (km Keymap) ExecuteAction(ctx context.Context, state *Peco, ev termbox.Event) (err error) {
+	if pdebug.Enabled {
+		g := pdebug.Marker("Keymap.ExecuteAction %v", ev).BindError(&err)
+		defer g.End()
+	}
+
 	a := km.LookupAction(ev)
 	if a == nil {
 		return errors.New("action not found")
 	}
 
-	a.(Action).Execute(ctx, state, ev)
+	ctx = context.WithValue(ctx, isTopLevelActionCall, true)
+	a.Execute(ctx, state, ev)
 	return nil
 }
 

peco.go

@@ -266,6 +266,10 @@ func (p *Peco) Run(ctx context.Context) (err error) {
 		defer g.End()
 	}
 
+	// do this only once
+	var readyOnce sync.Once
+	defer readyOnce.Do(func() { close(p.readyCh) })
+
 	if err := p.Setup(); err != nil {
 		return errors.Wrap(err, "failed to setup peco")
 	}
@@ -342,7 +346,7 @@ func (p *Peco) Run(ctx context.Context) (err error) {
 		}()
 	}
 
-	close(p.readyCh)
+	readyOnce.Do(func() { close(p.readyCh) })
 
 	// This has tobe AFTER close(p.readyCh), otherwise the query is
 	// ignored by us (queries are not run until peco thinks it's ready)

peco_test.go

@@ -3,6 +3,8 @@ package peco
 import (
 	"bytes"
 	"fmt"
+	"io"
+	"io/ioutil"
 	"runtime"
 	"sync"
 	"testing"
@@ -46,6 +48,17 @@ func (i *interceptor) record(name string, args []interface{}) {
 	events[name] = append(v, interceptorArgs(args))
 }
 
+func newConfig(s string) (string, error) {
+	f, err := ioutil.TempFile("", "peco-test-config-")
+	if err != nil {
+		return "", err
+	}
+
+	io.WriteString(f, s)
+	f.Close()
+	return f.Name(), nil
+}
+
 func newPeco() *Peco {
 	_, file, _, _ := runtime.Caller(0)
 	state := New()