-
-
Notifications
You must be signed in to change notification settings - Fork 534
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 timestamp to debug logging #994
Conversation
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.
Thanks! Left a couple comments because the implementation looks unnecessarily clever, but I like the idea.
src/index.ts
Outdated
const timestamp = function(){}; | ||
timestamp.toString = function(){ | ||
return "[ts-node " + (new Date).toISOString() + "]"; | ||
} |
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 looks unnecessarily clever. Let's keep it simple and call the function every time we need a timestamp. function timestamp() {return "ts-node ...
src/index.ts
Outdated
timestamp.toString = function(){ | ||
return "[ts-node " + (new Date).toISOString() + "]"; | ||
} | ||
const debug = shouldDebug ? console.log.bind(console, '%s', timestamp) : () => undefined |
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.
IMO we should simplifying by writing a function.
function(...args) {console.log(timestamp(), ...args)}
I now actioned your comments. |
src/index.ts
Outdated
@@ -38,7 +38,9 @@ function yn (value: string | undefined) { | |||
* Debugging `ts-node`. | |||
*/ | |||
const shouldDebug = yn(process.env.TS_NODE_DEBUG) | |||
const debug = shouldDebug ? console.log.bind(console, 'ts-node') : () => undefined | |||
const debug = shouldDebug ? | |||
(...args: any) => console.log(`ts-node ${new Date().toISOString()}`, ...args) |
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.
Square brackets need to be re-added. Otherwise looks good.
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.
done :)
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.
Thanks!
This PR add timestamp for debug logging.
example output is
Associated with #754 (comment)