Skip to content
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

Add :status field with color as a default #171

Closed
toymachiner62 opened this issue Jun 20, 2018 · 6 comments · May be fixed by #174
Closed

Add :status field with color as a default #171

toymachiner62 opened this issue Jun 20, 2018 · 6 comments · May be fixed by #174

Comments

@toymachiner62
Copy link

I love the dev config, only I want it to have a timestamp as well.

I know that if i copy the dev config manually, I can simply add :date[iso], but then I lose the awesome colors of the response codes.

Can you either add another default format which is the dev config with a timestamp or can you simply make a :status field that by default has color? e.g. :colored-status

@TiredFalcon
Copy link

You can see how the dev format is defined in the source code of morgan:

morgan.format('dev', function developmentFormatLine (tokens, req, res) {
  // get the status code if response written
  var status = headersSent(res)
    ? res.statusCode
    : undefined

  // get status color
  var color = status >= 500 ? 31 // red
    : status >= 400 ? 33 // yellow
    : status >= 300 ? 36 // cyan
    : status >= 200 ? 32 // green
    : 0 // no color

  // get colored function
  var fn = developmentFormatLine[color]

  if (!fn) {
    // compile
    fn = developmentFormatLine[color] = compile('\x1b[0m:method :url \x1b[' +
      color + 'm:status \x1b[0m:response-time ms - :res[content-length]\x1b[0m')
  }

  return fn(tokens, req, res)
})

All that is needed is to get the status code and decide a color accordingly.

st-sloth added a commit to st-sloth/morgan that referenced this issue Jul 14, 2018
Coloring status makes sense for whatever format user wants, not just the internal `dev` one. So using `:status[color]` gives that ability.

As for collaterally removed memoization for `dev` format, well, wouln't it be better to memoize all formats that go through `compile`, not just the `dev` one? If that's the case, I'll do that separately.

resolves expressjs#171
st-sloth added a commit to st-sloth/morgan that referenced this issue Jul 14, 2018
Coloring status makes sense for whatever format user wants, not just the internal `dev` one. So using `:status-colored` gives that ability.

As for collaterally removed memoization for `dev` format, well, wouln't it be better to memoize all formats that go through `compile`, not just the `dev` one? If that's the case, I'll do that separately.

resolves expressjs#171
st-sloth added a commit to st-sloth/morgan that referenced this issue Jul 14, 2018
Coloring status makes sense for whatever format user wants, not just the internal `dev` one. So using `:status-colored` gives that ability.

Had to edit tests because, in master, status coloring is being disabled after a space following status specifically in dev format. Now coloring is disabled right after status text.

As for collaterally removed memoization for `dev` format, well, wouln't it be better to memoize all formats that go through `compile`, not just the `dev` one? If that's the case, I'll do that separately.

resolves expressjs#171
@dougwilson
Copy link
Contributor

Yea, you can always make your own format that replicates the colors or if you would like you can always make a pull request to add it here 👍

@toymachiner62
Copy link
Author

@dougwilson Got a start on it, but struggling with failing unit tests on colored string assertions. Could use help if you've got time #175

@dougwilson
Copy link
Contributor

Hi @toymachiner62 I guess I misunderstood your request. I was under the assumption you were going to make a new token called :colored-status so you can use colored status in custom formats. The :status token cannot have colors because the formats using them must be color-less as these are written to logs and read by existing ecosystem log parsing software (which is why we offer out-of-the-box support for things like "Apache" style logs as this is what existing parsing software expects). Including the color control characters into the log files will break the parsing software.

@toymachiner62
Copy link
Author

Well i was originally going to create a :colored-status, but then thought ah why not just make it the default. Didn't know the formats had to be color-less

@st-sloth
Copy link

@dougwilson what about #174 with a new token?

st-sloth added a commit to st-sloth/morgan that referenced this issue Dec 1, 2018
Coloring status makes sense for whatever format user wants, not just the internal `dev` one. So using `:status-colored` gives that ability.

Memoization for `dev` format is collaterally removed since now `dev` format is just a string and coloring is compiled as the `status-colored` token.

resolves expressjs#171
ryhinchey pushed a commit to ryhinchey/morgan that referenced this issue Feb 27, 2020
Coloring status makes sense for whatever format user wants, not just the internal `dev` one. So using `:status-colored` gives that ability.

Memoization for `dev` format is collaterally removed since now `dev` format is just a string and coloring is compiled as the `status-colored` token.

resolves expressjs#171
ryhinchey pushed a commit to ryhinchey/morgan that referenced this issue Mar 26, 2020
Coloring status makes sense for whatever format user wants, not just the internal `dev` one. So using `:status-colored` gives that ability.

Memoization for `dev` format is collaterally removed since now `dev` format is just a string and coloring is compiled as the `status-colored` token.

resolves expressjs#171
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 a pull request may close this issue.

4 participants