Skip to content

Commit

Permalink
Fix: Prioritize CLI flags over config file properties (fixes #185)
Browse files Browse the repository at this point in the history
  • Loading branch information
sttk authored and phated committed Mar 23, 2019
1 parent 33e14d7 commit 1b80d67
Show file tree
Hide file tree
Showing 17 changed files with 347 additions and 23 deletions.
2 changes: 1 addition & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 7 additions & 1 deletion lib/shared/cli-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.'),
},
Expand All @@ -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.'),
Expand Down
8 changes: 7 additions & 1 deletion lib/shared/config/cli-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
34 changes: 24 additions & 10 deletions lib/shared/config/env-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
39 changes: 38 additions & 1 deletion test/config-flags-compactTasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
});
});
50 changes: 50 additions & 0 deletions test/config-flags-continue.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
});

});
15 changes: 15 additions & 0 deletions test/config-flags-gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
38 changes: 38 additions & 0 deletions test/config-flags-log-level.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
});
43 changes: 43 additions & 0 deletions test/config-flags-series.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
});
});
33 changes: 33 additions & 0 deletions test/config-flags-silent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});

});
Loading

0 comments on commit 1b80d67

Please sign in to comment.