Skip to content

Commit 80c1c0a

Browse files
committed
Rewrite Terminal formatter with Prettyprinter
This change removes the manual `Text`-building of our current terminal formatter and instead uses `Prettyprinter`. This is a much better tool for the job and produces very nice output in less code. The actual resulting terminal output was improved in a few small ways: 1. The colors of attributes is improved, because `Prettyprinter`'s annotations system makes it easier to do fancier things 2. Long messages that themselves span the breakpoint are wrapped in a hanging-indent style, since that too is easier to do with `Prettyprinter`. It may have been possible to do this in a way opaque to end-users, except for one thing: colors. The best-practice way to ansi-escape `Prettyprinter` output is to use its annotations feature. Elements within a `Doc` are annotated with the bits that should be colored at render timer. It's also a best-practice to annotate your `Doc`, not directly with color escapes, but with semantic annotations (see our `Ann` type). Those can then be mapped centrally to color escapes, again as part of rendering (see our `annToAnsi` function). This approach comes with a number of nice properties for our use-case in Blammo: it means the choice of colors-or-not, and what those colors are, need not be known and passed around at the time log messages are _built_. Rather, it is centralized at the place where the log messages are rendered. Therefore, I decided to lean into this and entirely replace our `Color` module with the `Terminal.Doc.Ann` type. For example, there is no longer a feature like `getLoggerColors`, which was meant to provide end-users colors-or-not as appropriate. Instead, such users can use `Ann` all the time and the colors-or-not concern is handled later. Most users, those who are not actually building and pushing messages directly to `Logger`, are not affected by this, but those that are will require pretty big changes with this new version.
1 parent d4dbe7c commit 80c1c0a

File tree

11 files changed

+263
-325
lines changed

11 files changed

+263
-325
lines changed

Blammo/Blammo.cabal

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ source-repository head
2626
library
2727
exposed-modules:
2828
Blammo.Logging
29-
Blammo.Logging.Colors
30-
Blammo.Logging.Internal.Colors
3129
Blammo.Logging.Internal.Logger
3230
Blammo.Logging.Logger
3331
Blammo.Logging.LogSettings
@@ -36,7 +34,7 @@ library
3634
Blammo.Logging.Setup
3735
Blammo.Logging.Simple
3836
Blammo.Logging.Terminal
39-
Blammo.Logging.Terminal.LogPiece
37+
Blammo.Logging.Terminal.Doc
4038
Blammo.Logging.Test
4139
Blammo.Logging.ThreadContext
4240
Blammo.Logging.WithLogger
@@ -67,6 +65,8 @@ library
6765
, lens
6866
, monad-logger-aeson
6967
, mtl
68+
, prettyprinter
69+
, prettyprinter-ansi-terminal
7070
, text
7171
, time
7272
, unliftio
@@ -136,7 +136,6 @@ test-suite spec
136136
Blammo
137137
, aeson
138138
, base <5
139-
, bytestring
140139
, envparse
141140
, hspec
142141
, mtl

Blammo/package.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ library:
5757
- lens
5858
- monad-logger-aeson
5959
- mtl
60+
- prettyprinter
61+
- prettyprinter-ansi-terminal
6062
- text
6163
- time
6264
- vector
@@ -74,7 +76,6 @@ tests:
7476
- Blammo
7577
- aeson
7678
- envparse
77-
- bytestring
7879
- hspec
7980
- mtl
8081
- text

Blammo/src/Blammo/Logging/Colors.hs

Lines changed: 0 additions & 62 deletions
This file was deleted.

Blammo/src/Blammo/Logging/Internal/Colors.hs

Lines changed: 0 additions & 66 deletions
This file was deleted.

Blammo/src/Blammo/Logging/LogSettings.hs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ module Blammo.Logging.LogSettings
4040

4141
import Prelude
4242

