The terminal widget uses it's own redraw logic to improve performance.
With the addition of a main event loop, the redraw logic can happen in
the main loop via the standard Invalidate logic.
Use the Invalidate method to mark aerc invalid, and immediately trigger
a redraw with ui.QueueRedraw. The follow up call to QueueRedraw is
needed because the terminal update happens in a separate goroutine. This
can result in the main event loop finishing it's process of the current
event, redrawing the screen, and the terminal having additional updates
to be drawn.
This fixes race conditions by drawing and calling screen.Show in a
separate goroutine.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
Combine tcell events with WorkerMessages to better synchronize state
with IO and UI. Remove Tick loop for rendering. Use events to trigger
renders.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
Subsitute the format specifier %w for %v in the logging facility. The
logging functions use a fmt.Sprintf call behind the scene which does not
recognize %w. %w should be used in fmt.Errorf when you want to wrap
errors. Hence, the log entries that use %w are improperly formatted like
this:
ERROR 2022/10/02 09:13:57.724529 worker.go:439: could not get message
info %!w(*fmt.wrapError=&{could not get structure: [snip] })
^
Links: https://go.dev/blog/go1.13-errors
Signed-off-by: Koni Marti <koni.marti@gmail.com>
Acked-by: Moritz Poldrack <moritz@poldrack.dev>
Upgrade tcell-term to v0.2.0
Use Start method from tcell-term. This prevents aerc from needing to
wait until the command has started to continue. The tcell-term start
method blocks until the command is started, similar to cmd.Start. By
doing so, we prevent a race condition between aerc and tcell-term on
access to cmd.Process.
Remove cleanup of cmd, this is all already handled by tcell-term when
Close is called.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
The terminal widget has two invalidation methods, one exported and one
private. The private one does nothing special.
Remove the private method and only use the exported method.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
If a :term is open and aerc is focused on another tab, it is possible
for the :term to redraw itself to the screen.
Only allow terminal to redraw itself when it is focused.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
The terminal widget uses it's own internal event handler to redraw
itself for improved performance. The event handler draws, updates the
screen, and invalidates. The last invalidate is redundant: Invalidating
has the result of telling aerc to redraw the screen. A race condition
can occur where an event is emitted from the terminal and the terminal
is closed before the event is handled. This results in handling the
event after the terminal is closed, and a panic:
panic: Attempted to invalidate unknown cell
goroutine 54685 [running]:
git.sr.ht/~rjarry/aerc/lib/ui.(*Grid).cellInvalidated(0xc00b0a34a0, {0xbb8f10?, 0xc00360cd80})
git.sr.ht/~rjarry/aerc/lib/ui/grid.go:287 +0x175
git.sr.ht/~rjarry/aerc/lib/ui.(*Invalidatable).DoInvalidate(0xc0002e7900?, {0xbb8f10?, 0xc00360cd80?})
git.sr.ht/~rjarry/aerc/lib/ui/invalidatable.go:22 +0x82
git.sr.ht/~rjarry/aerc/widgets.(*Terminal).invalidate(...)
git.sr.ht/~rjarry/aerc/widgets/terminal.go:93
git.sr.ht/~rjarry/aerc/widgets.(*Terminal).HandleEvent(0xc00360cd80, {0xbb4ba0?, 0xc006022690?})
git.sr.ht/~rjarry/aerc/widgets/terminal.go:192 +0xd2
github.com/gdamore/tcell/v2/views.(*WidgetWatchers).PostEvent(...)
github.com/gdamore/tcell/v2@v2.5.3/views/widget.go:113
github.com/gdamore/tcell/v2/views.(*WidgetWatchers).PostEventWidgetContent(0xc0001b9360, {0xbbc950?, 0xc0001b9320})
github.com/gdamore/tcell/v2@v2.5.3/views/widget.go:123 +0x12b
git.sr.ht/~rockorager/tcell-term.(*Terminal).run.func1()
git.sr.ht/~rockorager/tcell-term@v0.1.0/terminal.go:117 +0x9d
created by git.sr.ht/~rockorager/tcell-term.(*Terminal).run
git.sr.ht/~rockorager/tcell-term@v0.1.0/terminal.go:104 +0x110
Don't invalidate on EventWidgetContent. The terminal already handles
drawing and updating the tcell Screen internally.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
Improve terminal mouse support by forwarding mouse events to the
terminal widget. Clicking and dragging are supported.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
The terminal widget internally uses several context methods. Check that
context is not nil before calling any method to prevent panics.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
Replace go-libvterm package with tcell-term. go-libvterm provides the
embedded terminal for aerc. It uses a statically linked C library,
requiring CGO.
tcell-term is written in pure go and is written to be portable with
tcell applications by implementing the tcell Widget interface. This
allows the terminal to take a view (which aerc already supplies) and
draw directly to it, as well as issue tcell Events to a Watcher.
Enable setting cursor shapes in embedded terminals.
Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Signed-off-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>
Commit 1bac87e804 ("terminal: fix race when closing a terminal") fixed
a race in Terminal.Draw by using a mutex. The current locking of the
entire Draw function could create a deadlock, however, since this
function itself might call Terminal.Close which is protected by the same
mutex. A finer-grained locking solves both the race and deadlock
problem.
Signed-off-by: Koni Marti <koni.marti@gmail.com>
Acked-by: Robin Jarry <robin@jarry.cc>
Since panics still regularly "destroy" the terminal, it is hard to get a
stack trace for panics you do not anticipate. This commit adds a panic
handler that automatically creates a logfile inside the current working
directory.
It has to be added to every goroutine that is started and will repair
the terminal on a panic.
Signed-off-by: Moritz Poldrack <git@moritz.sh>
Acked-by: Robin Jarry <robin@jarry.cc>
pty.Getsize() is used in the Draw function of the terminal widget and wraps the
pty.GetsizeFull() function. However, pty.Getsize does not check the returned
error from pty.GetsizeFull before dereferencing the winsize struct. In case of
an error, this will cause a nil pointer deference and panic.
This has been reported in the upstream package, but in the meantime, we can
directly use pty.GetsizeFull.
References: https://todo.sr.ht/~rjarry/aerc/11
Signed-off-by: Koni Marti <koni.marti@gmail.com>
fixes the segmentation fault when copy-pasting a large text into the
composer editor. The problem is a concurrent read of the vterm field in the
Terminal widget in its flushTerminal() method which can be avoided with a mutex.
Fixes: https://todo.sr.ht/~rjarry/aerc/12
Signed-off-by: Koni Marti <koni.marti@gmail.com>
This fixes a substantial performance issue when scrolling emails with
long/complicated contents, where scrolling down a single line can take
something like hundreds of ms before the screen is updated to reflect
the scroll. It's really bad if the email has lots of columns, e.g. like
if it's an html email that was passed through a filter (w3m, etc) to
render it.
Using pprof, I found that the multiple calls to vterm.ScreenCell.Attrs()
in styleFromCell were really really expensive. This patch replaces them
with a single call.
Here's a before and after with a simple, but very manual test of opening
a large email with contents that went through a w3m filter and
continuously scrolling up and down over and over for ~30 seconds:
*** Before:
----------------------------------------------------------+-------------
flat flat% sum% cum cum% calls calls% + context
----------------------------------------------------------+-------------
28.25s 100% | git.x2esr.x2eht..z2f..z7esircmpwn..z2faerc..z2fwidgets.PartSwitcher.Draw
0 0% 99.94% 28.25s 82.31% | git.x2esr.x2eht..z2f..z7esircmpwn..z2faerc..z2fwidgets.PartViewer.Draw
28.25s 100% | git.x2esr.x2eht..z2f..z7esircmpwn..z2faerc..z2fwidgets.Terminal.Draw
----------------------------------------------------------+-------------
28.25s 100% | git.x2esr.x2eht..z2f..z7esircmpwn..z2faerc..z2fwidgets.PartViewer.Draw
0 0% 99.94% 28.25s 82.31% | git.x2esr.x2eht..z2f..z7esircmpwn..z2faerc..z2fwidgets.Terminal.Draw
19.23s 68.07% | git.x2esr.x2eht..z2f..z7esircmpwn..z2faerc..z2fwidgets.Terminal.styleFromCell
6.04s 21.38% | github.x2ecom..z2fddevault..z2fgo..z2dlibvterm.Screen.GetCellAt
1.38s 4.88% | git.x2esr.x2eht..z2f..z7esircmpwn..z2faerc..z2flib..z2fui.Context.Printf
0.62s 2.19% | runtime.mapassign
0.43s 1.52% | runtime.mapaccess2
0.20s 0.71% | runtime.newobject
0.19s 0.67% | runtime.callers (inline)
0.07s 0.25% | runtime.makeslice
0.07s 0.25% | runtime.mallocgc
----------------------------------------------------------+-------------
19.23s 100% | git.x2esr.x2eht..z2f..z7esircmpwn..z2faerc..z2fwidgets.Terminal.Draw
0 0% 99.94% 19.23s 56.03% | git.x2esr.x2eht..z2f..z7esircmpwn..z2faerc..z2fwidgets.Terminal.styleFromCell
19.21s 99.90% | github.x2ecom..z2fddevault..z2fgo..z2dlibvterm.ScreenCell.Attrs
*** After:
----------------------------------------------------------+-------------
flat flat% sum% cum cum% calls calls% + context
----------------------------------------------------------+-------------
0.31s 100% | git.x2esr.x2eht/~sircmpwn/aerc/widgets.Terminal.Draw
0 0% 99.87% 0.31s 1.33% | github.x2ecom/ddevault/go-libvterm.NewPos
0.25s 80.65% | runtime.callers (inline)
0.04s 12.90% | runtime.gomcache (inline)
----------------------------------------------------------+-------------
8.40s 100% | github.x2ecom/ddevault/go-libvterm.Screen.GetCellAt
0 0% 99.87% 8.40s 36.11% | github.x2ecom/ddevault/go-libvterm.Screen.GetCell
7.14s 85.00% | github.x2ecom/ddevault/go-libvterm._cgoCheckPointer
0.54s 6.43% | runtime.callers (inline)
0.35s 4.17% | runtime.exitsyscall
0.11s 1.31% | runtime.deferprocStack
0.07s 0.83% | doentersyscall
0.07s 0.83% | runtime.getg
0.05s 0.6% | runtime.casgstatus
0.03s 0.36% | _init
0.03s 0.36% | runtime.gomcache (inline)
----------------------------------------------------------+-------------
8.46s 100% | git.x2esr.x2eht/~sircmpwn/aerc/widgets.Terminal.Draw
0 0% 99.87% 8.46s 36.37% | github.x2ecom/ddevault/go-libvterm.Screen.GetCellAt
8.40s 99.29% | github.x2ecom/ddevault/go-libvterm.Screen.GetCell
0.06s 0.71% | runtime.callers (inline)
----------------------------------------------------------+-------------
0.31s 100% | git.x2esr.x2eht/~sircmpwn/aerc/widgets.Terminal.styleFromCell
0 0% 99.87% 0.31s 1.33% | github.x2ecom/ddevault/go-libvterm.ScreenCell.Attrs
)
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.
Soves an issue with go1.15 not letting ctty be a parent. See
https://github.com/creack/pty/pull/97 for more details.
Signed-off-by: Guillaume J. Charmes <git+guillaume@charmes.net>
The editor and pager were not properly being reaped, causing resource
leakage whenever a user replies to a message.
Signed-off-by: Kevin Kuehler <keur@xcf.berkeley.edu>
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.
vterm.Write and vterm.SetSize race when the window resizes, which
causing the underlying library to segfault.
Signed-off-by: Kevin Kuehler <keur@ocf.berkeley.edu>
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.
