From a5fe81bbd4d57aa0d9e1f0cee5d881cc8fd614aa Mon Sep 17 00:00:00 2001 From: Honza Javorek Date: Thu, 24 Jan 2019 18:31:43 +0100 Subject: [PATCH] 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. --- docs/how-to-guides.rst | 2 +- docs/usage-js.rst | 2 +- lib/CLI.js | 9 ++- lib/configuration.js | 15 ++-- lib/options.json | 4 +- test/integration/cli/hookfiles-cli-test.js | 27 +------- test/unit/configuration-test.js | 81 +--------------------- 7 files changed, 16 insertions(+), 124 deletions(-) diff --git a/docs/how-to-guides.rst b/docs/how-to-guides.rst index c4799b2ef..d21048b0b 100644 --- a/docs/how-to-guides.rst +++ b/docs/how-to-guides.rst @@ -712,7 +712,7 @@ As you can see, the parameters go like this: :: - $ dredd -c apiaryApiKey: -c apiaryApiName: + $ dredd -j apiaryApiKey: -j apiaryApiName: In addition to using parameters and ``dredd.yml``, you can also use environment variables: diff --git a/docs/usage-js.rst b/docs/usage-js.rst index b294d0d0a..f1e36d464 100644 --- a/docs/usage-js.rst +++ b/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 diff --git a/lib/CLI.js b/lib/CLI.js index 3b65b3f88..a62e170a6 100644 --- a/lib/CLI.js +++ b/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() { diff --git a/lib/configuration.js b/lib/configuration.js index 8c12de9e5..66d5686e9 100644 --- a/lib/configuration.js +++ b/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 diff --git a/lib/options.json b/lib/options.json index 4d3009c82..931baa757 100644 --- a/lib/options.json +++ b/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": { diff --git a/test/integration/cli/hookfiles-cli-test.js b/test/integration/cli/hookfiles-cli-test.js index b65f6f95c..07ff4598b 100644 --- a/test/integration/cli/hookfiles-cli-test.js +++ b/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; diff --git a/test/unit/configuration-test.js b/test/unit/configuration-test.js index 78480dbbc..50dca652d 100644 --- a/test/unit/configuration-test.js +++ b/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'); - }); -});