Add setLoggerReformat #62
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chris-martin
previously approved these changes
Feb 18, 2025
This refactors the contract between `reformatTerminal` and the
construction that happens in `newLogger`.
Previously, we constructed `lReformat` directly, which is a function of
only `LogLevel` and `ByteString`. This meant that a "reformat" function
such as `reformatTerminal` had to do some extra work that all such
functions would have to do:
- Construct the `Colors` value if needed
- Return that `ByteString` as-is if it doesn't parse as `LoggedMessage`
We can pull both of these responsibilities out into `newLogger` itself
and simplify `reformatTerminal` by giving it what it needs directly.
Also, by passing `LogSettings` entirely (instead of just the
breakpoint), we've now made its signature into something that works well
as a generic "reformat" interface.
Then, by putting that logic behind a `setLoggerReformat` that
`newLogger` uses internally, we give users a hook for assign any such
"reformat" function from the outside too:
```hs
main :: IO ()
main = do
withLoggerEnv $ \logger -> do
let app = App $ setLoggerReformat myReformat logger
runAppM app -- ...
myReformat :: LogSettings -> Colors -> LogLevel -> LoggedMessage -> ByteString
myReformat = undefined
```
An extension like this doesn't interact with `LOG_FORMAT`, which may be
surprising, but it works well for situations that don't need to be
runtime-dynamic like that, such as CLIs.
I'm a bit neutral on this HLint rule, but I figured it's less work to just follow the guidance than than to configure around it.
d03eea1 to
8d5531f
Compare
chris-martin
approved these changes
Feb 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This refactors the contract between
reformatTerminaland theconstruction that happens in
newLogger.Previously, we constructed
lReformatdirectly, which is a function ofonly
LogLevelandByteString. This meant that a "reformat" functionsuch as
reformatTerminalhad to do some extra work that all suchfunctions would have to do:
Colorsvalue if neededByteStringas-is if it doesn't parse asLoggedMessageWe can pull both of these responsibilities out into
newLoggeritselfand simplify
reformatTerminalby giving it what it needs directly.Also, by passing
LogSettingsentirely (instead of just thebreakpoint), we've now made its signature into something that works well
as a generic "reformat" interface.
Then, by putting that logic behind a
setLoggerReformatthatnewLoggeruses internally, we give users a hook for assign any such"reformat" function from the outside too:
An extension like this doesn't interact with
LOG_FORMAT, which may besurprising, but it works well for situations that don't need to be
runtime-dynamic like that, such as CLIs.