WIP: Adopt pretty rendering

[ony]

Purpose of PR

Unify how we render colorful and plain reports in hledger so there is no need to have them implemented twice.

Libraries considered

This is only my quick look and opinion. Maybe someone have more to add.

• pretty. Quiet old and no recent changes, but used by Cabal. Colors need to be introduced via annotations like in wl-pprint-terminfo.
• ansi-wl-pprint. Have support for colors out of the box via ansi-terminal that we use right now.
• prettyprinter. Seems to be fresh. As in pretty colors need to be supported via annotations, but there is already prettyprinter-ansi-terminal that use ansi-terminal.
• annotated-wl-pprint. Apparently stripped version ansi-wl-pprint. Used by rio-prettyprint (authored by Michael Snoyman).
• boxes. Highly oriented on layout management. Nothing
• table-layout. Looks to be highly oriented on how to render tables. No clear composition of primitive objects.

Progress

• Hledger.Data.Amount.
• Hledger.Commands.Balance.
• Hledger.Commands.Register.
• Hledger.Reports.BalanceReport.
• Benchmark added overhead for rendering.
• Review hledger-ui for potential re-use.

[simonmichael]

I've read the commits so far.. could you give a little more explanation of the goals and benefits of the proposed approach ? I'm not sure if there's a clear example yet of how it helps, or if you are just laying the groundwork so far.

[ony]

I've read the commits so far.. could you give a little more explanation of the goals and benefits of the proposed approach ?

Instead of having 4 implementations:

showMixedAmountWithoutPrice m = intercalate "\n" $ map showamt as
  where
    Mixed as = normaliseMixedAmountSquashPricesForDisplay $ mixedAmountStripPrices m
    showamt = printf (printf "%%%ds" width) . showAmountWithoutPrice
      where
        width = maximumDef 0 $ map (length . showAmount) as

cshowMixedAmountWithoutPrice m = intercalate "\n" $ map showamt as
  where
    Mixed as = normaliseMixedAmountSquashPricesForDisplay $ mixedAmountStripPrices m
    showamt a =
      (if isNegativeAmount a then color Dull Red else id) $
      printf (printf "%%%ds" width) $ showAmountWithoutPrice a
      where
        width = maximumDef 0 $ map (length . showAmount) as

showMixedAmountOneLineWithoutPrice m = intercalate ", " $ map showAmountWithoutPrice as
    where
      (Mixed as) = normaliseMixedAmountSquashPricesForDisplay $ stripPrices m
      stripPrices (Mixed as) = Mixed $ map stripprice as where stripprice a = a{aprice=Nothing}

cshowMixedAmountOneLineWithoutPrice m = intercalate ", " $ map cshowAmountWithoutPrice as
    where
      (Mixed as) = normaliseMixedAmountSquashPricesForDisplay $ stripPrices m
      stripPrices (Mixed as) = Mixed $ map stripprice as where stripprice a = a{aprice=Nothing}

We have kinda one

instance Pretty MixedAmount where
    pretty = prettyMixedAmountHelper pretty

prettyMixedAmountHelper prettyAmount m = verical `flatAlt` flat where
    Mixed as = normaliseMixedAmountSquashPricesForDisplay m
    docs = map prettyAmount as
    verical = vcatRight docs
    flat = encloseSep mempty mempty ", " docs

From which we can infer them all

showMixedAmountWithoutPrice = showWide . plain . pretty . mixedAmountStripPrices
cshowMixedAmountWithoutPrice = showWide . pretty . mixedAmountStripPrices
showMixedAmountOneLineWithoutPrice = showWide . plain . hgroup . pretty . mixedAmountStripPrices
cshowMixedAmountOneLineWithoutPrice = showWide . hgroup . pretty . mixedAmountStripPrices

• plain - strips color.
• hgroup - switches to horizontal layout.
• showWide - just renders it to String with unbounded width.

After that we can start using Doc AnsiStyle (or whatever we choose) in CLIs and benefit from flexibility to wrap text when we need or make it compact by putting in single line.

Another thing is to get rid of those helpers that can be replaced with adding mixedAmountStripPrices and/or simplifyZeroAmount.

I'm not sure if there's a clear example yet of how it helps, or if you are just laying the groundwork so far.

It supposed to be clear already. Further work might be about table layout abstraction with text re-flow inside of cells (e.g. for multi-commodity balances register looks kinda empty on the left side).

[simonmichael]

@ony, thanks. I'm getting clearer on this. I think:

Currently for debug info we use the Show typeclass augmented by pretty-show,
and for user-facing output we use various custom functions, often with "show" somewhere in their name.

You propose to replace the user output functions with ansi-wl-pprint's Pretty typeclass.
This might bring more consistency and clarity, and provide useful features and combinators.

It's probably a great idea but I propose we don't try to get it into 1.16 release.
FYI, in case it would be conflictful: there's a slight chance that tests will be ported to tasty for 1.16.

Some issues that come to mind:

