Robin Jarry: 2 compose: avoid panic when deleting the last header compose: respect header ordering from text editor 2 files changed, 52 insertions(+), 28 deletions(-)
aerc/patches: SUCCESS in 5m21s [compose: avoid panic when deleting the last header][0] from [Robin Jarry][1] [0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/44852 [1]: mailto:robin@jarry.cc ✓ #1059540 SUCCESS aerc/patches/openbsd.yml https://builds.sr.ht/~rjarry/job/1059540 ✓ #1059539 SUCCESS aerc/patches/alpine-edge.yml https://builds.sr.ht/~rjarry/job/1059539
Works as advertised. Tested-By: inwit <inwit@sindominio.net>
Applied. Thanks!
inwit, Sep 19, 2023 at 16:28:
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~rjarry/aerc-devel/patches/44852/mbox | git am -3Learn more about email & git
When removing the last header, the focusable list may be empty, and the focused index becomes invalid. Avoid out of bounds access. Signed-off-by: Robin Jarry <robin@jarry.cc> --- widgets/compose.go | 73 +++++++++++++++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/widgets/compose.go b/widgets/compose.go index 861f95a7e3ac..5664f54fcefa 100644 --- a/widgets/compose.go +++ b/widgets/compose.go @@ -745,9 +745,9 @@ func (c *Composer) focusTerminalPriv() *Composer { if c.editor == nil { return c } - c.focusable[c.focused].Focus(false) - c.focused = len(c.editors) - c.focusable[c.focused].Focus(true) + c.focusActiveWidget(false) + c.focused = len(c.focusable) - 1 + c.focusActiveWidget(true) return c } @@ -805,18 +805,31 @@ func (c *Composer) Bindings() string { switch c.editor { case nil: return "compose::review" - case c.focusable[c.focused]: + case c.focusedWidget(): return "compose::editor" default: return "compose" } } +func (c *Composer) focusedWidget() ui.MouseableDrawableInteractive { + if c.focused < 0 || c.focused >= len(c.focusable) { + return nil + } + return c.focusable[c.focused] +} + +func (c *Composer) focusActiveWidget(focus bool) { + if w := c.focusedWidget(); w != nil { + w.Focus(focus) + } +} + func (c *Composer) Event(event tcell.Event) bool { c.Lock() defer c.Unlock() - if c.editor != nil { - return c.focusable[c.focused].Event(event) + if w := c.focusedWidget(); c.editor != nil && w != nil { + return w.Event(event) } return false } @@ -836,9 +849,9 @@ func (c *Composer) MouseEvent(localX int, localY int, event tcell.Event) { for i, e := range c.focusable { he, ok := e.(*headerEditor) if ok && he.focused { - c.focusable[c.focused].Focus(false) + c.focusActiveWidget(false) c.focused = i - c.focusable[c.focused].Focus(true) + c.focusActiveWidget(true) return } } @@ -846,14 +859,16 @@ func (c *Composer) MouseEvent(localX int, localY int, event tcell.Event) { func (c *Composer) Focus(focus bool) { c.Lock() - c.focusable[c.focused].Focus(focus) + c.focusActiveWidget(focus) c.Unlock() } func (c *Composer) Show(visible bool) { c.Lock() - if vis, ok := c.focusable[c.focused].(ui.Visible); ok { - vis.Show(visible) + if w := c.focusedWidget(); w != nil { + if vis, ok := w.(ui.Visible); ok { + vis.Show(visible) + } } c.Unlock() } @@ -1325,12 +1340,12 @@ func (c *Composer) PrevField() { if c.editHeaders && c.editor != nil { return } - c.focusable[c.focused].Focus(false) + c.focusActiveWidget(false) c.focused-- if c.focused == -1 { c.focused = len(c.focusable) - 1 } - c.focusable[c.focused].Focus(true) + c.focusActiveWidget(true) } func (c *Composer) NextField() { @@ -1339,9 +1354,9 @@ func (c *Composer) NextField() { if c.editHeaders && c.editor != nil { return } - c.focusable[c.focused].Focus(false) + c.focusActiveWidget(false) c.focused = (c.focused + 1) % len(c.focusable) - c.focusable[c.focused].Focus(true) + c.focusActiveWidget(true) } func (c *Composer) FocusEditor(editor string) { @@ -1355,7 +1370,7 @@ func (c *Composer) FocusEditor(editor string) { func (c *Composer) focusEditor(editor string) { editor = strings.ToLower(editor) - c.focusable[c.focused].Focus(false) + c.focusActiveWidget(false) for i, f := range c.focusable { e := f.(*headerEditor) if strings.ToLower(e.name) == editor { @@ -1363,7 +1378,7 @@ func (c *Composer) focusEditor(editor string) { break } } - c.focusable[c.focused].Focus(true) + c.focusActiveWidget(true) } // AddEditor appends a new header editor to the compose window. @@ -1400,12 +1415,22 @@ func (c *Composer) addEditor(header string, value string, appendHeader bool) str } c.editors[header] = e c.layout = append(c.layout, []string{header}) - // Insert focus of new editor before terminal editor - c.focusable = append( - c.focusable[:len(c.focusable)-1], - e, - c.focusable[len(c.focusable)-1], - ) + switch { + case len(c.focusable) == 0: + c.focusable = []ui.MouseableDrawableInteractive{e} + case c.editor != nil: + // Insert focus of new editor before terminal editor + c.focusable = append( + c.focusable[:len(c.focusable)-1], + e, + c.focusable[len(c.focusable)-1], + ) + default: + c.focusable = append( + c.focusable[:len(c.focusable)-1], + e, + ) + } editor = e } @@ -1466,8 +1491,8 @@ func (c *Composer) delEditor(header string) { focusable = append(focusable, f) } } - focusable[c.focused].Focus(true) c.focusable = focusable + c.focusActiveWidget(true) delete(c.editors, header) } -- 2.41.0
When [compose].edit-headers=true, make sure to respect the order of headers as set in the text editor. Signed-off-by: Robin Jarry <robin@jarry.cc> --- widgets/compose.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/widgets/compose.go b/widgets/compose.go index 5664f54fcefa..f9e8b7b3b6c4 100644 --- a/widgets/compose.go +++ b/widgets/compose.go @@ -1267,15 +1267,14 @@ func (c *Composer) termClosed(err error) { } return } + // delete previous headers first for _, h := range c.headerOrder() { - if embedHeader.Get(h) == "" { - // user deleted header in text editor - c.delEditor(h) - } + c.delEditor(h) } hf := embedHeader.Fields() for hf.Next() { if hf.Value() != "" { + // add new header values in order c.addEditor(hf.Key(), hf.Value(), false) } } -- 2.41.0
builds.sr.ht <builds@sr.ht>aerc/patches: SUCCESS in 5m21s [compose: avoid panic when deleting the last header][0] from [Robin Jarry][1] [0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/44852 [1]: mailto:robin@jarry.cc ✓ #1059540 SUCCESS aerc/patches/openbsd.yml https://builds.sr.ht/~rjarry/job/1059540 ✓ #1059539 SUCCESS aerc/patches/alpine-edge.yml https://builds.sr.ht/~rjarry/job/1059539
Works as advertised. Tested-By: inwit <inwit@sindominio.net>