purebred-mua / purebred

A terminal based mail user agent based on notmuch
GNU Affero General Public License v3.0
139 stars 19 forks source link

Update to new brick version and re-use markup #461

Closed romanofski closed 2 years ago

romanofski commented 2 years ago

We used the markup module to render the mail body and highlight search results. Brick removed the Markup module in version 0.68 due to performance reasons. This is currently the best we got to highlight search results. Keep this until we can come up with something better.

Note: Currently we transform the mail body into an AST and then render the AST to markup. That allows us to store search results from a substring search, but also renders the body at a hard, but configurable text width of 82 characters. Apart from the performance loss rendering text vs. rendering markup, one idea could be to do that only when we search for sub strings. However parsing markup and rendering the AST has more benefits and would stay.

TL;DR: I keep the mailbody rendering including Brick.Markup for now until it becomes a performance bottleneck.

romanofski commented 2 years ago

@frasertweedale this could be merged if you think it's up to scratch :)

frasertweedale commented 2 years ago

@romanofski we need to set bounds brick >= 0.68 && < 0.72.

v0.72 brought another breaking change regarding event types so we'll deal with that in a separate PR. From changelog:

0.72

Package changes:

  • Increased lower bound on text-zipper to 0.12.

API changes:

  • handleEditorEvent now takes a BrickEvent rather than just a Vty Event.
  • Brick editors now handle mouse clicks to change their cursor positions.

Within those bounds it builds, and if tests are green we can merge it.

frasertweedale commented 2 years ago

@romanofski I took the liberty of amending the bounds and updating the source branch myself. Just waiting on CI now.