refactor: change behavior of --color

BREAKING CHANGE: The --color option doesn't support string values ('false', 'true') anymore. To disable coloring, use --no-color instead. The -c short option has been removed.

Author: honzajavorek

docs/how-to-guides.rst

@@ -712,7 +712,7 @@ As you can see, the parameters go like this:
 
 ::
 
-   $ dredd -c apiaryApiKey:<Apiary API Key> -c apiaryApiName:<API Project Subdomain>
+   $ dredd -j apiaryApiKey:<Apiary API Key> -j apiaryApiName:<API Project Subdomain>
 
 In addition to using parameters and ``dredd.yml``, you can also use environment variables:
 

docs/usage-js.rst

@@ -56,7 +56,7 @@ Let’s have a look at an example configuration first. (Please also see the :ref
 
        'require': null, // String, When using nodejs hooks, require the given module before executing hooks
 
-       'no-color': false,
+       'color': true,
      },
 
      'emitter': EventEmitterInstance, // optional - listen to test progress, your own instance of EventEmitter

lib/CLI.js

@@ -19,12 +19,11 @@ class CLI {
   constructor(options = {}, cb) {
     this.cb = cb;
     this.finished = false;
-    ({ exit: this.exit, custom: this.custom } = options);
+    this.exit = options.exit;
+    this.custom = options.custom || {};
 
     this.setExitOrCallback();
 
-    if (!this.custom) { this.custom = {}; }
-
     if (!this.custom.cwd || typeof this.custom.cwd !== 'string') {
       this.custom.cwd = process.cwd();
     }
@@ -52,7 +51,7 @@ Example:
       .wrap(80);
 
     this.argv = this.optimist.argv;
-    this.argv = applyLoggingOptions(this.argv);
+    applyLoggingOptions(this.argv);
   }
 
   // Gracefully terminate server
@@ -188,7 +187,7 @@ ${packageData.name} v${packageData.version} \
       }
     });
 
