From 8c7f63ac15862bd8ecafbcafc5ec45d6e1b00fef Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Wed, 24 Apr 2024 22:50:00 +0200 Subject: [PATCH 1/5] infopane: DoKeyEvent: ignore action return value It is not really defined what is the meaning of this return value. Currently this value is always true. And even if this value actually meant something (for example, the result of the last executed action in the chain), we should not use this value in HandleEvent(). The key event handling logic should behave the same regardless of whether the action triggered by this key succeeded or not. --- internal/action/infopane.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/action/infopane.go b/internal/action/infopane.go index a9baf431..c46faaf4 100644 --- a/internal/action/infopane.go +++ b/internal/action/infopane.go @@ -140,9 +140,9 @@ func (h *InfoPane) DoKeyEvent(e KeyEvent) bool { if !more { action, more = InfoBufBindings.NextEvent(e, nil) if action != nil && !more { - done := action(h.BufPane) + action(h.BufPane) InfoBufBindings.ResetEvents() - return done + return true } else if action == nil && !more { InfoBufBindings.ResetEvents() } From 36bf3f6619a3643a2b22fb89009b3a7341b447b0 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Wed, 24 Apr 2024 23:21:28 +0200 Subject: [PATCH 2/5] DoKeyEvent: document return value The return value of DoKeyEvent() has a dual meaning, which makes the code not obvious and confusing. So at least document it. --- internal/action/bufpane.go | 5 ++++- internal/action/infopane.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/action/bufpane.go b/internal/action/bufpane.go index ecc14f6a..084ccd62 100644 --- a/internal/action/bufpane.go +++ b/internal/action/bufpane.go @@ -540,7 +540,10 @@ func (h *BufPane) Bindings() *KeyTree { } // DoKeyEvent executes a key event by finding the action it is bound -// to and executing it (possibly multiple times for multiple cursors) +// to and executing it (possibly multiple times for multiple cursors). +// Returns true if the action was executed OR if there are more keys +// remaining to process before executing an action (if this is a key +// sequence event). Returns false if no action found. func (h *BufPane) DoKeyEvent(e Event) bool { binds := h.Bindings() action, more := binds.NextEvent(e, nil) diff --git a/internal/action/infopane.go b/internal/action/infopane.go index c46faaf4..32afb8d8 100644 --- a/internal/action/infopane.go +++ b/internal/action/infopane.go @@ -124,7 +124,10 @@ func (h *InfoPane) HandleEvent(event tcell.Event) { } } -// DoKeyEvent executes a key event for the command bar, doing any overridden actions +// DoKeyEvent executes a key event for the command bar, doing any overridden actions. +// Returns true if the action was executed OR if there are more keys remaining +// to process before executing an action (if this is a key sequence event). +// Returns false if no action found. func (h *InfoPane) DoKeyEvent(e KeyEvent) bool { action, more := InfoBindings.NextEvent(e, nil) if action != nil && !more { From 5b3737fb2abb9329fd2fa2608d566d8cd245470c Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Thu, 25 Apr 2024 00:05:27 +0200 Subject: [PATCH 3/5] infopane: HandleEvent: reset key sequence when handling y/n prompt Fix the following buggy behavior: 1. bind "" to the Paste action in the command bar 2. open a split pane, type some text and press Ctrl-q to close it 3. answer "n" to the "Save changes before closing?" prompt 4. press Ctrl-e to open the command prompt and press "a" -> result: instead of inserting the "a" letter, clipboard is pasted. --- internal/action/infopane.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/action/infopane.go b/internal/action/infopane.go index 32afb8d8..321fb45b 100644 --- a/internal/action/infopane.go +++ b/internal/action/infopane.go @@ -98,9 +98,15 @@ func (h *InfoPane) HandleEvent(event tcell.Event) { if (e.Rune() == 'y' || e.Rune() == 'Y') && hasYN { h.YNResp = true h.DonePrompt(false) + + InfoBindings.ResetEvents() + InfoBufBindings.ResetEvents() } else if (e.Rune() == 'n' || e.Rune() == 'N') && hasYN { h.YNResp = false h.DonePrompt(false) + + InfoBindings.ResetEvents() + InfoBufBindings.ResetEvents() } } if e.Key() == tcell.KeyRune && !done && !hasYN { From fade304667bbde7fae25e5d9f544bcfb2b20321e Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Thu, 25 Apr 2024 00:13:37 +0200 Subject: [PATCH 4/5] infopane: HandleEvent: refactor y/n prompt handling --- internal/action/infopane.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/action/infopane.go b/internal/action/infopane.go index 321fb45b..bfb6002b 100644 --- a/internal/action/infopane.go +++ b/internal/action/infopane.go @@ -95,14 +95,10 @@ func (h *InfoPane) HandleEvent(event tcell.Event) { done := h.DoKeyEvent(ke) hasYN := h.HasYN if e.Key() == tcell.KeyRune && hasYN { - if (e.Rune() == 'y' || e.Rune() == 'Y') && hasYN { - h.YNResp = true - h.DonePrompt(false) - - InfoBindings.ResetEvents() - InfoBufBindings.ResetEvents() - } else if (e.Rune() == 'n' || e.Rune() == 'N') && hasYN { - h.YNResp = false + y := e.Rune() == 'y' || e.Rune() == 'Y' + n := e.Rune() == 'n' || e.Rune() == 'N' + if y || n { + h.YNResp = y h.DonePrompt(false) InfoBindings.ResetEvents() From 8632b82cbee483c4d93612e69bd2733f437a8b25 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Thu, 25 Apr 2024 00:28:11 +0200 Subject: [PATCH 5/5] infopane: DoKeyEvent: it is buggy, let's add a TODO for now --- internal/action/infopane.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/action/infopane.go b/internal/action/infopane.go index bfb6002b..d3f30fd4 100644 --- a/internal/action/infopane.go +++ b/internal/action/infopane.go @@ -143,6 +143,20 @@ func (h *InfoPane) DoKeyEvent(e KeyEvent) bool { } if !more { + // If no infopane action found, try to find a bufpane action. + // + // TODO: this is buggy. For example, if the command bar has the following + // two bindings: + // + // "": "HistoryUp", + // "": "Paste", + // + // the 2nd binding (with a bufpane action) doesn't work, since + // has been already consumed by the 1st binding (with an infopane action). + // + // We should either iterate both InfoBindings and InfoBufBindings keytrees + // together, or just use the same keytree for both infopane and bufpane + // bindings. action, more = InfoBufBindings.NextEvent(e, nil) if action != nil && !more { action(h.BufPane)