Repository: gulpjs/vinyl-fs
Pull request #182 (2016-06-29 08:12:30)

Consistent callback names
@phated This was in my aesthetix branch I linked to earlier, but couldn't make a PR for it at the time because of conflicts. It is completely cosmetic, but you said you liked it, so here goes.


Patch:
diff --git a/lib/constants.js b/lib/constants.js
new file mode 100644
index 0000000..942d9b8
--- /dev/null
+++ b/lib/constants.js
@@ -0,0 +1,7 @@
+'use strict';
+
+module.exports = {
+  MASK_MODE: parseInt('7777', 8),
+  DEFAULT_FILE_MODE: parseInt('0666', 8),
+  DEFAULT_DIR_MODE: parseInt('0777', 8),
+};
diff --git a/lib/dest/write-contents/index.js b/lib/dest/write-contents/index.js
index 6e513bd..03d0b66 100644
--- a/lib/dest/write-contents/index.js
+++ b/lib/dest/write-contents/index.js
@@ -8,32 +8,32 @@ var writeSymbolicLink = require('./write-symbolic-link');
 function writeContents(file, callback) {
   // If directory then mkdirp it
   if (file.isDirectory()) {
-    return writeDir(file, written);
+    return writeDir(file, onWritten);
   }
 
   // Stream it to disk yo
   if (file.isStream()) {
-    return writeStream(file, written);
+    return writeStream(file, onWritten);
   }
 
   // Write it as a symlink
   if (file.symlink) {
-    return writeSymbolicLink(file, written);
+    return writeSymbolicLink(file, onWritten);
   }
 
   // Write it like normal
   if (file.isBuffer()) {
-    return writeBuffer(file, written);
+    return writeBuffer(file, onWritten);
   }
 
   // If no contents then do nothing
   if (file.isNull()) {
-    return written();
+    return onWritten();
   }
 
   // This is invoked by the various writeXxx modules when they've finished
   // writing the contents.
-  function written(err) {
+  function onWritten(err) {
     if (isErrorFatal(err)) {
       return callback(err);
     }
diff --git a/lib/dest/write-contents/write-buffer.js b/lib/dest/write-contents/write-buffer.js
index 16a3469..d4e3083 100644
--- a/lib/dest/write-contents/write-buffer.js
+++ b/lib/dest/write-contents/write-buffer.js
@@ -2,7 +2,7 @@
 
 var fo = require('../../file-operations');
 
-function writeBuffer(file, written) {
+function writeBuffer(file, onWritten) {
   var opt = {
     mode: file.stat.mode,
     flag: file.flag,
@@ -12,13 +12,13 @@ function writeBuffer(file, written) {
 
   function onWriteFile(writeErr, fd) {
     if (writeErr) {
-      return fo.closeFd(writeErr, fd, written);
+      return fo.closeFd(writeErr, fd, onWritten);
     }
 
     fo.updateMetadata(fd, file, onUpdate);
 
     function onUpdate(statErr) {
-      fo.closeFd(statErr, fd, written);
+      fo.closeFd(statErr, fd, onWritten);
     }
   }
 
diff --git a/lib/dest/write-contents/write-dir.js b/lib/dest/write-contents/write-dir.js
index f6e2f36..04715e3 100644
--- a/lib/dest/write-contents/write-dir.js
+++ b/lib/dest/write-contents/write-dir.js
@@ -4,12 +4,12 @@ var fs = require('graceful-fs');
 
 var fo = require('../../file-operations');
 
-function writeDir(file, written) {
+function writeDir(file, onWritten) {
   fo.mkdirp(file.path, file.stat.mode, onMkdirp);
 
   function onMkdirp(mkdirpErr) {
     if (mkdirpErr) {
-      return written(mkdirpErr);
+      return onWritten(mkdirpErr);
     }
 
     fs.open(file.path, 'r', onOpen);
@@ -18,17 +18,17 @@ function writeDir(file, written) {
   function onOpen(openErr, fd) {
     // If we don't have access, just move along
     if (isInaccessible(openErr)) {
-      return fo.closeFd(null, fd, written);
+      return fo.closeFd(null, fd, onWritten);
     }
 
     if (openErr) {
-      return fo.closeFd(openErr, fd, written);
+      return fo.closeFd(openErr, fd, onWritten);
     }
 
     fo.updateMetadata(fd, file, onUpdate);
 
     function onUpdate(statErr) {
-      fo.closeFd(statErr, fd, written);
+      fo.closeFd(statErr, fd, onWritten);
     }
   }
 
diff --git a/lib/dest/write-contents/write-stream.js b/lib/dest/write-contents/write-stream.js
index 8683bec..95c4c72 100644
--- a/lib/dest/write-contents/write-stream.js
+++ b/lib/dest/write-contents/write-stream.js
@@ -5,24 +5,31 @@ var fs = require('graceful-fs');
 var fo = require('../../file-operations');
 var readStream = require('../../src/get-contents/read-stream');
 
-function writeStream(file, written) {
+function writeStream(file, onWritten) {
   var opt = {
     mode: file.stat.mode,
     flag: file.flag,
   };
 
   var outStream = fs.createWriteStream(file.path, opt);
+  var fd = null;
 
   file.contents.once('error', complete);
   file.contents.once('end', readStreamEnd);
   outStream.once('error', complete);
   outStream.once('finish', complete);
+  outStream.once('open', onOpen);
 
   // Streams are piped with end disabled, this prevents the
   // WriteStream from closing the file descriptor after all
   // data is written.
   file.contents.pipe(outStream, { end: false });
 
+  // Obtain the file descriptor from the "open" event.
+  function onOpen(openFd) {
+    fd = openFd;
+  }
+
   function readStreamEnd() {
     readStream(file, complete);
   }
@@ -31,7 +38,7 @@ function writeStream(file, written) {
     outStream.end(onEnd);
 
     function onEnd(endErr) {
-      written(propagatedErr || endErr);
+      onWritten(propagatedErr || endErr);
     }
   }
 
@@ -41,16 +48,17 @@ function writeStream(file, written) {
     file.contents.removeListener('end', readStreamEnd);
     outStream.removeListener('error', complete);
     outStream.removeListener('finish', complete);
+    outStream.removeListener('open', onOpen);
 
     if (streamErr) {
       return end(streamErr);
     }
 
-    if (typeof outStream.fd !== 'number') {
+    if (typeof fd !== 'number') {
       return end();
     }
 
-    fo.updateMetadata(outStream.fd, file, end);
+    fo.updateMetadata(fd, file, end);
   }
 }
 
diff --git a/lib/dest/write-contents/write-symbolic-link.js b/lib/dest/write-contents/write-symbolic-link.js
index 954e47b..d122c9a 100644
--- a/lib/dest/write-contents/write-symbolic-link.js
+++ b/lib/dest/write-contents/write-symbolic-link.js
@@ -2,14 +2,14 @@
 
 var fs = require('graceful-fs');
 
-function writeSymbolicLink(file, written) {
+function writeSymbolicLink(file, onWritten) {
   // TODO handle symlinks properly
   fs.symlink(file.symlink, file.path, function(err) {
     if (isFatalError(err)) {
-      return written(err);
+      return onWritten(err);
     }
 
-    written();
+    onWritten();
   });
 }
 
diff --git a/lib/file-operations.js b/lib/file-operations.js
index 10ef8db..6ce4e83 100644
--- a/lib/file-operations.js
+++ b/lib/file-operations.js
@@ -6,10 +6,8 @@ var assign = require('object-assign');
 var isEqual = require('lodash.isequal');
 var isValidDate = require('vali-date');
 
-// TODO shared module
-var MASK_MODE = parseInt('7777', 8);
-var DEFAULT_FILE_MODE = parseInt('0666', 8);
-var DEFAULT_DIR_MODE = parseInt('0777', 8);
+var constants = require('./constants');
+
 var APPEND_MODE_REGEXP = /a/;
 
 function closeFd(propagatedErr, fd, callback) {
@@ -32,7 +30,7 @@ function getModeDiff(fsMode, vinylMode) {
   var modeDiff = 0;
 
   if (typeof vinylMode === 'number') {
-    modeDiff = (vinylMode ^ fsMode) & MASK_MODE;
+    modeDiff = (vinylMode ^ fsMode) & constants.MASK_MODE;
   }
 
   return modeDiff;
@@ -178,7 +176,7 @@ function writeFile(filepath, data, options, callback) {
   }
 
   // Default the same as node
-  var mode = options.mode || DEFAULT_FILE_MODE;
+  var mode = options.mode || constants.DEFAULT_FILE_MODE;
   var flag = options.flag || 'w';
   var position = APPEND_MODE_REGEXP.test(flag) ? null : 0;
 
@@ -203,7 +201,7 @@ function mkdirp(dirpath, mode, callback) {
     mode = undefined;
   }
 
-  var m = mode || DEFAULT_DIR_MODE;
+  var m = mode || constants.DEFAULT_DIR_MODE;
   var cb = callback || function() {};
   dirpath = path.resolve(dirpath);
 
diff --git a/lib/src/get-contents/index.js b/lib/src/get-contents/index.js
index a404078..c00eb41 100644
--- a/lib/src/get-contents/index.js
+++ b/lib/src/get-contents/index.js
@@ -7,25 +7,33 @@ var readBuffer = require('./read-buffer');
 var readSymbolicLink = require('./read-symbolic-link');
 
 function getContents(opt) {
-  return through2.obj(opt, function(file, enc, cb) {
+  return through2.obj(opt, function(file, enc, callback) {
     // Don't fail to read a directory
     if (file.isDirectory()) {
-      return readDir(file, opt, cb);
+      return readDir(file, opt, onRead);
     }
 
     // Process symbolic links included with `followSymlinks` option
     if (file.stat && file.stat.isSymbolicLink()) {
-      return readSymbolicLink(file, opt, cb);
+      return readSymbolicLink(file, opt, onRead);
     }
 
     // Read and pass full contents
     if (opt.buffer !== false) {
-      return readBuffer(file, opt, cb);
+      return readBuffer(file, opt, onRead);
     }
 
     // Don't buffer anything - just pass streams
-    return readStream(file, opt, cb);
+    return readStream(file, opt, onRead);
+
+    // This is invoked by the various readXxx modules when they've finished
+    // reading the contents.
+    function onRead(err) {
+      callback(err, file);
+    }
   });
 }
 
+
+
 module.exports = getContents;
diff --git a/lib/src/get-contents/read-buffer.js b/lib/src/get-contents/read-buffer.js
index 894dd9c..13ac02e 100644
--- a/lib/src/get-contents/read-buffer.js
+++ b/lib/src/get-contents/read-buffer.js
@@ -3,10 +3,10 @@
 var fs = require('graceful-fs');
 var stripBom = require('strip-bom');
 
-function bufferFile(file, opt, cb) {
+function bufferFile(file, opt, onRead) {
   fs.readFile(file.path, function(err, data) {
     if (err) {
-      return cb(err);
+      return onRead(err);
     }
 
     if (opt.stripBOM) {
@@ -15,7 +15,7 @@ function bufferFile(file, opt, cb) {
       file.contents = data;
     }
 
-    cb(null, file);
+    onRead(null);
   });
 }
 
diff --git a/lib/src/get-contents/read-dir.js b/lib/src/get-contents/read-dir.js
index a800d47..38296f4 100644
--- a/lib/src/get-contents/read-dir.js
+++ b/lib/src/get-contents/read-dir.js
@@ -1,8 +1,8 @@
 'use strict';
 
-function readDir(file, opt, cb) {
+function readDir(file, opt, onRead) {
   // Do nothing for now
-  cb(null, file);
+  onRead(null);
 }
 
 module.exports = readDir;
diff --git a/lib/src/get-contents/read-stream.js b/lib/src/get-contents/read-stream.js
index 43dc126..6c44611 100644
--- a/lib/src/get-contents/read-stream.js
+++ b/lib/src/get-contents/read-stream.js
@@ -4,9 +4,9 @@ var fs = require('graceful-fs');
 var stripBom = require('strip-bom-stream');
 var lazystream = require('lazystream');
 
-function streamFile(file, opt, cb) {
+function streamFile(file, opt, onRead) {
   if (typeof opt === 'function') {
-    cb = opt;
+    onRead = opt;
     opt = {};
   }
 
@@ -20,7 +20,7 @@ function streamFile(file, opt, cb) {
     file.contents = file.contents.pipe(stripBom());
   }
 
-  cb(null, file);
+  onRead(null);
 }
 
 module.exports = streamFile;
diff --git a/lib/src/get-contents/read-symbolic-link.js b/lib/src/get-contents/read-symbolic-link.js
index 794077a..beb27c6 100644
--- a/lib/src/get-contents/read-symbolic-link.js
+++ b/lib/src/get-contents/read-symbolic-link.js
@@ -2,16 +2,16 @@
 
 var fs = require('graceful-fs');
 
-function readLink(file, opt, cb) {
+function readLink(file, opt, onRead) {
   fs.readlink(file.path, function(err, target) {
     if (err) {
-      return cb(err);
+      return onRead(err);
     }
 
     // Store the link target path
     file.symlink = target;
 
-    return cb(null, file);
+    return onRead(null);
   });
 }
 


Test patch:
diff --git a/test/dest-modes.js b/test/dest-modes.js
index 8700486..fa735a0 100644
--- a/test/dest-modes.js
+++ b/test/dest-modes.js
@@ -10,6 +10,7 @@ var expect = require('expect');
 var through = require('through2');
 
 var vfs = require('../');
+var constants = require('../lib/constants');
 
 function wipeOut() {
   this.timeout(20000);
@@ -331,14 +332,17 @@ describe('.dest() with custom modes', function() {
       return;
     }
 
-
     var inputBase = path.join(__dirname, './fixtures');
     var inputPath = path.join(__dirname, './fixtures/wow/suchempty');
     var expectedBase = path.join(__dirname, './out-fixtures/wow');
     var expectedPath = path.join(__dirname, './out-fixtures/wow/suchempty');
     // NOTE: Darwin does not set setgid
-    var expectedDirMode = (isDarwin ? parseInt('777', 8) : parseInt('2777', 8)) & ~process.umask();
-    var expectedFileMode = parseInt('677', 8) & ~process.umask();
+    var expectedDirMode = constants.DEFAULT_DIR_MODE;
+    if (!isDarwin) {
+      expectedDirMode |= parseInt('2000', 8);
+    }
+    expectedDirMode &= ~process.umask();
+    var expectedFileMode = constants.DEFAULT_FILE_MODE & ~process.umask();
 
     var firstFile = new File({
       base: inputBase,