-
-
Notifications
You must be signed in to change notification settings - Fork 566
Respect NO_COLOR environment variable in the dev format #372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -211,6 +211,15 @@ morgan.format('tiny', ':method :url :status :res[content-length] - :response-tim | |
| */ | ||
|
|
||
| morgan.format('dev', function developmentFormatLine (tokens, req, res) { | ||
| // NO_COLOR (https://no-color.org/): when present and not an empty | ||
| // string, regardless of its value, ANSI color codes must not be added | ||
| if (process.env.NO_COLOR) { | ||
| var plainFn = developmentFormatLine.plain || (developmentFormatLine.plain = | ||
| compile(':method :url :status :response-time ms - :res[content-length]')) | ||
|
Comment on lines
+217
to
+218
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although it does match the behaviour elsewhere in this function, adding attributes to the function seems like an extremely strange design decision. It's not type-safe, and makes it quite unclear where the definitions actually come from.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah it's house style and not worth a rewrite for this, but these are internal if that makes you feel any better about the type safety of it |
||
|
|
||
| return plainFn(tokens, req, res) | ||
| } | ||
|
|
||
| // get the status code if response written | ||
| var status = headersSent(res) | ||
| ? res.statusCode | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nullish coalescing operator would be preferable for the expression too, although I don't think it is supported by versions of Node as old as what Morgan targets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct, cannot use
??in this release line