-    this.argv = applyLoggingOptions(this.argv);
+    applyLoggingOptions(this.argv);
   }
 
   parseCustomConfig() {

lib/configuration.js

@@ -18,15 +18,10 @@ function coerceToArray(value) {
 
 
 function applyLoggingOptions(options) {
-  // The CLI layer should handle 'color' and it should be implemented
-  // as a --color/--no-color boolean option with no values
-  if (options.color === 'false') {
-    options.color = false;
-  } else if (options.color === 'true') {
-    options.color = true;
+  if (options.color === false) {
+    logger.transports.console.colorize = false;
+    reporterOutputLogger.transports.console.colorize = false;
   }
-  logger.transports.console.colorize = options.color;
-  reporterOutputLogger.transports.console.colorize = options.color;
 
   // Handling the 'loglevel' value
   if (options.loglevel) {
@@ -46,8 +41,6 @@ function applyLoggingOptions(options) {
   } else {
     logger.transports.console.level = 'warn';
   }
-
-  return options;
 }
 
 
@@ -118,7 +111,7 @@ function applyConfiguration(config) {
     configuration.options.header.push(authHeader);
   }
 
-  configuration.options = applyLoggingOptions(configuration.options);
+  applyLoggingOptions(configuration.options);
 
   if (config) {
     if (config.hooksData

lib/options.json

@@ -88,8 +88,8 @@
     "default": []
   },
   "color": {
-    "alias": "c",
-    "description": "Determines whether console output should include colors.",
+    "description": "Use --no-color to suppress colors on the output",
+    "boolean": true,
     "default": true
   },
   "loglevel": {

test/integration/cli/hookfiles-cli-test.js

@@ -508,31 +508,6 @@ describe('CLI', () => {
       });
     });
 
-    describe('when suppressing color with --color=false', () => {
-      let runtimeInfo;
-
-      before((done) => {
-        const app = createServer();
-        app.get('/machines', (req, res) => res.json([{ type: 'bulldozer', name: 'willy' }]));
-
-        const args = [
-          './test/fixtures/single-get.apib',
-          `http://127.0.0.1:${DEFAULT_SERVER_PORT}`,
-          '--color=false',
-        ];
-        runCLIWithServer(args, app, (err, info) => {
-          runtimeInfo = info;
-          done(err);
-        });
-      });
-
-      it('should print without colors', () => {
-        // If colors are not on, there is no closing color code between
-        // the "pass" and the ":"
-        assert.include(runtimeInfo.dredd.stdout, 'pass:');
-      });
-    });
-
     describe('when setting the log output level with --loglevel', () => {
       let runtimeInfo;
 
@@ -544,7 +519,7 @@ describe('CLI', () => {
           './test/fixtures/single-get.apib',
           `http://127.0.0.1:${DEFAULT_SERVER_PORT}`,
           '--loglevel=error',
-          '--color=false',
+          '--no-color',
         ];
         runCLIWithServer(args, app, (err, info) => {
           runtimeInfo = info;

test/unit/configuration-test.js

@@ -19,35 +19,9 @@ describe('configuration.applyLoggingOptions()', () => {
     reporterOutputLogger.transports.console = reporterOutputLoggerSettings;
   });
 
-  describe('with color set to true', () => {
-    let options;
-
+  describe('with color not set', () => {
     beforeEach(() => {
-      options = configuration.applyLoggingOptions({ color: true });
-    });
-
-    it('resulting configuration should contain \'color\' set to boolean true', () => {
-      assert.isTrue(options.color);
-    });
-
-    it('the application logger should be set to colorize', () => {
-      assert.isTrue(logger.transports.console.colorize);
-    });
-
-    it('the application output should be set to colorize', () => {
-      assert.isTrue(reporterOutputLogger.transports.console.colorize);
-    });
-  });
-
-  describe('with color set to \'true\' string value', () => {
-    let options;
-
-    beforeEach(() => {
-      options = configuration.applyLoggingOptions({ color: 'true' });
-    });
-
-    it('resulting configuration should contain \'color\' set to boolean true', () => {
-      assert.isTrue(options.color);
+      configuration.applyLoggingOptions({});
     });
 
     it('the application logger should be set to colorize', () => {
@@ -60,34 +34,8 @@ describe('configuration.applyLoggingOptions()', () => {
   });
 
   describe('with color set to false', () => {
-    let options;
-
-    beforeEach(() => {
-      options = configuration.applyLoggingOptions({ color: false });
-    });
-
-    it('resulting configuration should contain \'color\' set to boolean false', () => {
-      assert.isFalse(options.color);
-    });
-
-    it('the application logger should be set not to colorize', () => {
-      assert.isFalse(logger.transports.console.colorize);
-    });
-
-    it('the application output should be set not to colorize', () => {
-      assert.isFalse(reporterOutputLogger.transports.console.colorize);
-    });
-  });
-
-  describe('with color set to \'false\' string value', () => {
-    let options;
-
     beforeEach(() => {
-      options = configuration.applyLoggingOptions({ color: 'false' });
-    });
-
-    it('resulting configuration should contain \'color\' set to boolean false', () => {
-      assert.isFalse(options.color);
+      configuration.applyLoggingOptions({ color: false });
     });
 
     it('the application logger should be set not to colorize', () => {
@@ -214,26 +162,3 @@ describe('configuration.applyLoggingOptions()', () => {
     });
   });
 });
-
-describe('configuration.applyConfiguration()', () => {
-  let loggerSettings;
-  let config;
-
-  beforeEach(() => { loggerSettings = clone(logger.transports.console); });
-  afterEach(() => { logger.transports.console = loggerSettings; });
-
-  it('applies logging options', () => {
-    config = configuration.applyConfiguration({
-      options: {
-        color: 'true',
-        loglevel: 'debug',
-      },
-    });
-
-    assert.nestedPropertyVal(config, 'options.color', true);
-    assert.equal(logger.transports.console.colorize, true);
-
-    assert.nestedPropertyVal(config, 'options.loglevel', 'debug');
-    assert.equal(logger.transports.console.level, 'debug');
-  });
-});