quchen / prettyprinter

A modern, extensible and well-documented prettyprinter.
BSD 2-Clause "Simplified" License
293 stars 34 forks source link

Add better color support #224

Open sofia-m-a opened 2 years ago

sofia-m-a commented 2 years ago

Followup to (https://github.com/quchen/prettyprinter/issues/65)

For background, I hope to replace the current ad-hoc style module used by Chapelure (https://hackage.haskell.org/package/chapelure-0.0.1.0/docs/Chapelure-Style.html)

Notes:

sjakobi commented 2 years ago

For background, I hope to replace the current ad-hoc style module used by Chapelure (https://hackage.haskell.org/package/chapelure-0.0.1.0/docs/Chapelure-Style.html)

Note that this project is pretty conservative regarding dependencies. Any non-boot package addition to the dependency tree of the existing packages will be a pretty hard sale.

I added an extra style marker, 'Inverted', to support the SetSwapForegroundBackground of ansi-terminal, which is widely-supported. There's also SetBlinkSpeed and SetVisible, should we support those too?

I don't really have an opinion on this. If you want to do this, I suggest leaving it for a follow-up PR, so this one stays at a more manageable size.

Is there a reason we define the types Color and Intensity ourselves rather than using and reexporting the identical versions from System.Console.ANSI.Types?

I'm not sure, but I suspect that the local types exist so the internals can be changed with less compatibility hassle. Re-exports also bring their own versioning problems.

Similarly, we could re-use ConsoleIntensity and Underlining instead of Bold and Underlined, but that introduces an explicit 'None' value that might be confusing (https://github.com/quchen/prettyprinter/issues/42). Or we could replace Maybe Bold → ConsoleIntensity in AnsiStyle. I took the first approach in Chapelure.Style

Not sure about these tradeoffs. Maybe @quchen has an opinion?!

I couldn't get the doctests for prettyprinter-ansi-terminal to work: I get several errors of the form "Could not find module ‘System.Console.ANSI’" etc

Yeah, doctests don't work well with cabal. stack handles them better. Feel free to record this on the issue tracker. Maybe we can fix these ergonomics issues somehow, e.g. by adopting cabal-docspec. I think there are other recent developments in this space, e.g. doctest-parallel.