From 781f057e6f6de7f715801869b49af399b4a3e1d2 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sat, 15 Jun 2024 01:00:36 +0200 Subject: [PATCH 1/9] Improve Undo & Redo actions return values Return false if there is nothing to undo/redo. This also fixes false "Undid action" and "Redid actions" infobar messages in the case when no action was actually undone or redone. --- internal/action/actions.go | 8 ++++++-- internal/buffer/eventhandler.go | 22 ++++++++++++---------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/internal/action/actions.go b/internal/action/actions.go index 28d0e53d..3cfb96f9 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -1163,7 +1163,9 @@ func (h *BufPane) DiffPrevious() bool { // Undo undoes the last action func (h *BufPane) Undo() bool { - h.Buf.Undo() + if !h.Buf.Undo() { + return false + } InfoBar.Message("Undid action") h.Relocate() return true @@ -1171,7 +1173,9 @@ func (h *BufPane) Undo() bool { // Redo redoes the last action func (h *BufPane) Redo() bool { - h.Buf.Redo() + if !h.Buf.Redo() { + return false + } InfoBar.Message("Redid action") h.Relocate() return true diff --git a/internal/buffer/eventhandler.go b/internal/buffer/eventhandler.go index 53f64025..f1fe2a07 100644 --- a/internal/buffer/eventhandler.go +++ b/internal/buffer/eventhandler.go @@ -253,11 +253,11 @@ func (eh *EventHandler) Execute(t *TextEvent) { ExecuteTextEvent(t, eh.buf) } -// Undo the first event in the undo stack -func (eh *EventHandler) Undo() { +// Undo the first event in the undo stack. Returns false if the stack is empty. +func (eh *EventHandler) Undo() bool { t := eh.UndoStack.Peek() if t == nil { - return + return false } startTime := t.Time.UnixNano() / int64(time.Millisecond) @@ -266,15 +266,16 @@ func (eh *EventHandler) Undo() { for { t = eh.UndoStack.Peek() if t == nil { - return + break } if t.Time.UnixNano()/int64(time.Millisecond) < endTime { - return + break } eh.UndoOneEvent() } + return true } // UndoOneEvent undoes one event @@ -303,11 +304,11 @@ func (eh *EventHandler) UndoOneEvent() { eh.RedoStack.Push(t) } -// Redo the first event in the redo stack -func (eh *EventHandler) Redo() { +// Redo the first event in the redo stack. Returns false if the stack is empty. +func (eh *EventHandler) Redo() bool { t := eh.RedoStack.Peek() if t == nil { - return + return false } startTime := t.Time.UnixNano() / int64(time.Millisecond) @@ -316,15 +317,16 @@ func (eh *EventHandler) Redo() { for { t = eh.RedoStack.Peek() if t == nil { - return + break } if t.Time.UnixNano()/int64(time.Millisecond) > endTime { - return + break } eh.RedoOneEvent() } + return true } // RedoOneEvent redoes one event From fc5d83f6c6dd9ef2c31f59015d6c7577cdb2194f Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sat, 15 Jun 2024 17:28:29 +0200 Subject: [PATCH 2/9] Improve RemoveMultiCursor & RemoveAllMultiCursors actions return values --- internal/action/actions.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/action/actions.go b/internal/action/actions.go index 3cfb96f9..0c0babd4 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -2023,8 +2023,10 @@ func (h *BufPane) RemoveMultiCursor() bool { h.Buf.RemoveCursor(h.Buf.NumCursors() - 1) h.Buf.SetCurCursor(h.Buf.NumCursors() - 1) h.Buf.UpdateCursors() - } else { + } else if h.multiWord { h.multiWord = false + } else { + return false } h.Relocate() return true @@ -2032,8 +2034,12 @@ func (h *BufPane) RemoveMultiCursor() bool { // RemoveAllMultiCursors removes all cursors except the base cursor func (h *BufPane) RemoveAllMultiCursors() bool { - h.Buf.ClearCursors() - h.multiWord = false + if h.Buf.NumCursors() > 1 || h.multiWord { + h.Buf.ClearCursors() + h.multiWord = false + } else { + return false + } h.Relocate() return true } From e2e8baf4f30ce319906f13277c45e4c2a72ce5c3 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sat, 15 Jun 2024 20:30:05 +0200 Subject: [PATCH 3/9] Improve misc actions return values --- internal/action/actions.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/internal/action/actions.go b/internal/action/actions.go index 0c0babd4..5cbe008b 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -1068,6 +1068,9 @@ func (h *BufPane) ToggleHighlightSearch() bool { // UnhighlightSearch unhighlights all instances of the last used search term func (h *BufPane) UnhighlightSearch() bool { + if !h.Buf.HighlightSearch { + return false + } h.Buf.HighlightSearch = false return true } @@ -1632,12 +1635,18 @@ func (h *BufPane) Escape() bool { // Deselect deselects on the current cursor func (h *BufPane) Deselect() bool { + if !h.Cursor.HasSelection() { + return false + } h.Cursor.Deselect(true) return true } // ClearInfo clears the infobar func (h *BufPane) ClearInfo() bool { + if InfoBar.Msg == "" { + return false + } InfoBar.Message("") return true } @@ -1728,6 +1737,10 @@ func (h *BufPane) AddTab() bool { // PreviousTab switches to the previous tab in the tab list func (h *BufPane) PreviousTab() bool { tabsLen := len(Tabs.List) + if tabsLen == 1 { + return false + } + a := Tabs.Active() + tabsLen Tabs.SetActive((a - 1) % tabsLen) @@ -1736,8 +1749,13 @@ func (h *BufPane) PreviousTab() bool { // NextTab switches to the next tab in the tab list func (h *BufPane) NextTab() bool { + tabsLen := len(Tabs.List) + if tabsLen == 1 { + return false + } + a := Tabs.Active() - Tabs.SetActive((a + 1) % len(Tabs.List)) + Tabs.SetActive((a + 1) % tabsLen) return true } @@ -1773,6 +1791,10 @@ func (h *BufPane) Unsplit() bool { // NextSplit changes the view to the next split func (h *BufPane) NextSplit() bool { + if len(h.tab.Panes) == 1 { + return false + } + a := h.tab.active if a < len(h.tab.Panes)-1 { a++ @@ -1787,6 +1809,10 @@ func (h *BufPane) NextSplit() bool { // PreviousSplit changes the view to the previous split func (h *BufPane) PreviousSplit() bool { + if len(h.tab.Panes) == 1 { + return false + } + a := h.tab.active if a > 0 { a-- From 7e09a921e4c4244a4ef9956418a851efdba8fc72 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sun, 16 Jun 2024 17:53:08 +0200 Subject: [PATCH 4/9] Make it clear that ClearStatus is the same as ClearInfo ClearInfo and ClearStatus actions do exactly the same thing. Let's keep them both, for compatibility reasons (who knows how many users are using either of the two), but at least document that there is no difference between the two. --- internal/action/actions.go | 5 ++--- runtime/help/keybindings.md | 1 + 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/action/actions.go b/internal/action/actions.go index 5cbe008b..3806b348 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -1577,10 +1577,9 @@ func (h *BufPane) ToggleRuler() bool { return true } -// ClearStatus clears the messenger bar +// ClearStatus clears the infobar. It is an alias for ClearInfo. func (h *BufPane) ClearStatus() bool { - InfoBar.Message("") - return true + return h.ClearInfo() } // ToggleHelp toggles the help screen diff --git a/runtime/help/keybindings.md b/runtime/help/keybindings.md index 3198c8fb..869af62d 100644 --- a/runtime/help/keybindings.md +++ b/runtime/help/keybindings.md @@ -242,6 +242,7 @@ ToggleDiffGutter ToggleRuler JumpLine ResetSearch +ClearInfo ClearStatus ShellMode CommandMode From 2793c37a94e390f7972ffed322aad38a7ccf94c7 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sat, 15 Jun 2024 20:32:16 +0200 Subject: [PATCH 5/9] Fix SkipMultiCursor behavior when there is no selection When there is no selection (i.e. selection is empty), SkipMultiCursor searches for the empty text, "finds" it as the beginning of the buffer, and as a result, jumps to the beginning of the buffer, which confuses the user. Fix it. --- internal/action/actions.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/action/actions.go b/internal/action/actions.go index 3806b348..a4d6fdea 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -2013,6 +2013,9 @@ func (h *BufPane) MouseMultiCursor(e *tcell.EventMouse) bool { // SkipMultiCursor moves the current multiple cursor to the next available position func (h *BufPane) SkipMultiCursor() bool { lastC := h.Buf.GetCursor(h.Buf.NumCursors() - 1) + if !lastC.HasSelection() { + return false + } sel := lastC.GetSelection() searchStart := lastC.CurSelection[1] From aa9c476b1efe1d7da3bc6eb97b53c6662a6c945e Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sat, 15 Jun 2024 20:35:55 +0200 Subject: [PATCH 6/9] Improve RemoveMultiCursor behavior If the original selection was not done by the user manually but as a result of the initial SpawnMultiCursor, deselect this original selection if we execute RemoveMultiCursor and there is no multicursor to remove (i.e. the original spawned cursor is the only one). This improves user experience by making RemoveMultiCursor behavior nicely symmetrical to SpawnMultiCursor. --- internal/action/actions.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/action/actions.go b/internal/action/actions.go index a4d6fdea..d69b95e8 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -2053,6 +2053,7 @@ func (h *BufPane) RemoveMultiCursor() bool { h.Buf.UpdateCursors() } else if h.multiWord { h.multiWord = false + h.Cursor.Deselect(true) } else { return false } From 5c8bf6b3a6cee37c26806fc7b5c0586f82b7fb6a Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sat, 15 Jun 2024 20:42:50 +0200 Subject: [PATCH 7/9] Improve RemoveAllMultiCursors behavior Use Deselect() in order to place the cursor at the beginning of the selection, not at the end of it, and to refresh its LastVisualX. --- internal/buffer/buffer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/buffer/buffer.go b/internal/buffer/buffer.go index 689d524d..661ea8b5 100644 --- a/internal/buffer/buffer.go +++ b/internal/buffer/buffer.go @@ -1089,7 +1089,7 @@ func (b *Buffer) ClearCursors() { b.cursors = b.cursors[:1] b.UpdateCursors() b.curCursor = 0 - b.GetActiveCursor().ResetSelection() + b.GetActiveCursor().Deselect(true) } // MoveLinesUp moves the range of lines up one row From 765889f610150ed27812ed44d9058435258d9df4 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sat, 15 Jun 2024 20:47:59 +0200 Subject: [PATCH 8/9] Fix execution of {Spawn,Remove}MultiCursor in chained actions - SpawnMultiCursor and RemoveMultiCursor actions change the set of cursors, so we cannot assume that it stays the same. So refresh the `cursors` list after executing every action in the chain. - If execAction() did not execute an action since it is not a multicursor, it should return true, not false, to not prevent executing next actions in the chain. --- internal/action/bufpane.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/action/bufpane.go b/internal/action/bufpane.go index 14e04089..40f4379c 100644 --- a/internal/action/bufpane.go +++ b/internal/action/bufpane.go @@ -150,10 +150,10 @@ func BufMapEvent(k Event, action string) { actionfns = append(actionfns, afn) } bufAction := func(h *BufPane, te *tcell.EventMouse) bool { - cursors := h.Buf.GetCursors() success := true for i, a := range actionfns { innerSuccess := true + cursors := h.Buf.GetCursors() for j, c := range cursors { if c == nil { continue @@ -589,6 +589,9 @@ func (h *BufPane) execAction(action BufAction, name string, cursor int, te *tcel return success } + } else { + // do nothing but return true, to not break the chain + return true } return false From 0373a63b31e666773b7fb2555dfe85d833597ac9 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sun, 16 Jun 2024 00:05:12 +0200 Subject: [PATCH 9/9] Refactor action execution code Instead of calling execAction() and then letting it check whether it should actually execute this action, do this check before calling execAction(), to make the code clear and straightforward. Precisely: for multicursor actions, call execAction() in a loop for every cursor, but for non-multicursor actions, call execAction() just once, without a loop. This, in particular, allows to get rid of the hacky "c == nil" check, since we no longer iterate a slice that may change in the meantime (since SpawnMultiCursor and RemoveMultiCursor are non-multicursor actions and thus are no longer executed while iterating the slice). --- internal/action/bufpane.go | 78 ++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/internal/action/bufpane.go b/internal/action/bufpane.go index 40f4379c..4c5b8858 100644 --- a/internal/action/bufpane.go +++ b/internal/action/bufpane.go @@ -150,29 +150,31 @@ func BufMapEvent(k Event, action string) { actionfns = append(actionfns, afn) } bufAction := func(h *BufPane, te *tcell.EventMouse) bool { - success := true for i, a := range actionfns { - innerSuccess := true - cursors := h.Buf.GetCursors() - for j, c := range cursors { - if c == nil { - continue - } - h.Buf.SetCurCursor(c.Num) - h.Cursor = c - if i == 0 || (success && types[i-1] == '&') || (!success && types[i-1] == '|') || (types[i-1] == ',') { - innerSuccess = innerSuccess && h.execAction(a, names[i], j, te) - } else { - break + var success bool + if _, ok := MultiActions[names[i]]; ok { + success = true + for _, c := range h.Buf.GetCursors() { + h.Buf.SetCurCursor(c.Num) + h.Cursor = c + success = success && h.execAction(a, names[i], te) } + } else { + h.Buf.SetCurCursor(0) + h.Cursor = h.Buf.GetActiveCursor() + success = h.execAction(a, names[i], te) } + // if the action changed the current pane, update the reference h = MainTab().CurPane() - success = innerSuccess if h == nil { // stop, in case the current pane is not a BufPane break } + + if (!success && types[i] == '&') || (success && types[i] == '|') { + break + } } return true } @@ -562,39 +564,33 @@ func (h *BufPane) DoKeyEvent(e Event) bool { return more } -func (h *BufPane) execAction(action BufAction, name string, cursor int, te *tcell.EventMouse) bool { +func (h *BufPane) execAction(action BufAction, name string, te *tcell.EventMouse) bool { if name != "Autocomplete" && name != "CycleAutocompleteBack" { h.Buf.HasSuggestions = false } - _, isMulti := MultiActions[name] - if (!isMulti && cursor == 0) || isMulti { - if h.PluginCB("pre" + name) { - var success bool - switch a := action.(type) { - case BufKeyAction: - success = a(h) - case BufMouseAction: - success = a(h, te) - } - success = success && h.PluginCB("on"+name) - - if isMulti { - if recordingMacro { - if name != "ToggleMacro" && name != "PlayMacro" { - curmacro = append(curmacro, action) - } - } - } - - return success - } - } else { - // do nothing but return true, to not break the chain - return true + if !h.PluginCB("pre" + name) { + return false } - return false + var success bool + switch a := action.(type) { + case BufKeyAction: + success = a(h) + case BufMouseAction: + success = a(h, te) + } + success = success && h.PluginCB("on"+name) + + if _, ok := MultiActions[name]; ok { + if recordingMacro { + if name != "ToggleMacro" && name != "PlayMacro" { + curmacro = append(curmacro, action) + } + } + } + + return success } func (h *BufPane) completeAction(action string) {