Codecov Report

Attention: Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.73%. Comparing base (4eb101d) to head (dc73ca2).

@@            Coverage Diff             @@
##             main    #3752      +/-   ##
==========================================
+ Coverage   91.72%   91.73%   +0.01%     
==========================================
  Files         159      159              
  Lines       10178    10180       +2     
  Branches     2896     2885      -11     
==========================================
+ Hits         9336     9339       +3     
+ Misses        841      840       -1     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

After school, I will add tests for deno

Hi @kingmesal.
Can you review this?

Hi @EdamAme-x

Thank you for the PR. But I can't agree on this way using NO_COLOR in c.env. It will be confusing if someone uses NO_COLOR in c.env for another purpose. I think the better way is to create colorized option for the Logger middleware and write it like this:

const app = new Hono<{
  Bindings: {
    NO_COLOR: boolean
  }
}>()

app.use(async (c, next) => {
  return logger(undefined, {
    colorized: !c.env.NO_COLOR,
  })
})

But, I'm not sure it is best to add options for the second arg of logger.

We use NO_COLOR variable in process.env since it's de facto standard on the Node.js environment:

https://no-color.org/

But, I think we don't have the manner in Cloudflare Workers.

I agree there is an option to add an opt
ion. (Colored, noColor, etc)

@kingmesel
What do you think?

My perspective is that the simpler and more consistent, the solution the better.

I think if it were on the c.env it is the simplest, but I can understand the argument that it is possible (albeit highly unlikely) that someone has used an c.env.NO_COLOR somewhere.

Passing in options is useful as it makes it pretty easy for people to find and alter the logger functionality, which I think is probably the better reason to use that approach over the c.env directly.

I have created a PR to update the website with a tip on using the NO_COLOR environment variable to disable ANSI escape codes in the output from the build-in logger middleware (honojs/website#553).

@kingmesal has noted that the default logging behaviour on CloudFlare Workers is to not have colored log output, which makes this PR unnecessary (sorry @EdamAme-x!).

Note that the default behavior of the logger on CloudFlare Workers is different to Node/Bun/Deno, which default to colored status codes.

okay, I will close this

@StuartMoncrieff @EdamAme-x Thanks!