• We would render everything in (ANSI) colour by default, and use ansi-wl-pprint's plain helper
  to strip colours when needed. But I imagine we'll want to be able to customise colours,
  as hledger-ui does. This can probably be done with Pretty, but it may complicate things.

• Does ansi-wl-pprint have any support for wide characters ?

[ony]

It's probably a great idea but I propose we don't try to get it into 1.16 release.

Sure. I use hledger in rolling release mode and don't care much about releases since I don't dedicate effort for them 😛 .

I'd think this PR more like a branch on the review at this moment. We should drop if during this exploration we'll not benefit or will suffer significantly on performance degradation.

FYI, in case it would be conflictful: there's a slight chance that tests will be ported to tasty for 1.16.

Not sure how this will affect. As I understand tasty is just organizer for tests. For shell tests I think it will be easier to write adapter similar to tasty-golden rather than move them all into code on top of some framework.

Some issues that come to mind:

* We would render everything in (ANSI) colour by default, and use ansi-wl-pprint's `plain` helper
  to strip colours when needed. But I imagine we'll want to be able to customise colours,
  as hledger-ui does. This can probably be done with Pretty, but it may complicate things.

Indeed. I switched to prettyprinter that allows to easily switching to something else beside AnsiStyle and do styling right before rendering to terminal. But before reaching that point we need to get rid of intermediate String/Text that we currently have.

* Does ansi-wl-pprint have any support for wide characters ?

I don't think so 🙊 ( quchen/prettyprinter#103 ). We can push this problem to them 😆 .

For pandoc it can be somehow justified since they are directly related with rendering to various formats.
But for hledger I really think it is inappropriate to embed implementation of charWidth. We should either delegate this to somewhere or we should chop off that part of hledger into self-sufficient library that lives through its own release cycles. Making it available for consumption by other projects we increase pairs of eyes that are looking for better solution.

[simonmichael]

Great. I'm all for extracting/outsourcing reusable functionality like wide chars, as long as we keep international characters working in hledger.

[Xitian9]

What's the status here? I think this has mostly been unified with the addition of AmountDisplayOpts. Is there more to do?

[ony]

@Xitian9 , if I understand correctly idea of AmountDisplayOpts is that we pass in configuration how to render to render function.

hledger/hledger-lib/Hledger/Data/Amount.hs

Line 468 in ff76927

showAmountB :: AmountDisplayOpts -> Amount -> WideBuilder

hledger/hledger-lib/Hledger/Data/Amount.hs

Lines 460 to 461 in ff76927

	showAmount :: Amount -> String
	showAmount = wbUnpack . showAmountB noColour

In this proposal idea was a bit different. Instead of passing in options color/no-color that goes through all renderers we return value that can be both colorful and non-colorful.

hledger/hledger-lib/Hledger/Utils/Pretty.hs

Lines 64 to 66 in 4c27916

	showPretty, cshowPretty :: Pretty a => a -> String
	showPretty = showWide . plain . pretty
	cshowPretty = showWide . pretty

Notice that plain is on the left side (applied to result). With migrating whole chain to to root of call from CLI we could remove those individual functions.

I.e. we can get rid of new displayColour in

hledger/hledger-lib/Hledger/Data/Amount.hs

Line 203 in ff76927

, displayColour :: Bool -- ^ Whether to colourise negative Amounts.

I'm not familiar with new code-base, so I'm not sure how right now that information reaches showAmountB, but I would expect that it had to be passed around tossing from some TransactionDisplayOpts to AmountDisplayOpts to have global effect from --colour or detection of terminal.

Beside that idea was to get rid of unique names for every data-type and use one overloaded showPretty instead.

If I would try to re-based this on top of AmountDisplayOpts it might look like:

class Render a where
    type DisplayOpts a
    renderWith :: DisplayOpts a -> a -> Display b
    render :: a -> Display (DisplayOpts a)

-- | Some "markup" maybe in some library
data Display opts where

-- | That can be arranged in plain text accounting specifics of output device (e.g. terminal width)
foldDisplayTo :: OutputInfo -> Display Void -> Text

class LiftDisplay a b where
    liftDisplay :: Display b -> Display a
    unpackDisplayOpts :: a -> b

instance LiftDisplay TransactionDisplayOpts AmountDisplayOpts where
    unpackDisplayOpts = amountDisplayOpts

instance Render Transaction where
    type DisplayOpts Transaction = TransactionDisplayOpts
    render txn = {- ... -} liftDisplay (render amt) {- ... -}

It was also an attempt to adopt use of frameworks to construct text output that does all the re-flowing and alignment for us.

I don't mind closing this draft PR since I don't have plans to invest into it in near future.

[Xitian9]

Ah, I understand. That makes a lot of sense.

I'm pushing to move out table rendering infrastructure over to table-layout, in which case their Formatted class would be a natural fit for this. Once that code is ready perhaps we can implement this within that context.

[simonmichael]

Closing for now, we can reactivate this later if needed.