~sircmpwn/aerc

Don't propagate negative offsets when drawing v1 PROPOSED

Connor Kuehl
Connor Kuehl: 1
 Don't propagate negative offsets when drawing

 1 files changed, 4 insertions(+), 0 deletions(-)
Perhaps we shouldn't be panicking here at all? If we returned an error
here and forced the caller to handle that then we could probably avoid a
lot of these hard to reproduce panics. Although context.Subcontext does
get called in a lot of locations so perhaps the churn wouldn't be worth
it.
> Perhaps we shouldn't be panicking here at all? If we returned an error
> here and forced the caller to handle that 
The best way to handle errors, is to handle them gracefully IMO. Since
the size is outside of the scope of aerc, so aerc should do the next
reasonable thing: nothing.

We could try to show an error, but what would this mean to the user? (If
they can at all read it) Close to nothing, if the user doesn't ignore
the error outright.
> then we could probably avoid a lot of these hard to reproduce panics.
Panics are there for a reason, so getting rid of panic doesn't really
help here if we don't adress the underlying reason.

I personally like the patch so far as it does what I'd consider
"gracefuly handling" of the error: ignore the faulty state and try again
later with different values.
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~sircmpwn/aerc/patches/24864/mbox | git am -3
Learn more about email & git
View this thread in the archives

[PATCH] Don't propagate negative offsets when drawing Export this patch

Connor Kuehl
I've been observing a 100% reproducible panic when running aerc in a tab
in my terminal emulator (Terminal.app on macOS).

To reproduce:
1. Run aerc in a tab
2. Open another tab in the terminal emulator session so aerc does not
   have the entire window to itself.
3. Close all other tabs in the terminal emulator session so aerc's tab
   is the last one.
4. Observe the panic
goroutine 1 [running]:
runtime/debug.Stack()
    /opt/homebrew/Cellar/go/1.17/libexec/src/runtime/debug/stack.go:24 +0x88
runtime/debug.PrintStack()
    /opt/homebrew/Cellar/go/1.17/libexec/src/runtime/debug/stack.go:16 +0x20
main.PanicTermFix(0x0)
    /Users/connor/git/aerc/aerc.go:212 +0x48
panic({0x10341cf40, 0x1400013c360})
    /opt/homebrew/Cellar/go/1.17/libexec/src/runtime/panic.go:1052 +0x2ac
git.sr.ht/~sircmpwn/aerc/lib/ui.(*Context).Subcontext(0x14000420240, 0x0, 0xffffffffffffffff, 0x82, 0x1)
    /Users/connor/git/aerc/lib/ui/context.go:47 +0x1b0
git.sr.ht/~sircmpwn/aerc/lib/ui.(*Grid).Draw(0x140000d84d0, 0x14000420240)
    /Users/connor/git/aerc/lib/ui/grid.go:143 +0x2a4
git.sr.ht/~sircmpwn/aerc/widgets.(*Aerc).Draw(0x140000d8580, 0x14000420240)
    /Users/connor/git/aerc/widgets/aerc.go:177 +0x34
git.sr.ht/~sircmpwn/aerc/lib/ui.(*UI).Tick(0x140003aa000)
    /Users/connor/git/aerc/lib/ui/ui.go:113 +0x2ac
main.main()
    /Users/connor/git/aerc/aerc.go:197 +0x9f8
aerc crashed: Attempted to create context with negative offset

In my terminal emulator, when the other tabs are closed, the window size
changes to remove the space that the tabs took up. This is when aerc
will panic.

To avoid panicking, just skip over the cells with negative offsets since
they are "off screen" anyway.

Signed-off-by: Connor Kuehl <cipkuehl@gmail.com>
---
  Frankly, I am not 100% certain this patch is the best way to resolve
  this. I am sending it anyway to hopefully cultivate some discussion or
  guidance on the best way forward, since this doesn't strike me as
  resolving the root cause that is resulting in negative offsets in the
  first place. I am not (yet) an expert on aerc internals :-)

 lib/ui/grid.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/ui/grid.go b/lib/ui/grid.go
index f505ce0..2d19571 100644
--- a/lib/ui/grid.go
+++ b/lib/ui/grid.go
@@ -123,6 +123,10 @@ func (grid *Grid) Draw(ctx *Context) {
		cols := grid.columnLayout[cell.Column : cell.Column+cell.ColSpan]
		x := cols[0].Offset
		y := rows[0].Offset
		if x < 0 || y < 0 {
			continue
		}

		width := 0
		height := 0
		for _, col := range cols {
-- 
2.30.1 (Apple Git-130)