Fix: Prioritize CLI flags over config file properties (fixes #185)

Author: sttk

index.js

@@ -92,7 +92,7 @@ function run() {
     var cfgLoadOrder = ['home', 'cwd'];
     var cfg = loadConfigFiles(env.configFiles['.gulp'], cfgLoadOrder);
     opts = mergeConfigToCliFlags(opts, cfg);
-    env = mergeConfigToEnvFlags(env, cfg);
+    env = mergeConfigToEnvFlags(env, cfg, opts);
     env.configProps = cfg;
 
     // Set up event listeners for logging again after configuring.

lib/shared/cli-options.js

@@ -62,17 +62,20 @@ module.exports = {
     alias: 'depth',
     type: 'number',
     requiresArg: true,
+    default: undefined,  // To detect if this cli option is specified.
     desc: ansi.gray(
       'Specify the depth of the task dependency tree.'),
   },
   'compact-tasks': {
     type: 'boolean',
+    default: undefined,  // To detect if this cli option is specified.
     desc: ansi.gray(
       'Reduce the output of task dependency tree by printing ' +
       'only top tasks and their child tasks.'),
   },
   'sort-tasks': {
     type: 'boolean',
+    default: undefined,  // To detect if this cli option is specified.
     desc: ansi.gray(
       'Will sort top tasks of task dependency tree.'),
   },
@@ -91,24 +94,27 @@ module.exports = {
   silent: {
     alias: 'S',
     type: 'boolean',
+    default: undefined,  // To detect if this cli option is specified.
     desc: ansi.gray(
       'Suppress all gulp logging.'),
   },
   continue: {
     type: 'boolean',
+    default: undefined,  // To detect if this cli option is specified.
     desc: ansi.gray(
       'Continue execution of tasks upon failure.'),
   },
   series: {
     type: 'boolean',
+    default: undefined,  // To detect if this cli option is specified.
     desc: ansi.gray(
       'Run tasks given on the CLI in series (the default is parallel).'),
   },
   'log-level': {
     alias: 'L',
     // Type isn't needed because count acts as a boolean
     count: true,
-    // Can't use `default` because it seems to be off by one
+    default: undefined,  // To detect if this cli option is specified.
     desc: ansi.gray(
       'Set the loglevel. -L for least verbose and -LLLL for most verbose. ' +
       '-LLL is default.'),

lib/shared/config/cli-flags.js

@@ -13,7 +13,13 @@ var fromTo = {
 };
 
 function mergeConfigToCliFlags(opt, config) {
-  return copyProps(config, opt, fromTo);
+  return copyProps(config, opt, fromTo, defaults);
+}
+
+function defaults(cfgInfo, optInfo) {
+  if (optInfo.value === undefined) {
+    return cfgInfo.value;
+  }
 }
 
 module.exports = mergeConfigToCliFlags;

lib/shared/config/env-flags.js

@@ -9,18 +9,32 @@ var toFrom = {
   require: 'flags.require',
 };
 
-function mergeConfigToEnvFlags(env, config) {
-  return copyProps(env, config, toFrom, convert, true);
-}
+function mergeConfigToEnvFlags(env, config, cliOpts) {
+  // This must reverse because `flags.gulpfile` determines 2 different properties
+  var reverse = true;
+  return copyProps(env, config, toFrom, convert, reverse);
 
-function convert(configInfo, envInfo) {
-  if (envInfo.keyChain === 'configBase') {
-    return path.dirname(configInfo.value);
-  }
-  if (envInfo.keyChain === 'require') {
-    return [].concat(envInfo.value, configInfo.value);
+  function convert(configInfo, envInfo) {
+    if (envInfo.keyChain === 'configBase') {
+      if (cliOpts.gulpfile === undefined) {
+        return path.dirname(configInfo.value);
+      }
+      return;
+    }
+
+    if (envInfo.keyChain === 'configPath') {
+      if (cliOpts.gulpfile === undefined) {
+        return configInfo.value;
+      }
+      return;
+    }
+
+    if (envInfo.keyChain === 'require') {
+      return [].concat(envInfo.value, configInfo.value);
+    }
+
+    return configInfo.value;
   }
-  return configInfo.value;
 }
 
 module.exports = mergeConfigToEnvFlags;

test/config-flags-compactTasks.js

@@ -10,7 +10,7 @@ var fixturesDir = path.join(__dirname, 'fixtures/config');
 var expectedDir = path.join(__dirname, 'expected');
 var runner = require('gulp-test-tools').gulpRunner().basedir(fixturesDir);
 
-describe ('config: flags.compactTasks', function() {
+describe('config: flags.compactTasks', function() {
 
   it('Should compact task lists when `flags.compactTasks` is true in .gulp.*',
   function(done) {
@@ -52,4 +52,41 @@ describe ('config: flags.compactTasks', function() {
     }
   });
 
+  it('Should overridden by cli flag: --compact-tasks', function(done) {
+    runner
+      .chdir('flags/compactTasks/f')
+      .gulp('--tasks --compact-tasks')
+      .run(cb);
+
+    function cb(err, stdout, stderr) {
+      var filepath = path.join(expectedDir, 'flags-tasks-compact.txt');
+      var expected = fs.readFileSync(filepath, 'utf-8');
+      expected = skipLines(expected, 1);
+
+      stdout = eraseTime(skipLines(stdout, 1));
+
+      expect(stdout).toEqual(expected);
+      expect(stderr).toEqual('');
+      done(err);
+    }
+  });
+
+  it('Should overridden by cli flag: --no-compact-tasks', function(done) {
+    runner
+      .chdir('flags/compactTasks/t')
+      .gulp('--tasks --no-compact-tasks')
+      .run(cb);
+
+    function cb(err, stdout, stderr) {
+      var filepath = path.join(expectedDir, 'flags-tasks-unsorted.txt');
+      var expected = fs.readFileSync(filepath, 'utf-8');
+      expected = skipLines(expected, 1);
+
+      stdout = eraseTime(skipLines(stdout, 1));
+
+      expect(stdout).toEqual(expected);
+      expect(stderr).toEqual('');
+      done(err);
+    }
+  });
 });

test/config-flags-continue.js

@@ -64,4 +64,54 @@ describe('config: flags.continue', function() {
     }
   });
 
+  it('Should overridden by cli flag: --continue', function(done) {
+    runner
+      .chdir('flags/continue/f')
+      .gulp('--continue')
+      .run(cb);
+
+    function cb(err, stdout, stderr) {
+      expect(err).toNotEqual(null);
+
+      stdout = eraseLapse(eraseTime(skipLines(stdout, 1)));
+      expect(stdout).toEqual(
+        'Starting \'default\'...\n' +
+        'Starting \'err\'...\n' +
+        'Starting \'next\'...\n' +
+        'Finished \'next\' after ?\n' +
+        ''
+      );
+      stderr = eraseLapse(eraseTime(headLines(stderr, 2)));
+      expect(stderr).toEqual(
+        '\'err\' errored after ?\n' +
+        'Error: Error!'
+      );
+      done();
+    }
+  });
+
+  it('Should overridden by cli flag: --no-continue', function(done) {
+    runner
+      .chdir('flags/continue/t')
+      .gulp('--no-continue')
+      .run(cb);
+
+    function cb(err, stdout, stderr) {
+      expect(err).toNotEqual(null);
+
+      stdout = eraseLapse(eraseTime(skipLines(stdout, 1)));
+      expect(stdout).toEqual(
+        'Starting \'default\'...\n' +
+        'Starting \'err\'...\n' +
+        ''
+      );
+      stderr = eraseLapse(eraseTime(headLines(stderr, 2)));
+      expect(stderr).toEqual(
+        '\'err\' errored after ?\n' +
+        'Error: Error!'
+      );
+      done();
+    }
+  });
+
 });

test/config-flags-gulpfile.js

@@ -89,6 +89,21 @@ describe('config: flags.gulpfile', function() {
     }
   });
 
+  it('Should overridden by cli flag: --gulpfile', function(done) {
+    runner
+      .chdir('./flags/gulpfile/override-by-cliflag')
+      .gulp('--gulpfile mygulpfile.js')
+      .run(cb);
+
+    function cb(err, stdout, stderr) {
+      expect(err).toEqual(null);
+      expect(stderr).toEqual('');
+      stdout = headLines(stdout, 1, 2);
+      expect(stdout).toEqual('Gulpfile : ' + path.join(fixturesDir, 'flags/gulpfile/override-by-cliflag/mygulpfile.js'));
+      done(err);
+    }
+  });
+
   it('Should autoload a module for loading a specified gulpfile', function(done) {
     this.timeout(0);
 

test/config-flags-log-level.js

@@ -185,4 +185,42 @@ describe('config: flag.logLevel', function() {
         });
     });
   });
+
+  describe('Overridden by cli flag: -L/-LL/-LLL', function() {
+    it('Should not output info log by -L', function(done) {
+      var gulp = runner({ verbose: false })
+        .basedir(path.join(__dirname, 'fixtures/config/flags/logLevel/LLL'))
+        .gulp;
+
+      var packageJsonPath = path.resolve(__dirname, 'fixtures/packages',
+        'valid-package.json');
+
+      gulp('-L', '--verify', packageJsonPath)
+        .run(function(err, stdout, stderr) {
+          expect(err).toEqual(null);
+          expect(stdout).toEqual('');
+          expect(stderr).toEqual('');
+          done(err);
+        });
+    });
+
+    it('Should output info log by -LLL', function(done) {
+      var gulp = runner({ verbose: false })
+        .basedir(path.join(__dirname, 'fixtures/config/flags/logLevel/L'))
+        .gulp;
+
+      var packageJsonPath = path.resolve(__dirname, 'fixtures/packages',
+        'valid-package.json');
+
+      gulp('-LLL', '--verify', packageJsonPath)
+        .run(function(err, stdout, stderr) {
+          expect(err).toEqual(null);
+          expect(eraseTime(stdout)).toEqual(
+            'Verifying plugins in ' + packageJsonPath + '\n' +
+            'There are no blacklisted plugins in this project\n');
+          expect(stderr).toEqual('');
+          done(err);
+        });
+    });
+  });
 });

test/config-flags-series.js

@@ -58,4 +58,47 @@ describe('config: flags.series', function() {
       }
     });
 
