diff --git a/action.go b/action.go index 8bc183f..42c90b6 100644 --- a/action.go +++ b/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) }) } diff --git a/hub/hub.go b/hub/hub.go index 9b88a30..5a28cd2 100644 --- a/hub/hub.go +++ b/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 diff --git a/hub/hub_test.go b/hub/hub_test.go index b6bdd3f..516d0dd 100644 --- a/hub/hub_test.go +++ b/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", diff --git a/issues_test.go b/issues_test.go index 940a4ba..b1462d5 100644 --- a/issues_test.go +++ b/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) + +} diff --git a/keymap.go b/keymap.go index a826a71..b159cb6 100644 --- a/keymap.go +++ b/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 } diff --git a/peco.go b/peco.go index 232ee24..9894269 100644 --- a/peco.go +++ b/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) diff --git a/peco_test.go b/peco_test.go index 2711080..c10ebd8 100644 --- a/peco_test.go +++ b/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()