ability to pass the 'stringify' option to common.log #74

Closed
JoeZ99 opened this Issue Dec 6, 2011 · 2 comments

Comments

Projects
None yet
2 participants

JoeZ99 commented Dec 6, 2011

file.js, L92-99

File.prototype.log = function (level, msg, meta, callback) {
  if (this.silent) {
    return callback(null, true);
  }

  var self = this, output = common.log({
    level:     level,
    message:   msg,
    meta:      meta,
    json:      this.json,
    colorize:  this.colorize,
    timestamp: this.timestamp
  }) + '\n';

common.js, L102-120

exports.log = function (options) {
  var timestampFn = typeof options.timestamp === 'function' ? options.timestamp : exports.timestamp,
      timestamp   = options.timestamp ? timestampFn() : null,
      meta        = options.meta ? exports.clone(options.meta) : null,
      output;

  if (options.json) {
    output         = meta || {};
    output.level   = options.level;
    output.message = options.message;

    if (timestamp) {
      output.timestamp = timestamp;
    }

    return typeof options.stringify === 'function' 
      ? options.stringify(output)
      : JSON.stringify(output);
  }

common.log() expects -among others- the 'stringify' option to be a function to be called when outputting the log msg.

Since common.log() is called with fixed options from within file.js (or console.js), that 'stringify' feature can not be used, and instead the log format is always JSON.stringify({level:'info', msg: 'log message'})

Since common.log already have that option, it could be passed to it through the options passed along when adding the transport.

JoeZ99 commented Dec 15, 2011

It's a very simple enhancement, and it fits perfectly in the current code. I would be more than happy to make the pull request.

Owner

indexzero commented Jan 18, 2012

@JoeZ99 Sorry. This fell through the cracks over the holidays. Yes. Please open up a pull-request @mnutt was asking for this feature last week.

indexzero closed this in 3824637 Jul 8, 2012

cjbarth referenced this issue in DefinitelyTyped/DefinitelyTyped Feb 26, 2016

Merged

Add support for Winston 'stringify' option #8280

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