+  it('Should overridden by cli flag: --series', function(done) {
+    runner
+      .chdir('flags/series/f')
+      .gulp('--series', 'task1 task2')
+      .run(cb);
+
+    function cb(err, stdout, stderr) {
+      expect(err).toEqual(null);
+      expect(stderr).toEqual('');
+
+      stdout = eraseLapse(eraseTime(skipLines(stdout, 1)));
+      expect(stdout).toEqual(
+        'Starting \'task1\'...\n' +
+        'Finished \'task1\' after ?\n' +
+        'Starting \'task2\'...\n' +
+        'Finished \'task2\' after ?\n' +
+        ''
+      );
+      done();
+    }
+  });
+
+  it('Should overridden by cli flag: --no-series', function(done) {
+    runner
+      .chdir('flags/series/t')
+      .gulp('--no-series', 'task1 task2')
+      .run(cb);
+
+    function cb(err, stdout, stderr) {
+      expect(err).toEqual(null);
+      expect(stderr).toEqual('');
+
+      stdout = eraseLapse(eraseTime(skipLines(stdout, 1)));
+      expect(stdout).toEqual(
+        'Starting \'task1\'...\n' +
+        'Starting \'task2\'...\n' +
+        'Finished \'task2\' after ?\n' +
+        'Finished \'task1\' after ?\n' +
+        ''
+      );
+      done();
+    }
+  });
 });

test/config-flags-silent.js

@@ -46,4 +46,37 @@ describe('config: flags.silent', function() {
     }
   });
 
+  it('Should overridden by cli flag: --silent', function(done) {
+    runner
+      .chdir('flags/silent/f')
+      .gulp('--silent')
+      .run(cb);
+
+    function cb(err, stdout, stderr) {
+      expect(err).toEqual(null);
+      expect(stderr).toEqual('');
+      expect(stdout).toEqual('');
+      done(err);
+    }
+  });
+
+  it('Should overridden by cli flag: --no-silent', function(done) {
+    runner
+      .chdir('flags/silent/t')
+      .gulp('--no-silent')
+      .run(cb);
+
+    function cb(err, stdout, stderr) {
+      expect(err).toEqual(null);
+      expect(stderr).toEqual('');
+      stdout = eraseLapse(eraseTime(skipLines(stdout, 1)));
+      expect(stdout).toEqual(
+        'Starting \'default\'...\n' +
+        'Finished \'default\' after ?\n' +
+        ''
+      );
+      done(err);
+    }
+  });
+
 });