From 1305a7cd92cfaf29aab0e1d46b6a9659c750b560 Mon Sep 17 00:00:00 2001 From: Jeroen Engels Date: Wed, 15 Feb 2017 22:25:32 +0100 Subject: [PATCH] fix: Bump inquirer to v3 and promisify everything (#34) BREAKING CHANGE: Drop support for Node < v4. This uses native Promises available from Node v4. * fix: Bump inquirer to v3.0.1. Fixes #33 to improve Windows support. * refactor: Promisify everything as inquirer uses Promises from 1.0.0 onwards --- cli.js | 73 ++++++++++++++++----------------- lib/contributors/add.js | 16 ++++---- lib/contributors/add.test.js | 53 +++++++++++------------- lib/contributors/github.js | 16 ++++---- lib/contributors/github.test.js | 21 ++++------ lib/contributors/index.js | 34 ++++++++------- lib/contributors/prompt.js | 8 ++-- lib/init/index.js | 38 +++++++---------- lib/init/prompt.js | 26 ++++++------ lib/util/config-file.js | 11 ++--- lib/util/config-file.test.js | 11 ++--- lib/util/git.js | 35 +++++++--------- lib/util/markdown.js | 9 ++-- package.json | 6 ++- 14 files changed, 164 insertions(+), 193 deletions(-) diff --git a/cli.js b/cli.js index e52969a..42451c1 100755 --- a/cli.js +++ b/cli.js @@ -39,39 +39,31 @@ var argv = yargs }) .argv; -function startGeneration(argv, cb) { - argv.files - .map(function (file) { - return path.join(cwd, file); - }) - .forEach(function (file) { - util.markdown.read(file, function (error, fileContent) { - if (error) { - return cb(error); - } +function startGeneration(argv) { + return Promise.all( + argv.files.map(file => { + const filePath = path.join(cwd, file); + return util.markdown.read(filePath) + .then(fileContent => { var newFileContent = generate(argv, argv.contributors, fileContent); - util.markdown.write(file, newFileContent, cb); + return util.markdown.write(filePath, newFileContent); }); - }); + }) + ); } -function addContribution(argv, cb) { +function addContribution(argv) { var username = argv._[1]; var contributions = argv._[2]; // Add or update contributor in the config file - updateContributors(argv, username, contributions, function (error, data) { - if (error) { - return onError(error); - } + return updateContributors(argv, username, contributions) + .then(data => { argv.contributors = data.contributors; - startGeneration(argv, function (error) { - if (error) { - return cb(error); + return startGeneration(argv) + .then(() => { + if (argv.commit) { + return util.git.commit(argv, data); } - if (!argv.commit) { - return cb(); - } - return util.git.commit(argv, data, cb); }); }); } @@ -81,10 +73,10 @@ function onError(error) { console.error(error.message); process.exit(1); } - process.exit(); + process.exit(0); } -function promptForCommand(argv, cb) { +function promptForCommand(argv) { var questions = [{ type: 'list', name: 'command', @@ -99,17 +91,24 @@ function promptForCommand(argv, cb) { when: !argv._[0], default: 0 }]; - inquirer.prompt(questions, function treatAnswers(answers) { - return cb(answers.command || argv._[0]); + + return inquirer.prompt(questions) + .then(answers => { + return answers.command || argv._[0]; }); } -promptForCommand(argv, function (command) { - if (command === 'init') { - init(onError); - } else if (command === 'generate') { - startGeneration(argv, onError); - } else if (command === 'add') { - addContribution(argv, onError); - } -}); +promptForCommand(argv) + .then(command => { + switch (command) { + case 'init': + return init(); + case 'generate': + return startGeneration(argv); + case 'add': + return addContribution(argv); + default: + throw new Error(`Unknown command ${command}`); + } + }) + .catch(onError); diff --git a/lib/contributors/add.js b/lib/contributors/add.js index cda2c05..548056d 100644 --- a/lib/contributors/add.js +++ b/lib/contributors/add.js @@ -30,21 +30,19 @@ function updateExistingContributor(options, username, contributions) { }); } -function addNewContributor(options, username, contributions, infoFetcher, cb) { - infoFetcher(username, function (error, userData) { - if (error) { - return cb(error); - } +function addNewContributor(options, username, contributions, infoFetcher) { + return infoFetcher(username) + .then(userData => { var contributor = _.assign(userData, { contributions: formatContributions(options, [], contributions) }); - return cb(null, options.contributors.concat(contributor)); + return options.contributors.concat(contributor); }); } -module.exports = function addContributor(options, username, contributions, infoFetcher, cb) { +module.exports = function addContributor(options, username, contributions, infoFetcher) { if (_.find({login: username}, options.contributors)) { - return cb(null, updateExistingContributor(options, username, contributions)); + return Promise.resolve(updateExistingContributor(options, username, contributions)); } - return addNewContributor(options, username, contributions, infoFetcher, cb); + return addNewContributor(options, username, contributions, infoFetcher); }; diff --git a/lib/contributors/add.test.js b/lib/contributors/add.test.js index 6ea20d3..6e43dd2 100644 --- a/lib/contributors/add.test.js +++ b/lib/contributors/add.test.js @@ -1,8 +1,8 @@ import test from 'ava'; import addContributor from './add'; -function mockInfoFetcher(username, cb) { - return cb(null, { +function mockInfoFetcher(username) { + return Promise.resolve({ login: username, name: 'Some name', avatar_url: 'www.avatar.url', @@ -34,27 +34,27 @@ function fixtures() { return {options}; } -test.cb('should callback with error if infoFetcher fails', t => { +test('should callback with error if infoFetcher fails', t => { const {options} = fixtures(); const username = 'login3'; const contributions = ['doc']; - function infoFetcher(username, cb) { - return cb(new Error('infoFetcher error')); + function infoFetcher() { + return Promise.reject(new Error('infoFetcher error')); } - return addContributor(options, username, contributions, infoFetcher, function (error) { - t.is(error.message, 'infoFetcher error'); - t.end(); - }); + return t.throws( + addContributor(options, username, contributions, infoFetcher), + 'infoFetcher error' + ); }); -test.cb('should add new contributor at the end of the list of contributors', t => { +test('should add new contributor at the end of the list of contributors', t => { const {options} = fixtures(); const username = 'login3'; const contributions = ['doc']; - return addContributor(options, username, contributions, mockInfoFetcher, function (error, contributors) { - t.falsy(error); + return addContributor(options, username, contributions, mockInfoFetcher) + .then(contributors => { t.is(contributors.length, 3); t.deepEqual(contributors[2], { login: 'login3', @@ -65,18 +65,17 @@ test.cb('should add new contributor at the end of the list of contributors', t = 'doc' ] }); - t.end(); }); }); -test.cb('should add new contributor at the end of the list of contributors with a url link', t => { +test('should add new contributor at the end of the list of contributors with a url link', t => { const {options} = fixtures(); const username = 'login3'; const contributions = ['doc']; options.url = 'www.foo.bar'; - return addContributor(options, username, contributions, mockInfoFetcher, function (error, contributors) { - t.falsy(error); + return addContributor(options, username, contributions, mockInfoFetcher) + .then(contributors => { t.is(contributors.length, 3); t.deepEqual(contributors[2], { login: 'login3', @@ -87,28 +86,26 @@ test.cb('should add new contributor at the end of the list of contributors with {type: 'doc', url: 'www.foo.bar'} ] }); - t.end(); }); }); -test.cb(`should not update an existing contributor's contributions where nothing has changed`, t => { +test(`should not update an existing contributor's contributions where nothing has changed`, t => { const {options} = fixtures(); const username = 'login2'; const contributions = ['blog', 'code']; - return addContributor(options, username, contributions, mockInfoFetcher, function (error, contributors) { - t.falsy(error); + return addContributor(options, username, contributions, mockInfoFetcher) + .then(contributors => { t.deepEqual(contributors, options.contributors); - t.end(); }); }); -test.cb(`should update an existing contributor's contributions if a new type is added`, t => { +test(`should update an existing contributor's contributions if a new type is added`, t => { const {options} = fixtures(); const username = 'login1'; const contributions = ['bug']; - return addContributor(options, username, contributions, mockInfoFetcher, function (error, contributors) { - t.falsy(error); + return addContributor(options, username, contributions, mockInfoFetcher) + .then(contributors => { t.is(contributors.length, 2); t.deepEqual(contributors[0], { login: 'login1', @@ -120,18 +117,17 @@ test.cb(`should update an existing contributor's contributions if a new type is 'bug' ] }); - t.end(); }); }); -test.cb(`should update an existing contributor's contributions if a new type is added with a link`, t => { +test(`should update an existing contributor's contributions if a new type is added with a link`, t => { const {options} = fixtures(); const username = 'login1'; const contributions = ['bug']; options.url = 'www.foo.bar'; - return addContributor(options, username, contributions, mockInfoFetcher, function (error, contributors) { - t.falsy(error); + return addContributor(options, username, contributions, mockInfoFetcher) + .then(contributors => { t.is(contributors.length, 2); t.deepEqual(contributors[0], { login: 'login1', @@ -143,6 +139,5 @@ test.cb(`should update an existing contributor's contributions if a new type is {type: 'bug', url: 'www.foo.bar'} ] }); - t.end(); }); }); diff --git a/lib/contributors/github.js b/lib/contributors/github.js index 3b04847..3c6a140 100644 --- a/lib/contributors/github.js +++ b/lib/contributors/github.js @@ -1,24 +1,22 @@ 'use strict'; -var request = require('request'); +var pify = require('pify'); +var request = pify(require('request')); -module.exports = function getUserInfo(username, cb) { - request.get({ +module.exports = function getUserInfo(username) { + return request.get({ url: 'https://api.github.com/users/' + username, headers: { 'User-Agent': 'request' } - }, function (error, res) { - if (error) { - return cb(error); - } + }) + .then(res => { var body = JSON.parse(res.body); - var user = { + return { login: body.login, name: body.name || username, avatar_url: body.avatar_url, profile: body.blog || body.html_url }; - return cb(null, user); }); }; diff --git a/lib/contributors/github.test.js b/lib/contributors/github.test.js index 2acbed8..0ddbaed 100644 --- a/lib/contributors/github.test.js +++ b/lib/contributors/github.test.js @@ -2,18 +2,15 @@ import test from 'ava'; import nock from 'nock'; import getUserInfo from './github'; -test.cb('should handle errors', t => { +test('should handle errors', t => { nock('https://api.github.com') .get('/users/nodisplayname') .replyWithError(404); - getUserInfo('nodisplayname', err => { - t.truthy(err); - t.end(); - }); + return t.throws(getUserInfo('nodisplayname')); }); -test.cb('should fill in the name when null is returned', t => { +test('should fill in the name when null is returned', t => { nock('https://api.github.com') .get('/users/nodisplayname') .reply(200, { @@ -23,14 +20,13 @@ test.cb('should fill in the name when null is returned', t => { html_url: 'https://github.com/nodisplayname' }); - getUserInfo('nodisplayname', (err, info) => { - t.falsy(err); + return getUserInfo('nodisplayname') + .then(info => { t.is(info.name, 'nodisplayname'); - t.end(); }); }); -test.cb('should fill in the name when an empty string is returned', t => { +test('should fill in the name when an empty string is returned', t => { nock('https://api.github.com') .get('/users/nodisplayname') .reply(200, { @@ -40,9 +36,8 @@ test.cb('should fill in the name when an empty string is returned', t => { html_url: 'https://github.com/nodisplayname' }); - getUserInfo('nodisplayname', (err, info) => { - t.falsy(err); + return getUserInfo('nodisplayname') + .then(info => { t.is(info.name, 'nodisplayname'); - t.end(); }); }); diff --git a/lib/contributors/index.js b/lib/contributors/index.js index 4f0377a..2cb908d 100644 --- a/lib/contributors/index.js +++ b/lib/contributors/index.js @@ -10,20 +10,24 @@ function isNewContributor(contributorList, username) { return !_.find({login: username}, contributorList); } -module.exports = function addContributor(options, username, contributions, cb) { - prompt(options, username, contributions, function (answers) { - add(options, answers.username, answers.contributions, github, function (error, contributors) { - if (error) { - return cb(error); - } - util.configFile.writeContributors(options.config, contributors, function (error) { - return cb(error, { - username: answers.username, - contributions: answers.contributions, - contributors: contributors, - newContributor: isNewContributor(options.contributors, answers.username) - }); - }); - }); +module.exports = function addContributor(options, username, contributions) { + const answersP = prompt(options, username, contributions); + const contributorsP = answersP + .then(answers => add(options, answers.username, answers.contributions, github)); + + const writeContributorsP = contributorsP.then( + contributors => util.configFile.writeContributors(options.config, contributors) + ); + + return Promise.all([answersP, contributorsP, writeContributorsP]) + .then(res => { + const answers = res[0]; + const contributors = res[1]; + return { + username: answers.username, + contributions: answers.contributions, + contributors: contributors, + newContributor: isNewContributor(options.contributors, answers.username) + }; }); }; diff --git a/lib/contributors/prompt.js b/lib/contributors/prompt.js index bea1c23..5b4910b 100644 --- a/lib/contributors/prompt.js +++ b/lib/contributors/prompt.js @@ -33,14 +33,12 @@ function getQuestions(options, username, contributions) { }]; } -module.exports = function prompt(options, username, contributions, cb) { +module.exports = function prompt(options, username, contributions) { var defaults = { username: username, contributions: contributions && contributions.split(',') }; var questions = getQuestions(options, username, contributions); - inquirer.prompt(questions, _.flow( - _.assign(defaults), - cb - )); + return inquirer.prompt(questions) + .then(_.assign(defaults)); }; diff --git a/lib/init/index.js b/lib/init/index.js index e7acef2..e6b0c2b 100644 --- a/lib/init/index.js +++ b/lib/init/index.js @@ -1,35 +1,25 @@ 'use strict'; -var _ = require('lodash/fp'); -var series = require('async/series'); var util = require('../util'); var prompt = require('./prompt'); var initContent = require('./init-content'); var configFile = util.configFile; var markdown = util.markdown; -function injectInFile(file, fn, cb) { - markdown.read(file, function (error, content) { - if (error) { - return cb(error); - } - markdown.write(file, fn(content), cb); - }); +function injectInFile(file, fn) { + return markdown.read(file) + .then(content => markdown.write(file, fn(content))); } -module.exports = function init(callback) { - prompt(function postPrompt(result) { - var tasks = [ - function writeConfig(cb) { - configFile.writeConfig('.all-contributorsrc', result.config, cb); - }, - function addContributorsList(cb) { - injectInFile(result.contributorFile, initContent.addContributorsList, cb); - }, - result.badgeFile && function addBadge(cb) { - injectInFile(result.badgeFile, initContent.addBadge, cb); - } - ]; - series(_.compact(tasks), callback); - }); +module.exports = function init() { + return prompt() + .then(result => { + return configFile.writeConfig('.all-contributorsrc', result.config) + .then(() => injectInFile(result.contributorFile, initContent.addContributorsList)) + .then(() => { + if (result.badgeFile) { + return injectInFile(result.badgeFile, initContent.addBadge); + } + }); + }); }; diff --git a/lib/init/prompt.js b/lib/init/prompt.js index 6ef2bbb..2319950 100644 --- a/lib/init/prompt.js +++ b/lib/init/prompt.js @@ -49,29 +49,27 @@ var uniqueFiles = _.flow( _.uniq ); -module.exports = function prompt(cb) { - git.getRepoInfo(function (error, repoInfo) { - if (error) { - return cb(error); - } +module.exports = function prompt() { + return git.getRepoInfo() + .then(repoInfo => { if (repoInfo) { questions[0].default = repoInfo.projectName; questions[1].default = repoInfo.projectOwner; } - inquirer.prompt(questions, function treatAnswers(answers) { - var config = { + return inquirer.prompt(questions); + }) + .then(answers => { + return { + config: { projectName: answers.projectName, projectOwner: answers.projectOwner, files: uniqueFiles([answers.contributorFile, answers.badgeFile]), imageSize: answers.imageSize, commit: answers.commit, contributors: [] - }; - return cb({ - config: config, - contributorFile: answers.contributorFile, - badgeFile: answers.badgeFile - }); - }); + }, + contributorFile: answers.contributorFile, + badgeFile: answers.badgeFile + }; }); }; diff --git a/lib/util/config-file.js b/lib/util/config-file.js index 5078b11..2d370c1 100644 --- a/lib/util/config-file.js +++ b/lib/util/config-file.js @@ -1,6 +1,7 @@ 'use strict'; var fs = require('fs'); +var pify = require('pify'); var _ = require('lodash/fp'); function readConfig(configPath) { @@ -14,19 +15,19 @@ function readConfig(configPath) { } } -function writeConfig(configPath, content, cb) { - return fs.writeFile(configPath, JSON.stringify(content, null, 2) + '\n', cb); +function writeConfig(configPath, content) { + return pify(fs.writeFile)(configPath, JSON.stringify(content, null, 2) + '\n'); } -function writeContributors(configPath, contributors, cb) { +function writeContributors(configPath, contributors) { var config; try { config = readConfig(configPath); } catch (error) { - return cb(error); + return Promise.reject(error); } var content = _.assign(config, {contributors: contributors}); - return writeConfig(configPath, content, cb); + return writeConfig(configPath, content); } module.exports = { diff --git a/lib/util/config-file.test.js b/lib/util/config-file.test.js index 24291d3..f532687 100644 --- a/lib/util/config-file.test.js +++ b/lib/util/config-file.test.js @@ -5,14 +5,9 @@ const absentFile = './abc'; const expected = 'Configuration file not found: ' + absentFile; test('Reading an absent configuration file throws a helpful error', t => { - t.throws(() => { - configFile.readConfig(absentFile); - }, expected); + t.throws(() => configFile.readConfig(absentFile), expected); }); -test.cb('Writing contributors in an absent configuration file throws a helpful error', t => { - configFile.writeContributors(absentFile, [], error => { - t.is(error.message, expected); - t.end(); - }); +test('Writing contributors in an absent configuration file throws a helpful error', t => { + t.throws(configFile.writeContributors(absentFile, []), expected); }); diff --git a/lib/util/git.js b/lib/util/git.js index 9711ad7..c448e0a 100644 --- a/lib/util/git.js +++ b/lib/util/git.js @@ -3,10 +3,11 @@ var path = require('path'); var spawn = require('child_process').spawn; var _ = require('lodash/fp'); +var pify = require('pify'); var commitTemplate = '<%= (newContributor ? "Add" : "Update") %> @<%= username %> as a contributor'; -function getRemoteOriginData(cb) { +var getRemoteOriginData = pify(cb => { var output = ''; var git = spawn('git', 'config --get remote.origin.url'.split(' ')); git.stdout.on('data', function (data) { @@ -17,7 +18,7 @@ function getRemoteOriginData(cb) { git.on('close', function () { cb(null, output); }); -} +}); function parse(originUrl) { var result = /:(\w+)\/([A-Za-z0-9-_]+)/.exec(originUrl); @@ -31,33 +32,27 @@ function parse(originUrl) { }; } -function getRepoInfo(cb) { - getRemoteOriginData(function (error, originUrl) { - if (error) { - return cb(error); - } - return cb(null, parse(originUrl)); - }); +function getRepoInfo() { + return getRemoteOriginData() + .then(parse); } -function spawnGitCommand(args, cb) { +var spawnGitCommand = pify((args, cb) => { var git = spawn('git', args); git.stderr.on('data', cb); git.on('close', cb); -} +}); -function commit(options, data, cb) { +function commit(options, data) { var files = options.files.concat(options.config); - var absolutePathFiles = files.map(function (file) { + var absolutePathFiles = files.map(file => { return path.resolve(process.cwd(), file); }); - spawnGitCommand(['add'].concat(absolutePathFiles), function (error) { - if (error) { - return cb(error); - } - var commitMessage = _.template(options.commitTemplate || commitTemplate)(data); - spawnGitCommand(['commit', '-m', commitMessage], cb); - }); + return spawnGitCommand(['add'].concat(absolutePathFiles)) + .then(() => { + var commitMessage = _.template(options.commitTemplate || commitTemplate)(data); + return spawnGitCommand(['commit', '-m', commitMessage]); + }); } module.exports = { diff --git a/lib/util/markdown.js b/lib/util/markdown.js index 35f9c8a..fcb6cef 100644 --- a/lib/util/markdown.js +++ b/lib/util/markdown.js @@ -1,13 +1,14 @@ 'use strict'; var fs = require('fs'); +var pify = require('pify'); -function read(filePath, cb) { - fs.readFile(filePath, 'utf8', cb); +function read(filePath) { + return pify(fs.readFile)(filePath, 'utf8'); } -function write(filePath, content, cb) { - fs.writeFile(filePath, content, cb); +function write(filePath, content) { + return pify(fs.writeFile)(filePath, content); } function injectContentBetween(lines, content, startIndex, endIndex) { diff --git a/package.json b/package.json index ac4d53b..04c0846 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,9 @@ "bin": { "all-contributors": "cli.js" }, + "engines": { + "node": ">=4" + }, "scripts": { "test": "xo && nyc ava", "semantic-release": "semantic-release pre && npm publish && semantic-release post" @@ -25,8 +28,9 @@ "homepage": "https://github.com/jfmengels/all-contributors-cli#readme", "dependencies": { "async": "^2.0.0-rc.1", - "inquirer": "^0.12.0", + "inquirer": "^3.0.1", "lodash": "^4.11.2", + "pify": "^2.3.0", "request": "^2.72.0", "yargs": "^4.7.0" },