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

brick-0.68 removed `Brick.Markup` #457

Closed frasertweedale closed 2 years ago

frasertweedale commented 2 years ago

Describe the bug brick v0.68 removed the Brick.Markup module - see changelog.

This breaks the build. I will push a commit to set the dep upper bound at < 0.68 but we need to migrate away from this feature.

To Reproduce Build against brick >= 0.68.

Observe compilation failure:

[42 of 47] Compiling Purebred.UI.Mail.Main ( src/Purebred/UI/Mail/Main.hs, /home/ftweedal/dev/hs/purebred/dist-newstyle/build/x86_64-linux/ghc-8.10.5/purebred-0.1.0.0/build/Purebred/UI/Mail/Main.o, /home/ftweedal/dev/hs/purebred/dist-newstyle/build/x86_64-linux/ghc-8.10.5/purebred-0.1.0.0/build/Purebred/UI/Mail/Main.dyn_o )

src/Purebred/UI/Mail/Main.hs:33:1: error:
    Could not find module ‘Brick.Markup’
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
   |
33 | import Brick.Markup (markup, (@?))
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

src/Purebred/UI/Mail/Main.hs:35:1: error:
    Could not find module ‘Data.Text.Markup’
    Use -v (or `:set -v` in ghci) to see a list of the files searched for.
   |
35 | import Data.Text.Markup (Markup, markupSet)
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cabal: Failed to build purebred-0.1.0.0 (which is required by exe:purebred
from purebred-0.1.0.0).

Expected behavior Build succeeds and program works.

frasertweedale commented 2 years ago

Assigning to you @romanofski - I think this is your department :)

romanofski commented 2 years ago

The markup module makes it possible to turn text into widgets. We use it to highlight search results. Perhaps we should copy the modules over and continue use them? What do you think @frasertweedale?

frasertweedale commented 2 years ago

@romanofski honest truth, I don't know enough about it to comment. But if it was removed from brick because:

Removed the "markup" feature, which included Data.Text.Markup, Brick.Markup, and brick-markup-demo. This feature never performed well and was awkward to use.

then maybe we should see if there's another, more robust and performant way to do it?

romanofski commented 2 years ago

then maybe we should see if there's another, more robust and performant way to do it?

Yeah there probably is, but I think for the mean time we should take it over. The thing is also that currently we alway turn the mail body into markup, but I think we only need to do this if we search the text. That might give us both worlds: the search highlighting and the speed in 99% of cases.

frasertweedale commented 2 years ago

We migrated the required code into Purebred. That solves the acute issue so I'll close this. We can tackle cleanups or performance improvements separately.