refactor(git): use execa to invoke git

Use the execa library when invoing git

package.json

@@ -77,6 +77,7 @@
  "cz-conventional-changelog": "1.2.0",
  "dedent": "0.6.0",
  "detect-indent": "4.0.0",
+ "execa": "0.5.1",
  "find-node-modules": "1.0.4",
  "find-root": "1.0.0",
  "fs-extra": "^1.0.0",
@@ -85,6 +86,7 @@
  "lodash": "4.17.2",
  "minimist": "1.2.0",
  "path-exists": "2.1.0",
+ "pify": "2.3.0",
  "shelljs": "0.7.5",
  "strip-json-comments": "2.0.1"
  },

src/cli/strategies/git-cz.js

@@ -60,16 +60,20 @@ function gitCz (rawGitArgs, environment, adapterConfig) {
  let adapterPackageJson = getParsedPackageJsonFromPath(resolvedAdapterRootPath);
  let cliPackageJson = getParsedPackageJsonFromPath(environment.cliPath);
  console.log(`cz-cli@${cliPackageJson.version}, ${adapterPackageJson.name}@${adapterPackageJson.version}\n`);
- commit(sh, inquirer, process.cwd(), prompter, {
+ commit(inquirer, process.cwd(), prompter, {
  args: parsedGitCzArgs,
  disableAppendPaths: true,
  emitData: true,
  quiet: false,
  retryLastCommit
- }, function (error) {
- if (error) {
- throw error;
- }
+ }).catch(function (error) {
+ //
+ // Throw in next tick to use Node.js built in error handling
+ //
+ // FIXME: This should probably be refactored so that this
+ // function returns a rejected promise instead...
+ //
+ process.nextTick(function () { throw error; });
  });
  });
 

src/commitizen/commit.js

@@ -1,5 +1,6 @@
 import path from 'path';
 
+import pify from 'pify';
 import dedent from 'dedent';
 import cacheDir from 'cachedir';
 import {ensureDir} from 'fs-extra';
@@ -8,62 +9,47 @@ import * as cache from './cache';
 
 export default commit;
 
-/**
- * Takes all of the final inputs needed in order to make dispatch a git commit
- */
-function dispatchGitCommit (sh, repoPath, template, options, overrideOptions, done) {
- // Commit the user input -- side effect that we'll test
- gitCommit(sh, repoPath, template, { ...options, ...overrideOptions }, function (error) {
- done(error, template);
+function askUser (inquirer, prompter) {
+ return new Promise(function (resolve, reject) {
+ prompter(inquirer, function (arg0, arg1) {
+ // Allow adapters to error out by providing an Error
+ if (arg0 instanceof Error) {
+ return reject(arg0);
+ }
+
+ resolve({ template: arg0, overrideOptions: arg1 });
  });
+ });
 }
 
  /**
  * Asynchronously commits files using commitizen
  */
-function commit (sh, inquirer, repoPath, prompter, options, done) {
+function commit (inquirer, repoPath, prompter, options) {
  var cacheDirectory = cacheDir('commitizen');
  var cachePath = path.join(cacheDirectory, 'commitizen.json');
 
- ensureDir(cacheDirectory, function (error) {
- if (error) {
- console.error("Couldn't create commitizen cache directory: ", error);
- // TODO: properly handle error?
- } else {
- if (options.retryLastCommit) {
-
- console.log('Retrying last commit attempt.');
+ return pify(ensureDir)(cacheDirectory).then(function () {
+ if (options.retryLastCommit) {
+ console.log('Retrying last commit attempt.');
 
- // We want to use the last commit instead of the current commit,
- // so lets override some options using the values from cache.
- let {
- options: retryOptions,
- overrideOptions: retryOverrideOptions,
- template: retryTemplate
- } = cache.getCacheValueSync(cachePath, repoPath);
- dispatchGitCommit(sh, repoPath, retryTemplate, retryOptions, retryOverrideOptions, done);
+ // We want to use the last commit instead of the current commit,
+ // so lets override some options using the values from cache.
+ let {
+ options: retryOptions,
+ overrideOptions: retryOverrideOptions,
+ template: retryTemplate
+ } = cache.getCacheValueSync(cachePath, repoPath);
 
- } else {
- // Get user input -- side effect that is hard to test
- prompter(inquirer, function (error, template, overrideOptions) {
- // Allow adapters to error out
- // (error: Error?, template: String, overrideOptions: Object)
- if (!(error instanceof Error)) {
- overrideOptions = template;
- template = error;
- error = null;
- }
+ return gitCommit(repoPath, retryTemplate, { ...retryOptions, ...retryOverrideOptions });
+ }
 
- if (error) {
- return done(error);
- }
+ // Get user input -- side effect that is hard to test
+ return askUser(inquirer, prompter).then(function ({ template, overrideOptions }) {
+ // We don't want to add retries to the cache, only actual commands
+ cache.setCacheValueSync(cachePath, repoPath, { template, options, overrideOptions });
 
- // We don't want to add retries to the cache, only actual commands
- cache.setCacheValueSync(cachePath, repoPath, { template, options, overrideOptions });
- dispatchGitCommit(sh, repoPath, template, options, overrideOptions, done);
- });
- }
- }
+ return gitCommit(repoPath, template, { ...options, ...overrideOptions });
+ });
  });
-
 }

src/git/add.js

@@ -1,9 +1,10 @@
+import execa from 'execa';
+
 export { addPath };
 
 /**
  * Synchronously adds a path to git staging
  */
-function addPath (sh, repoPath) {
- sh.cd(repoPath);
- sh.exec('git add .');
+function addPath (repoPath) {
+ execa.sync('git', ['add', '.'], { cwd: repoPath });
 }

src/git/commit.js

@@ -1,6 +1,7 @@
 import os from 'os';
 import {spawn} from 'child_process';
 
+import execa from 'execa';
 import dedent from 'dedent';
 import {isString} from '../common/util';
 
@@ -9,29 +10,9 @@ export { commit };
 /**
  * Asynchronously git commit at a given path with a message
  */
-function commit (sh, repoPath, message, options, done) {
- let called = false;
- let args = ['commit', '-m', dedent(message), ...(options.args || [])];
- let child = spawn('git', args, {
- cwd: repoPath,
- stdio: options.quiet ? 'ignore' : 'inherit'
- });
+function commit (repoPath, message, options) {
+ const args = ['commit', '-m', dedent(message), ...(options.args || [])];
+ const opts = { cwd: repoPath, stdio: options.quiet ? 'ignore' : 'inherit' };
 
- child.on('error', function (err) {
- if (called) return;
- called = true;
-
- done(err);
- });
-
- child.on('exit', function (code, signal) {
- if (called) return;
- called = true;
-
- if (code) {
- done(Object.assign(new Error(`git exited with error code ${code}`), { code, signal }));
- } else {
- done(null);
- }
- });
+ return execa('git', args, opts);
 }

src/git/init.js

@@ -1,9 +1,10 @@
+import execa from 'execa';
+
 export { init };
 
 /**
  * Synchronously creates a new git repo at a path
  */
-function init (sh, repoPath) {
- sh.cd(repoPath);
- sh.exec('git init');
+function init (repoPath) {
+ execa.sync('git', ['init'], { cwd: repoPath });
 }

src/git/log.js

@@ -1,18 +1,12 @@
-import { exec } from 'child_process';
+import execa from 'execa';
 
 export { log };
 
 /**
  * Asynchronously gets the git log output
  */
-function log (repoPath, done) {
- exec('git log', {
- maxBuffer: Infinity,
- cwd: repoPath
- }, function (error, stdout, stderr) {
- if (error) {
- throw error;
- }
- done(stdout);
- });
+function log (repoPath) {
+ const opts = { maxBuffer: Infinity, cwd: repoPath };
+
+ return execa.stdout('git', ['log'], opts);
 }

test/tests/commit.js

@@ -33,7 +33,7 @@ beforeEach(function () {
 
 describe('commit', function () {
 
- it('should commit simple messages', function (done) {
+ it('should commit simple messages', function () {
 
  this.timeout(config.maxTimeout); // this could take a while
 
@@ -68,16 +68,12 @@ describe('commit', function () {
 
  // Pass in inquirer but it never gets used since we've mocked out a different
  // version of prompter.
- commitizenCommit(sh, inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true}, function () {
- log(repoConfig.path, function (logOutput) {
- expect(logOutput).to.have.string(dummyCommitMessage);
- done();
- });
- });
-
+ return commitizenCommit(inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true})
+ .then(() => log(repoConfig.path))
+ .then(logOutput => expect(logOutput).to.have.string(dummyCommitMessage))
  });
 
- it('should commit message with quotes', function (done) {
+ it('should commit message with quotes', function () {
 
  this.timeout(config.maxTimeout); // this could take a while
 
@@ -112,17 +108,13 @@ describe('commit', function () {
 
  // Pass in inquirer but it never gets used since we've mocked out a different
  // version of prompter.
- commitizenCommit(sh, inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true}, function () {
- log(repoConfig.path, function (logOutput) {
- expect(logOutput).to.have.string(dummyCommitMessage);
- done();
- });
- });
-
+ return commitizenCommit(inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true})
+ .then(() => log(repoConfig.path))
+ .then(logOutput => expect(logOutput).to.have.string(dummyCommitMessage))
  });
 
 
- it('should commit multiline messages', function (done) {
+ it('should commit multiline messages', function () {
 
  this.timeout(config.maxTimeout); // this could take a while
 
@@ -173,16 +165,12 @@ describe('commit', function () {
 
  // Pass in inquirer but it never gets used since we've mocked out a different
  // version of prompter.
- commitizenCommit(sh, inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true}, function () {
- log(repoConfig.path, function (logOutput) {
- expect(logOutput).to.have.string(dummyCommitMessage);
- done();
- });
- });
-
+ return commitizenCommit(inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true})
+ .then(() => log(repoConfig.path))
+ .then(logOutput => expect(logOutput).to.have.string(dummyCommitMessage))
  });
 
- it('should allow to override git commit options', function (done) {
+ it('should allow to override git commit options', function () {
 
  this.timeout(config.maxTimeout); // this could take a while
 
@@ -222,14 +210,12 @@ describe('commit', function () {
 
  // Pass in inquirer but it never gets used since we've mocked out a different
  // version of prompter.
- commitizenCommit(sh, inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true}, function () {
- log(repoConfig.path, function (logOutput) {
+ return commitizenCommit(inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true})
+ .then(() => log(repoConfig.path))
+ .then(logOutput => {
  expect(logOutput).to.have.string(author);
  expect(logOutput).to.have.string(dummyCommitMessage);
- done();
  });
- });
-
  });
 
 });
@@ -263,11 +249,11 @@ function quickPrompterSetup (sh, repoConfig, adapterConfig, commitMessage, optio
  commit(commitMessage, options);
  }
 
- gitInit(sh, repoConfig.path);
+ gitInit(repoConfig.path);
 
  writeFilesToPath(repoConfig.files, repoConfig.path);
 
- gitAdd(sh, repoConfig.path);
+ gitAdd(repoConfig.path);
 
  // NOTE: In the real world we would not be returning
  // this we would instead be just making the commented

test/tests/staging.js

@@ -49,7 +49,7 @@ describe('staging', function () {
  }
  };
 
- gitInit(sh, repoConfig.path);
+ gitInit(repoConfig.path);
 
  staging.isClean('./@this-actually-does-not-exist', function (stagingError) {
  expect(stagingError).to.be.an.instanceof(Error);
@@ -60,7 +60,7 @@ describe('staging', function () {
 
  writeFilesToPath(repoConfig.files, repoConfig.path);
 
- gitAdd(sh, repoConfig.path);
+ gitAdd(repoConfig.path);
 
  staging.isClean(repoConfig.path, function (afterWriteStagingIsCleanError, afterWriteStagingIsClean) {
  expect(afterWriteStagingIsCleanError).to.be.null;