Create "light" and "dark" color themes #140

Open
Marak opened this Issue Dec 8, 2011 · 13 comments

Projects

None yet

6 participants

@Marak
Contributor
Marak commented Dec 8, 2011

We should add the ability to expose winston's color configuration options to jitsu and we should create two default themes.

  1. "Light" - This would be the current color theme ( and stay the default )
  2. "Dark" - This would be a color theme consisting of black text ( instead of white text ) and be intended for use on systems with light terminal backgrounds.

The theme should be a configurable options in .jitsuconf

@FotoVerite FotoVerite was assigned Dec 8, 2011
@rtgibbons

Thanks Marak for throwing this in an issue.

I was looking over Winston and wondering if it would be as easy as defining new array colors and calling winston.addColors(config.get('colors')) where config.colors is an array defined in .jitsuconf

@FotoVerite FotoVerite was assigned Dec 8, 2011
@FotoVerite

That would make a lot of sense for user defined themes. But there still should be defaults for ease of use.

@Marak
Contributor
Marak commented Dec 8, 2011

@FotoVerite - What's up? I don't understand your comment. I proposed the default color theme remains the same.

@Marak
Contributor
Marak commented Dec 8, 2011

@rtgibbons - It should be something like that, I don't see it being more then a few lines of change. :-)

@FotoVerite

@Marak It should be default. I just meant there should be a few themes and also a way for a user to define a theme herself in the config file.

@FotoVerite

@rtgibbons It is for the most part already have a working draft but then you have the line colors that are hand coded like so.

'To sign up for Nodejitsu'.cyan.

So I built a util function colorize to deal with this. So that the colors will match the theme.

colorize('To sign up for Nodejitsu', 'help'),

@Marak
Contributor
Marak commented Dec 8, 2011

@FotoVerite - +1 That's a really good idea

We should actually remove any places where colors are hard-coded on strings like that and replace them with "style" methods instead.

I think adding this to colors might actually be the best move. It could use the same format that winston expects for logging levels?

The idea would be:

 'To sign up for Nodejitsu'.help

and then help would be defined in something like:

cliConfig.colors = {
  silly: 'magenta',
  input: 'grey',
  verbose: 'cyan',
  prompt: 'grey',
  info: 'green',
  data: 'grey',
  help: 'cyan',
  warn: 'yellow',
  debug: 'blue',
  error: 'red'
};
@jfhbrook
Contributor
jfhbrook commented Dec 8, 2011

It'd be cool too if instead of just one color you could use a function instead. This could, for example, embolden key words like "nodejitsu", or implement a custom "rainbow"-like color.

@Marak
Contributor
Marak commented Dec 8, 2011

I'm on it. This will require minor refactor to colors to add a theme engine. Should be easy.

@indexzero
Member

@Marak The theme engine for colors could get really dangerous given that you're modifying the string prototype. I think it's acceptable if the theme names defined by colors are not configurable (e.g. best-practices style names like bold, strong, emphasis, etc)

For example if someone creates a style for toString, match, split, etc it would get really messy.

@Marak
Contributor
Marak commented Dec 9, 2011

I wasn't planning on making the style names configurable.

Regardless, I'll add in a guard so colors can't override existing methods on String. There's no good reason to allow that anyway and it should only be a one liner.

Thanks!

@Marak
Contributor
Marak commented Dec 9, 2011

Which is funny...because bold is actually a real String.prototype method!

I'll probably make that one a special case :-)

@goloroden

I don't know whether this helps, but an idea might be to use a color schema that works for dark and bright as well.

See http://ethanschoonover.com/solarized for such a schema, which gained some attraction in the (.NET) development scene.

Perhaps it helps.

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