43-
import Blammo.Logging.Internal.Colors (Colors)
4443
import Blammo.Logging.LogSettings.LogLevels (LogLevels)
4544
import qualified Blammo.Logging.LogSettings.LogLevels as LogLevels
45+
import Blammo.Logging.Terminal.Doc
4646
import Control.Monad.IO.Class (MonadIO (..))
4747
import Control.Monad.Logger.Aeson
4848
import System.IO (Handle, hIsTerminalDevice)
@@ -53,7 +53,7 @@ data LogSettings = LogSettings
5353
, lsDestination :: LogDestination
5454
, lsFormat :: LogFormat
5555
, lsColor :: LogColor
56-
, lsColors :: Colors -> Colors
56+
, lsColors :: Ann -> AnsiStyle
5757
, lsBreakpoint :: Int
5858
, lsConcurrency :: Maybe Int
5959
}
@@ -121,7 +121,7 @@ defaultLogSettings =
121121
, lsDestination = LogDestinationStdout
122122
, lsFormat = LogFormatTerminal
123123
, lsColor = LogColorAuto
124-
, lsColors = id
124+
, lsColors = annToAnsi
125125
, lsBreakpoint = 120
126126
, lsConcurrency = Just 1
127127
}
@@ -174,7 +174,7 @@ setLogSettingsConcurrency :: Maybe Int -> LogSettings -> LogSettings
174174
setLogSettingsConcurrency x ls = ls {lsConcurrency = x}
175175

176176
-- | Set a function to modify 'Colors' used in logging
177-
setLogSettingsColors :: (Colors -> Colors) -> LogSettings -> LogSettings
177+
setLogSettingsColors :: (Ann -> AnsiStyle) -> LogSettings -> LogSettings
178178
setLogSettingsColors f ls = ls {lsColors = f}
179179

180180
getLogSettingsLevels :: LogSettings -> LogLevels
@@ -195,7 +195,7 @@ getLogSettingsBreakpoint = lsBreakpoint
195195
getLogSettingsConcurrency :: LogSettings -> Maybe Int
196196
getLogSettingsConcurrency = lsConcurrency
197197

198-
adjustColors :: LogSettings -> Colors -> Colors
198+
adjustColors :: LogSettings -> Ann -> AnsiStyle
199199
adjustColors = lsColors
200200

201201
shouldLogLevel :: LogSettings -> LogSource -> LogLevel -> Bool

Blammo/src/Blammo/Logging/LogSettings/Env.hs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ module Blammo.Logging.LogSettings.Env
5757

5858
import Prelude
5959

60-
import Blammo.Logging.Colors (Colors (..))
6160
import Blammo.Logging.LogSettings
61+
import Blammo.Logging.Terminal.Doc
6262
import Data.Bifunctor (first)
6363
import Data.Bool (bool)
6464
import Data.Semigroup (Endo (..))
@@ -88,7 +88,7 @@ parserWith defaults =
8888
, endoVar readLogFormat setLogSettingsFormat "LOG_FORMAT"
8989
, endoSwitch (setLogSettingsColor LogColorNever) "NO_COLOR"
9090
, endoOn "dumb" (setLogSettingsColor LogColorNever) "TERM"
91-
, endoOn "true" (setLogSettingsColors fixGitHubActions) "GITHUB_ACTIONS"
91+
, endoOn "true" (setLogSettingsColors annToAnsiGHA) "GITHUB_ACTIONS"
9292
]
9393

9494
endoVar
@@ -124,17 +124,26 @@ endoWhen f = bool mempty (Endo f)
124124

125125
-- |
126126
--
127-
-- GitHub Actions doesn't support 'dim' (such content just appears white). But
128-
-- if you use 'gray', it looks like 'dim' should. But one shouldn't just use
129-
-- 'gray' all the time because that won't look right /not/ in GitHub Actions.
127+
-- GitHub Actions doesn't support 'faint' (such content just appears white). But
128+
-- if you use gray (@'colorDull' 'White'@), it looks like 'faint' should. But
129+
-- one shouldn't just use gray all the time because that won't look right /not/
130+
-- in GitHub Actions.
130131
--
131-
-- We can help by automatically substituting 'gray' for 'dim', only in the
132+
-- We can help by automatically substituting gray for 'faint', only in the
132133
-- GitHub Actions environment. We take on this extra complexity because:
133134
--
134135
-- 1. It's trivial and zero dependency
135136
-- 2. It's lower complexity overall to do here, vs from the outside
136137
-- 3. GitHub Actions is a very common logging environment, and
137138
-- 4. I suspect we'll encounter more cases where GitHub Actions can be improved
138139
-- though such means, increasing its usefulness
139-
fixGitHubActions :: Colors -> Colors
140-
fixGitHubActions colors = colors {dim = gray colors}
140+
--
141+
-- __NOTE__: for now, you can ignore all that. @prettyprinter-ansi-terminal@
142+
-- doesn't actually support 'faint' yet:
143+
--
144+
-- <https://github.com/quchen/prettyprinter/pull/224>
145+
--
146+
-- So our normal 'annToAnsi' is already using gray always (and it just looks
147+
-- bad) and this function uses it as-is for now.
148+
annToAnsiGHA :: Ann -> AnsiStyle
149+
annToAnsiGHA = annToAnsi

