MarkHerhold / palin

beautiful Bristol formatter for development logging
MIT License
10 stars 5 forks source link

Feature Request: Optionally disable chalk #13

Open marcelcremer opened 5 years ago

marcelcremer commented 5 years ago

Hey,

I love to use palin for console output. It's nicely formatted, and expands stack traces from errors. I'd love to use that same feature set for files, because the default formatters "swallow" the stack traces from nested errors.

However, as chalk ist always enabled, the logfile has many control characters in it, which isn't nice to read. If chalk could be disabled by palin configuration, it would be also useful to write to files.

Best Regards Marcel

MarkHerhold commented 5 years ago

Hey Marcel, thanks for the positive feedback! 👍

I can see two options here - mono-repo to share code with a separately-published package (we could call it plainlin 😆) or a config that turns chalk.* functions into no-op functions.

The pro of the first option is that it removes a dependency but increases complexity. The second sounds be pretty straightforward but doesn't really have separation of concerns. I think I'd be inclined to go with the 2nd option. What are your thoughts?

Out of curiosity, how are you currently piping output to log files?

marcelcremer commented 5 years ago

Hey Mark,

thank you for your answer. My solution for this topic would be even more pragmatic tbh 😆

From the chalk Repository:

Color support is automatically detected, but you can override it by setting the level property. You should however only do this in your own code as it applies globally to all Chalk consumers. If you need to change this in a reusable module, create a new instance:

const ctx = new chalk.constructor({level: 0});

Levels are as follows:

All colors disabled Basic color support (16 colors) 256 color support Truecolor support (16 million colors) ... Color support is automatically detected, as is the level (see chalk.level). However, if you'd like to simply enable/disable Chalk, you can do so via the .enabled property.

Chalk is enabled by default unless explicitly disabled via the constructor or chalk.level is 0

Palin might then be configurable like:

log
  .addTarget('file')
  .withFormatter(palin, {
   color: {
    level: 0; // disable chalk
   }
  });

So we'd be relying on chalk default functionality to disable colors.

I can see two options here - mono-repo to share code with a separately-published package (we could call it plainlin 😆) or a config that turns chalk.* functions into no-op functions.

I was thinking in the same direction too. I mean, chalk is only called 8 times in the code. It might also be possible to create a function which is responsible for formatting, takes some options, and applies chalk (and maybe other formatting specifics in the future?) if it's installed (peer-dependency). But this also needs to be configurable, so someone can use console and file logging together:

format(what: string, options: any) {
  let chalk = null;
  try{
    const chalkOrigin = require('chalk');
    if(chalkOrigin)
      chalk = new chalkOrigin.constructor({level: options.level});
  }
  catch(e) {}
  if(!chalk) return what;
  else return chalk[options.color](what);
}

That option might be good for testing and has only loose coupling to chalk.

Out of curiosity, how are you currently piping output to log files?

At the moment, I'm using bristol and palin for console output, and was using the default human formatter for output files. However, as nested objects aren't sanitized good enough, I tried to also use palin for the log files (to have the same output in file - this is where it comes from).

Cheers Marcel

MarkHerhold commented 5 years ago

Hey Marcel, I elected to make the most minimal changes and limit the API surface we expose to chalk.

Please take a look at the changes and let me know what you think!

marcelcremer commented 5 years ago

Hi Mark,

from the code changes alone, I'd say perfect. But testing it still outputs colorized text into the file, even when I have only 1 palin instance (for file level logging) and no additional console-logger. Also, it looks like we have only a "global" chalk instance, so using console and file level logging together would overwrite each others settings.

I'm gonna test a bit more the next days and create a PR for this. Maybe I need to do some refactoring to encapsulate chalk a bit which could also be helpful to solve #12 . Would that be okay for you?

MarkHerhold commented 5 years ago

That would be great, @marcelcremer !