-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fixes #278: adds support for verbose access logging #279
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
Conversation
currently 'verbose' simply means printing out the headers for each request.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I like this, and think it could be useful! There is a conflict, probably because this PR is so old (what's up with that?). Probably an easy fix though since it's not a big change. |
|
If there is interest in merging this I can take a look at the merge conflict and fix it. Can't imagine it would be too hard to fix |
| httpServer = require('../lib/http-server'), | ||
| portfinder = require('portfinder'), | ||
| opener = require('opener'), | ||
| columnify = require('columnify'), |
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.
I would prefer that we either don't add a new dependency, or use it for all logging (and the help output).
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.
I am not a big fan of adding this dependency. It does make the headers look better, but I'm not sure we need the extra baggage to do that. I think util.format should do just fine without any extra libraries
| ' -K --key Path to ssl key file (default: key.pem).', | ||
| '', | ||
| ' -r --robots Respond to /robots.txt [User-agent: *\\nDisallow: /]', | ||
| ' -v --verbose Log out the full headers of each request along with the typical content', |
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.
We now have -v implemented as the short option for --version, which I think is fine. This option could be just --verbose with no short version.
On the other hand, since http-server is pretty verbose by default, I wonder if this might be better called --very-verbose?
| httpServer = require('../lib/http-server'), | ||
| portfinder = require('portfinder'), | ||
| opener = require('opener'), | ||
| columnify = require('columnify'), |
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.
I am not a big fan of adding this dependency. It does make the headers look better, but I'm not sure we need the extra baggage to do that. I think util.format should do just fine without any extra libraries
| ' -K --key Path to ssl key file (default: key.pem).', | ||
| '', | ||
| ' -r --robots Respond to /robots.txt [User-agent: *\\nDisallow: /]', | ||
| ' -v --verbose Log out the full headers of each request along with the typical content', |
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.
Could we also move this up next to --silent to keep it all together?
| ' -K --key Path to ssl key file (default: key.pem).', | ||
| '', | ||
| ' -r --robots Respond to /robots.txt [User-agent: *\\nDisallow: /]', | ||
| ' -v --verbose Log out the full headers of each request along with the typical content', |
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.
This also should to be added to the README and doc/http-server.1
|
Closing due to inactivity, please re-open if we'd like to revisit this. I'll leave the associated issue open since it would be nice to have this functionality |
Fixes #278
Use
-vor--verboseto enable verbose access logging.Looks like this:
Adds dependency on
columnifyfor pretty printing the headers when enabled.