Skip to content

A11y: support NO_COLOR environment variable in output - #303

Open
MaddyGuthridge wants to merge 2 commits into
expressjs:masterfrom
MaddyGuthridge:maddy-implement-no-color
Open

A11y: support NO_COLOR environment variable in output#303
MaddyGuthridge wants to merge 2 commits into
expressjs:masterfrom
MaddyGuthridge:maddy-implement-no-color

Conversation

@MaddyGuthridge

@MaddyGuthridge MaddyGuthridge commented May 29, 2025

Copy link
Copy Markdown

Resolves #302

This is a quick fix, rather than a major change to the system Morgan uses for styled console output. In an ideal world, using NO_COLOR would disable all styling of output, which would remove the presence of control characters in things like CI logs. Currently, this PR just sets the color to 0 if NO_COLOR is set in process.env, which is acceptable for accessibility.

As a side note: I'm not used to writing code for such old versions of Node, so apologies if I made any mistakes that break that compatibility. I tried testing using Node 0.8.28, but was unable to get npm i to work correctly. Let me know if I broke anything and I'll do my best to fix it up :)

  • Implementation
  • Test suite passes with 100% coverage
  • Linting passes with no errors or warnings

I accept the certificate of origin, and am happy for my contribution to use the project's MIT software license.

@krzysdz krzysdz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that NO_COLOR=1 should entirely remove the colour-related ANSI escape codes instead of emitting the reset sequence.

@MaddyGuthridge

Copy link
Copy Markdown
Author

Makes sense to me, although that would require a bit of a bigger refactor. I am pretty flat-out right now so probably won't have time to sort that out for a few weeks at least. I'll keep it on my todo list though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

morgan('dev') does not respect NO_COLOR=1 environment variable

2 participants