Many panics occur from calling Draw on a nil widget, stemming from the
grid ui element. Protect the calls to Draw from within grid to prevent
this method of panic.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
The grid method Children returns the children of a grid, and is never
used. The function is reimplemented in both aerc.go and account.go, also
never called.
Remove these unused methods.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
Apply GoDoc comment policy (comments for humans should have a space
after the //; machine-readable comments shouldn't)
Use strings.ReplaceAll instead of strings.Replace when appropriate
Remove if/else chains by replacing them with switches
Use short assignment/increment notation
Replace single case switches with if statements
Combine else and if when appropriate
Signed-off-by: Moritz Poldrack <moritz@poldrack.dev>
Acked-by: Robin Jarry <robin@jarry.cc>
aerc panics when using macOS's default terminal emulator, Terminal.app,
when closing all but aerc's tab:
This error was also written to: /tmp/aerc-crash-20220427-194134.log
panic: Attempted to create context with negative offset [recovered]
panic: Attempted to create context with negative offset
goroutine 1 [running]:
git.sr.ht/~rjarry/aerc/logging.PanicHandler()
/Users/ckuehl/src/aerc/logging/panic-logger.go:47 +0x58c
panic({0x100d077a0, 0x14000032700})
/opt/homebrew/Cellar/go/1.18.1/libexec/src/runtime/panic.go:844 +0x258
git.sr.ht/~rjarry/aerc/lib/ui.(*Context).Subcontext(0x1400013e420?, 0x14000202360?, 0x140000ffc48?, 0x1009a10e4?, 0x100da9440?)
/Users/ckuehl/src/aerc/lib/ui/context.go:47 +0x160
git.sr.ht/~rjarry/aerc/lib/ui.(*Grid).Draw(0x1400013e420, 0x14000202360)
/Users/ckuehl/src/aerc/lib/ui/grid.go:143 +0x2bc
git.sr.ht/~rjarry/aerc/widgets.(*Aerc).Draw(0x1400013e4d0, 0x14000202360)
/Users/ckuehl/src/aerc/widgets/aerc.go:178 +0x30
git.sr.ht/~rjarry/aerc/lib/ui.(*UI).Tick(0x1400022bcc0)
/Users/ckuehl/src/aerc/lib/ui/ui.go:116 +0x248
main.main()
/Users/ckuehl/src/aerc/aerc.go:226 +0x9e8
I'm not entirely sure what the interactions are between the terminal
emulator, aerc's grid, and the space that moves around when the tab bar
disappears because there are no more tabs, but this fixes the issue 100%
of the time and I haven't noticed any issues.
Signed-off-by: Connor Kuehl <cipkuehl@gmail.com>
Acked-by: Robin Jarry <robin@jarry.cc>
Also update to the tcell v2 PaletteColor api, which should keep the chosen
theme of the user intact.
Note, that if $TRUECOLOR is defined and a truecolor given, aerc will now stop
clipping the value to one of the theme colors.
Generally this is desired behaviour though.
The grid used static sizes which meant that changing settings didn't
have an effect on elements of the ui, notably the sidebar width. This
patch makes the `Size` parameter of a cell a function which returns the
`int`, allowing for dynamic sizes.
A `Const` function is also included for ease of use for static sizes.
This adds the Mouseable interface. When this is implemented for a
component that item can accept and process mouseevents.
At the top level when a mouse event is received it is passed to the
grid's handler and then it trickles down until it reaches a component
that can actually handle it, such as the tablist, dirlist or msglist.
A mouse event is passed so that components can handle other things such
as scrolling with the mousewheel. The components themselves then perform
the necessary actions.
Clicking emails in the messagelist opens them in a new tab.
Textinputs can be clicked to position the cursor inside them.
Mouseevents are not forwarded to the terminal at the moment.
Elements which do not handle mouse events are not required to implement
the Mouseable interface.
The grid was not checking there was enough space for the cells so would
just attempt to create subcontexts that don't actually fit.
This attempts to use the remaining space and then if there is no space
then it just skips drawing this cell.
If the column weights do not collectively divide the extent of the grid
layout then some width was not used and so would not be redrawn,
resulting in previous drawings showing through.
This fixes this by checking if there is any remainingExact width and if
there is it is assigned to the weighted columns by their proportion from
left to right.
This was is more complicated than others. The cells list is accessed by
multiple goroutines:
- Some change the Grid's contents via AddChild/RemoveChild
- Some call Draw
- Some invalidate the grid via Invalidate
Invalidate calls are tricky to handle because they will also invalidate all
child cells. This will inturn trigger the cellInvalidated callback, which needs
to read the list of cells.
For this reason, we use a sync.RWLock which allows multiple concurrent reads.
Below is the race fixed by this commit.
Read at 0x00c0000bc3d0 by goroutine 7:
git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Grid).cellInvalidated()
/home/simon/src/aerc2/lib/ui/grid.go:181 +0x45
git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Grid).cellInvalidated-fm()
/home/simon/src/aerc2/lib/ui/grid.go:179 +0x55
git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Invalidatable).DoInvalidate()
/home/simon/src/aerc2/lib/ui/invalidatable.go:22 +0x85
git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Bordered).contentInvalidated-fm()
/home/simon/src/aerc2/lib/ui/borders.go:39 +0x56
git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Invalidatable).DoInvalidate()
/home/simon/src/aerc2/lib/ui/invalidatable.go:22 +0x85
git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList.func1()
/home/simon/src/aerc2/widgets/dirlist.go:81 +0x55
git.sr.ht/~sircmpwn/aerc2/lib/ui.(*Invalidatable).DoInvalidate()
/home/simon/src/aerc2/lib/ui/invalidatable.go:22 +0x85
git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start.func1()
/home/simon/src/aerc2/widgets/spinner.go:88 +0x82
Previous write at 0x00c0000bc3d0 by main goroutine:
[failed to restore the stack]
Goroutine 7 (running) created at:
git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start()
/home/simon/src/aerc2/widgets/spinner.go:46 +0x98
git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList()
/home/simon/src/aerc2/widgets/dirlist.go:37 +0x28b
git.sr.ht/~sircmpwn/aerc2/widgets.NewAccountView()
/home/simon/src/aerc2/widgets/account.go:49 +0x5ca
git.sr.ht/~sircmpwn/aerc2/widgets.NewAerc()
/home/simon/src/aerc2/widgets/aerc.go:60 +0x807
main.main()
/home/simon/src/aerc2/aerc.go:65 +0x33e
Many Drawable implementations have their own Invalidate and OnInvalidate
functions, with an unexported onInvalidate field. However OnInvalidate and
Invalidate are usually not called in the same goroutine. This results in a race
on this field, e.g.:
Read at 0x00c000094748 by goroutine 7:
git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList.func1()
/home/simon/src/aerc2/widgets/dirlist.go:85 +0x56
git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start.func1()
/home/simon/src/aerc2/widgets/spinner.go:93 +0x1bb
Previous write at 0x00c000094748 by main goroutine:
[failed to restore the stack]
Goroutine 7 (running) created at:
git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start()
/home/simon/src/aerc2/widgets/spinner.go:46 +0x8f
git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList()
/home/simon/src/aerc2/widgets/dirlist.go:37 +0x286
git.sr.ht/~sircmpwn/aerc2/widgets.NewAccountView()
/home/simon/src/aerc2/widgets/account.go:50 +0x5ca
git.sr.ht/~sircmpwn/aerc2/widgets.NewAerc()
/home/simon/src/aerc2/widgets/aerc.go:60 +0x800
main.main()
/home/simon/src/aerc2/aerc.go:65 +0x33e
To fix this, introduce a new type, Invalidatable, which protects the field.
Unfortunately the Drawable must be passed to the callback function in
Invalidate, so we still need to re-implement this in each Invalidatable user.
