Closed synapse closed 2 months ago
Hey @mcollina, sure.
I took a look at pino-pretty
and it's using colorette
to determine if colorization is supported const { isColorSupported } = require('colorette')
.
What would you recommend, install that dependency or to try grabbing the code from it?
I think we could expose it in pino-pretty, and then use it here from there.
@mcollina could you give me a review on this one: https://github.com/pinojs/pino-pretty/pull/513
Hi @mcollina, I wanted to let you know that I have made a new commit that includes a check for the presence of colors in TTY, which is set to true
by default. However, updating pino-pretty
is still necessary. Once the PR is merged and deployed, I will create another commit. Let me know if that works for you.
pino-pretty v11.1.0 is out.
I've pulled this and it does not seem to work. If I redirect the output to a file, the colors are still there.
I've pulled this and it does not seem to work. If I redirect the output to a file, the colors are still there.
Hey @mcollina just a quick question, were you running this from your local bash? I'm guessing pino-pretty.isColorSupported
is looking and setting the boolean value based on your TTY support for color and not the redirect output by looking at the colorette
package.
const isDisabled = "NO_COLOR" in env || argv.includes("--no-color")
const isForced = "FORCE_COLOR" in env || argv.includes("--color")
const isWindows = platform === "win32"
const isDumbTerminal = env.TERM === "dumb"
const isCompatibleTerminal =
tty && tty.isatty && tty.isatty(1) && env.TERM && !isDumbTerminal
const isCI =
"CI" in env &&
("GITHUB_ACTIONS" in env || "GITLAB_CI" in env || "CIRCLECI" in env)
export const isColorSupported =
!isDisabled &&
(isForced || (isWindows && !isDumbTerminal) || isCompatibleTerminal || isCI)
Do you have any suggestions on how to know, from inside the logger formatter, what will be the outcome of the log (file, std or else)? I'm am guessing this should also overwrite the isColorSupported
value, right?
running the process with CI=true
shows the colors
From the colorette
's logic when you set CI
env you should also need one of the other 3.
As for the other thing you've mention to prevent having colors when redirected to a file, we're still looking into it (will keep you in the loop). Would it make sense to have this directly in pino-pretty
?
Maybe. I'm pretty happy with how pino-pretty work, I need to do some checks first, as I might have been too quick on my last test.
Hey folks, just a quick note here, after discussing offline with @synapse . In order to figure out how existing tools handle it, we checked out jest and here's what we found.
So we started looking into how jest does it, and we discovered that:
Hey @mcollina any news on this?
This is ok. There is still one tiny bug to fix.
Consider this:
'use strict'
const fastify = require('fastify')({
logger: {
transport: {
target: './index.js',
options: {
colorize: true,
}
}
}
})
fastify.get('/', (req, reply) => {
req.log.fatal('some info')
reply.send({ hello: 'world' })
})
fastify.listen({ port: 3042 })
Note that colorize: true
is set.
If I do node example.js | less
, I get:
However if run it in the normal terminal, I get:
Hey @mcollina,
The less
command behaves differently across operating systems (in my case I get colored texts in utf-8 codes).
Here's a summary of our current status to help us decide the next steps:
colorize
option and relies on the isColorSupported
function from pino-pretty
, which is controlled through various options found in colorette
to disable the colors (see here).Unfortunately, color detection in colorette
is very basic (compared to chalk
- which is also used in libraries like jest
and tap
).
To automatically disable the output of UTF-8 color strings when not needed, we should address this issue in either the pino-pretty
or colorette
repositories.
What do you suggest as the best steps moving forward?
It doesn't seem we are in the same page. I see that the colorize: true
option generates two different outputs when the destination is a TTY or not.
However, the same does not happen with pino-pretty
. As both use the same utility, you should be able to achieve the same behavior.
@mcollina I've updated the PR with the changes and also added it in the docs
linting is failing
fixes #35
Checklist
npm run test
andnpm run benchmark