fix(mv): add -n option, make -f default behavior

nfischer · nfischer · commit 2d1ee4dae43e · 2016-01-31T15:34:01.000-08:00
diff --git a/README.md b/README.md
@@ -246,12 +246,13 @@ Removes files. The wildcard `*` is accepted.
 ### mv([options ,] source_array, dest')
 Available options:
 
-+ `-f`: force
++ `-f`: force (default behavior)
++ `-n`: no-clobber
 
 Examples:
 
 ```javascript
-mv('-f', 'file', 'dir/');
+mv('-n', 'file', 'dir/');
 mv('file1', 'file2', 'dir/');
 mv(['file1', 'file2'], 'dir/'); // same as above
 ```
diff --git a/src/common.js b/src/common.js
@@ -74,8 +74,10 @@ function parseOptions(str, map) {
 
   // All options are false by default
   var options = {};
-  for (var letter in map)
-    options[map[letter]] = false;
+  for (var letter in map) {
+    if (!map[letter].match('^!'))
+      options[map[letter]] = false;
+  }
 
   if (!str)
     return options; // defaults
@@ -91,11 +93,17 @@ function parseOptions(str, map) {
   // e.g. chars = ['R', 'f']
   var chars = match[1].split('');
 
+  var opt;
   chars.forEach(function(c) {
-    if (c in map)
-      options[map[c]] = true;
-    else
+    if (c in map) {
+      opt = map[c];
+      if (opt.match('^!'))
+        options[opt.slice(1, opt.length-1)] = false;
+      else
+        options[opt] = true;
+    } else {
       error('option not recognized: '+c);
+    }
   });
 
   return options;
diff --git a/src/mv.js b/src/mv.js
@@ -7,20 +7,22 @@ var common = require('./common');
 //@ ### mv([options ,] source_array, dest')
 //@ Available options:
 //@
-//@ + `-f`: force
+//@ + `-f`: force (default behavior)
+//@ + `-n`: no-clobber
 //@
 //@ Examples:
 //@
 //@ ```javascript
-//@ mv('-f', 'file', 'dir/');
+//@ mv('-n', 'file', 'dir/');
 //@ mv('file1', 'file2', 'dir/');
 //@ mv(['file1', 'file2'], 'dir/'); // same as above
 //@ ```
 //@
 //@ Moves files. The wildcard `*` is accepted.
 function _mv(options, sources, dest) {
   options = common.parseOptions(options, {
-    'f': 'force'
+    'f': '!no_force',
+    'n': 'no_force'
   });
 
   // Get sources, dest
@@ -47,7 +49,7 @@ function _mv(options, sources, dest) {
     common.error('dest is not a directory (too many sources)');
 
   // Dest is an existing file, but no -f given
-  if (exists && stats.isFile() && !options.force)
+  if (exists && stats.isFile() && options.no_force)
     common.error('dest file already exists: ' + dest);
 
   sources.forEach(function(src) {
@@ -64,7 +66,7 @@ function _mv(options, sources, dest) {
     if (fs.existsSync(dest) && fs.statSync(dest).isDirectory())
       thisDest = path.normalize(dest + '/' + path.basename(src));
 
-    if (fs.existsSync(thisDest) && !options.force) {
+    if (fs.existsSync(thisDest) && options.no_force) {
       common.error('dest file already exists: ' + thisDest, true);
       return; // skip file
     }
diff --git a/test/common.js b/test/common.js
@@ -46,6 +46,36 @@ var result = common.expand(['**/file*.js']);
 assert.equal(shell.error(), null);
 assert.deepEqual(result.sort(), ["resources/file1.js","resources/file2.js","resources/ls/file1.js","resources/ls/file2.js"].sort());
 
+// common.parseOptions (normal case)
+var result = common.parseOptions('-Rf', {
+  'R': 'recursive',
+  'f': 'force',
+  'r': 'reverse'
+});
+assert.ok(result.recursive === true);
+assert.ok(result.force === true);
+assert.ok(result.reverse === false);
+
+// common.parseOptions (with mutually-negating options)
+var result = common.parseOptions('-f', {
+  'n': 'no_force',
+  'f': '!no_force',
+  'R': 'recursive'
+});
+assert.ok(result.recursive === false);
+assert.ok(result.no_force === false);
+assert.ok(result.force === undefined); // this key shouldn't exist
+
+// common.parseOptions (the last of the conflicting options should hold)
+var result = common.parseOptions('-fn', {
+  'n': 'no_force',
+  'f': '!no_force',
+  'R': 'recursive'
+});
+assert.ok(result.recursive === false);
+assert.ok(result.no_force === true);
+assert.ok(result.force === undefined); // this key shouldn't exist
+
 shell.exit(123);
 
 
diff --git a/test/mv.js b/test/mv.js
@@ -43,7 +43,17 @@ assert.equal(fs.existsSync('tmp/asdfasdf2'), false);
 shell.mv('asdfasdf1', 'asdfasdf2', 'tmp/file1'); // too many sources (dest is file)
 assert.ok(shell.error());
 
-shell.mv('tmp/file1', 'tmp/file2'); // dest already exists
+// -n is no-force/no-clobber
+shell.mv('-n', 'tmp/file1', 'tmp/file2'); // dest already exists
+assert.ok(shell.error());
+
+// -f is the default behavior
+shell.cp('tmp/file1', 'tmp/tmp_file');
+shell.mv('tmp/tmp_file', 'tmp/file2'); // dest already exists (but that's ok)
+assert.ok(!shell.error());
+
+// -fn is the same as -n
+shell.mv('-fn', 'tmp/file1', 'tmp/file2');
 assert.ok(shell.error());
 
 shell.mv('tmp/file1', 'tmp/file2', 'tmp/a_file'); // too many sources (exist, but dest is file)