Blammo/src/Blammo/Logging/Logger.hs

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@ module Blammo.Logging.Logger
2525

2626
import Prelude
2727

28-
import Blammo.Logging.Colors (Colors, getColors)
2928
import Blammo.Logging.Internal.Logger
3029
import Blammo.Logging.LogSettings
3130
import Blammo.Logging.Terminal
31+
import Blammo.Logging.Terminal.Doc (Ann, Doc, RenderSettings (..))
32+
import qualified Blammo.Logging.Terminal.Doc as Doc
3233
import Blammo.Logging.Test hiding (getLoggedMessages)
3334
import qualified Blammo.Logging.Test as LoggedMessages
3435
import Control.Lens (view)
@@ -42,6 +43,7 @@ import Data.Either (partitionEithers, rights)
4243
import Data.List (intercalate)
4344
import Data.Maybe (fromMaybe)
4445
import Data.Text (Text)
46+
import Data.Text.Encoding (encodeUtf8)
4547
import GHC.Stack (HasCallStack)
4648
import System.IO (stderr, stdout)
4749
import System.Log.FastLogger (LoggerSet, defaultBufSize)
@@ -94,18 +96,14 @@ getLoggerReformat :: Logger -> LogLevel -> ByteString -> ByteString
9496
getLoggerReformat = lReformat
9597

9698
setLoggerReformat
97-
:: (LogSettings -> Colors -> LogLevel -> LoggedMessage -> ByteString)
99+
:: (LogLevel -> LoggedMessage -> Doc Ann)
98100
-> Logger
99101
-> Logger
100102
setLoggerReformat f logger =
101103
logger
102104
{ lReformat = \level bytes -> fromMaybe bytes $ do
103105
lm <- Aeson.decodeStrict bytes
104-
let colors =
105-
adjustColors (lLogSettings logger)
106-
$ getColors
107-
$ lShouldColor logger
108-
pure $ f (lLogSettings logger) colors level lm
106+
pure $ encodeUtf8 $ renderDoc logger $ f level lm
109107
}
110108

111109
getLoggerShouldLog :: Logger -> LogSource -> LogLevel -> Bool
@@ -173,15 +171,15 @@ flushLogger = do
173171
logger <- view loggerL
174172
flushLogStr logger
175173

176-
pushLogger :: (MonadIO m, MonadReader env m, HasLogger env) => Text -> m ()
177-
pushLogger msg = do
174+
pushLogger :: (MonadIO m, MonadReader env m, HasLogger env) => Doc Ann -> m ()
175+
pushLogger doc = do
178176
logger <- view loggerL
179-
pushLogStr logger $ toLogStr msg
177+
pushLogStr logger $ toLogStr $ renderDoc logger doc
180178

181-
pushLoggerLn :: (MonadIO m, MonadReader env m, HasLogger env) => Text -> m ()
182-
pushLoggerLn msg = do
179+
pushLoggerLn :: (MonadIO m, MonadReader env m, HasLogger env) => Doc Ann -> m ()
180+
pushLoggerLn doc = do
183181
logger <- view loggerL
184-
pushLogStrLn logger $ toLogStr msg
182+
pushLogStrLn logger $ toLogStr $ renderDoc logger doc
185183

186184
-- | Create a 'Logger' that will capture log messages instead of logging them
187185
--
@@ -222,3 +220,16 @@ getLoggedMessagesUnsafe = do
222220
$ "Messages were logged that didn't parse as LoggedMessage:"
223221
: failed
224222
)
223+
224+
renderDoc :: Logger -> Doc Ann -> Text
225+
renderDoc = Doc.renderDoc . loggerRenderSettings
226+
227+
loggerRenderSettings :: Logger -> RenderSettings
228+
loggerRenderSettings logger =
229+
RenderSettings
230+
{ rsUseColor = getLoggerShouldColor logger
231+
, rsPageWidth = getLogSettingsBreakpoint settings
232+
, rsAnnToAnsi = adjustColors settings
233+
}
234+
where
235+
settings = getLoggerLogSettings logger

0 commit comments

Comments
 (0)