fix(commit): fix multiline commits with newlines (#413)

* fix(commit): fix multiline commits with newlines

* test(commit): fix override commit options test

Author: LinusU

src/cli/parsers/git-cz.js

@@ -6,65 +6,50 @@ export {
   parse
 };
 
+const reShortMessage = /^-([a-zA-Z]*)m(.*)$/;
+const reLongMessage = /^--message(=.*)?$/;
+
 /**
- * Takes args, parses with minimist and some ugly vudoo, returns output
- * 
- * TODO: Aww shit this is ugly. Rewrite with mega leet tests plz, kthnx.
+ * Strip message declaration from git arguments
  */
 function parse (rawGitArgs) {
+  let result = [];
+  let skipNext = false;
+
+  for (const arg of rawGitArgs) {
+    let match;
 
-  var args = minimist(rawGitArgs, { 
-    alias: {
-      m: 'message'
+    if (skipNext) {
+      skipNext = false;
+      continue;
     }
-  });
 
-  // Loop through all keys
-  var output = ' ';
+    match = reShortMessage.exec(arg);
+
+    if (match) {
+      if (match[1]) {
+        result.push(`-${match[1]}`);
+      }
 
-  for (let arg in args) {
+      if (!match[2]) {
+        skipNext = true;
+      }
 
-    if (!args.hasOwnProperty(arg)) {
-      // The current property is not a direct property
       continue;
     }
+    
+    match = reLongMessage.exec(arg);
+    
+    if (match) {
+      if (!match[1]) {
+        skipNext = true;
+      }
 
-    var key = arg;
-    var value = args[arg];
-
-    /**
-     * Ugly, but this is recompiles an argument string without 
-     * any messages passed in.
-     */
-    if (key === '_' && value.length > 0) {
-      // Anything in the _ array of strings is a one off file
-      output += value.join(' ') + ' ';
-    } else if (key === 'verbose') {
-      output += '--verbose ';
-    } else if (key === 'message') {
-      /**
-       * We strip out message because we're already handling this
-       * in minimist's aliases.
-       */
-      continue;
-    } else if (isString(value)) {
-      output += '-' + key + ' ' + value + ' ';
-    } else if (isArray(value) && value.length > 0) {
-      output += '-' + key + ' ' + value.join(' -' + key) + ' ';
-    } else if (value === true || value === false) {
-      output += '-' + key + ' ';
-    } else {
-      /**
-       * Based on the current minimist object structure, we should
-       * never get here, but we'll protect against breaking changes.
-       */
       continue;
     }
-  }
 
-  if (output.trim().length < 1) {
-    return '';
-  } else {
-    return output;
+    result.push(arg);
   }
+
+  return result;
 }

src/git/commit.js

@@ -1,38 +1,37 @@
 import os from 'os';
-import {exec} from 'child_process';
+import {spawn} from 'child_process';
 
 import dedent from 'dedent';
 import {isString} from '../common/util';
 
 export { commit };
 
-function normalizeCommitMessage (message) {
-  const signs = os.platform() === 'win32' ?
-    /(")/g :
-    /(["|`])/g;
-
-  const msg = ` -m "` + dedent(message)
-    .replace(signs, '\\$1') + `"`;
-  return msg;
-}
-
 /**
  * Asynchronously git commit at a given path with a message
  */
 function commit (sh, repoPath, message, options, done) {
-  let args = options.args || '';
-  let commitMessage = normalizeCommitMessage(message);
-
-  exec(`git commit ${commitMessage} ${args}`, {
-    maxBuffer: Infinity,
+  let called = false;
+  let args = ['commit', '-m', dedent(message), ...(options.args || [])];
+  let child = spawn('git', args, {
     cwd: repoPath,
     stdio: options.quiet ? 'ignore' : 'inherit'
-  }, function (error, stdout, stderror) {
-    if (error) {
-      /* istanbul ignore next */
-      error.message = [error.message, stderror].filter(Boolean).join('\n');
-      return done(error);
+  });
+
+  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);
     }
-    done();
   });
 }

test/tests/commit.js

@@ -213,7 +213,7 @@ describe('commit', function () {
     };
 
     let options = {
-      args: `--author="${author}" --no-edit`
+      args: [`--author="${author}"`, '--no-edit']
     };
 
     // Quick setup the repos, adapter, and grab a simple prompter

test/tests/index.js

@@ -3,5 +3,6 @@ import './cli';
 import './commit';
 import './configLoader';
 import './init';
+import './parsers';
 import './staging';
 import './util';

test/tests/parsers.js

@@ -0,0 +1,36 @@
+/* eslint-env mocha */
+
+import {expect} from 'chai';
+import { parse } from '../../src/cli/parsers/git-cz';
+
+describe('parsers', () => {
+  describe('git-cz', () => {
+    it('should parse --message "Hello, World!"', () => {
+      expect(parse(['--amend', '--message', 'Hello, World!'])).to.deep.equal(['--amend']);
+    });
+
+    it('should parse --message="Hello, World!"', () => {
+      expect(parse(['--amend', '--message=Hello, World!'])).to.deep.equal(['--amend']);
+    });
+
+    it('should parse -amwip', () => {
+      expect(parse(['-amwip'])).to.deep.equal(['-a']);
+    });
+
+    it('should parse -am=wip', () => {
+      expect(parse(['-am=wip'])).to.deep.equal(['-a']);
+    });
+
+    it('should parse -am wip', () => {
+      expect(parse(['-am', 'wip'])).to.deep.equal(['-a']);
+    });
+
+    it('should parse -a -m wip -n', () => {
+      expect(parse(['-a', '-m', 'wip', '-n'])).to.deep.equal(['-a', '-n']);
+    });
+
+    it('should parse -a -m=wip -n', () => {
+      expect(parse(['-a', '-m=wip', '-n'])).to.deep.equal(['-a', '-n']);
+    });
+  });
+});