-
Notifications
You must be signed in to change notification settings - Fork 29.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fs: minor refactoring #1870
fs: minor refactoring #1870
Changes from all commits
a087ed9
f53c082
73d011e
08f2691
365bc72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,10 @@ const isWindows = process.platform === 'win32'; | |
const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG); | ||
const errnoException = util._errnoException; | ||
|
||
function throwOptionsError(options) { | ||
throw new TypeError('Expected options to be either an object or a string, ' + | ||
'but got ' + typeof options + ' instead'); | ||
} | ||
|
||
function rethrow() { | ||
// Only enable in debug mode. A backtrace uses ~1000 bytes of heap space and | ||
|
@@ -45,8 +49,7 @@ function rethrow() { | |
if (err) { | ||
backtrace.stack = err.name + ': ' + err.message + | ||
backtrace.stack.substr(backtrace.name.length); | ||
err = backtrace; | ||
throw err; | ||
throw backtrace; | ||
} | ||
}; | ||
} | ||
|
@@ -91,16 +94,12 @@ function nullCheck(path, callback) { | |
er.code = 'ENOENT'; | ||
if (typeof callback !== 'function') | ||
throw er; | ||
process.nextTick(nullCheckCallNT, callback, er); | ||
process.nextTick(callback, er); | ||
return false; | ||
} | ||
return true; | ||
} | ||
|
||
function nullCheckCallNT(callback, er) { | ||
callback(er); | ||
} | ||
|
||
// Static method to set the stats properties on a Stats object. | ||
fs.Stats = function( | ||
dev, | ||
|
@@ -226,12 +225,13 @@ fs.existsSync = function(path) { | |
fs.readFile = function(path, options, callback_) { | ||
var callback = maybeCallback(arguments[arguments.length - 1]); | ||
|
||
if (!options || typeof options === 'function') | ||
if (!options || typeof options === 'function') { | ||
options = { encoding: null, flag: 'r' }; | ||
else if (typeof options === 'string') | ||
} else if (typeof options === 'string') { | ||
options = { encoding: options, flag: 'r' }; | ||
else if (typeof options !== 'object') | ||
throw new TypeError('Bad arguments'); | ||
} else if (typeof options !== 'object') { | ||
throwOptionsError(options); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like the {} were added for stylistic reasons? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @trevnorris Yup, when I was preparing the patch, I am not sure why, but linter was complaining about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thefourtheye it is probably this eslint bug again. We already encountered it in #1742 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @targos Yup, that looks more like it. But, I just locally ran |
||
|
||
var encoding = options.encoding; | ||
assertEncoding(encoding); | ||
|
@@ -266,27 +266,25 @@ function ReadFileContext(callback, encoding) { | |
} | ||
|
||
ReadFileContext.prototype.read = function() { | ||
var fd = this.fd; | ||
var size = this.size; | ||
var buffer; | ||
var offset; | ||
var length; | ||
|
||
if (size === 0) { | ||
if (this.size === 0) { | ||
buffer = this.buffer = new SlowBuffer(kReadFileBufferLength); | ||
offset = 0; | ||
length = kReadFileBufferLength; | ||
} else { | ||
buffer = this.buffer; | ||
offset = this.pos; | ||
length = size - this.pos; | ||
length = this.size - this.pos; | ||
} | ||
|
||
var req = new FSReqWrap(); | ||
req.oncomplete = readFileAfterRead; | ||
req.context = this; | ||
|
||
binding.read(fd, buffer, offset, length, -1, req); | ||
binding.read(this.fd, buffer, offset, length, -1, req); | ||
}; | ||
|
||
ReadFileContext.prototype.close = function(err) { | ||
|
@@ -301,8 +299,7 @@ function readFileAfterOpen(err, fd) { | |
var context = this.context; | ||
|
||
if (err) { | ||
var callback = context.callback; | ||
callback(err); | ||
context.callback(err); | ||
return; | ||
} | ||
|
||
|
@@ -389,7 +386,7 @@ fs.readFileSync = function(path, options) { | |
} else if (typeof options === 'string') { | ||
options = { encoding: options, flag: 'r' }; | ||
} else if (typeof options !== 'object') { | ||
throw new TypeError('Bad arguments'); | ||
throwOptionsError(options); | ||
} | ||
|
||
var encoding = options.encoding; | ||
|
@@ -416,7 +413,7 @@ fs.readFileSync = function(path, options) { | |
if (size === 0) { | ||
buffers = []; | ||
} else { | ||
var threw = true; | ||
threw = true; | ||
try { | ||
buffer = new Buffer(size); | ||
threw = false; | ||
|
@@ -426,16 +423,18 @@ fs.readFileSync = function(path, options) { | |
} | ||
|
||
var done = false; | ||
var bytesRead; | ||
|
||
while (!done) { | ||
var threw = true; | ||
threw = true; | ||
try { | ||
if (size !== 0) { | ||
var bytesRead = fs.readSync(fd, buffer, pos, size - pos); | ||
bytesRead = fs.readSync(fd, buffer, pos, size - pos); | ||
} else { | ||
// the kernel lies about many files. | ||
// Go ahead and try to read some bytes. | ||
buffer = new Buffer(8192); | ||
var bytesRead = fs.readSync(fd, buffer, 0, 8192); | ||
bytesRead = fs.readSync(fd, buffer, 0, 8192); | ||
if (bytesRead) { | ||
buffers.push(buffer.slice(0, bytesRead)); | ||
} | ||
|
@@ -528,8 +527,8 @@ function modeNum(m, def) { | |
return undefined; | ||
} | ||
|
||
fs.open = function(path, flags, mode, callback) { | ||
callback = makeCallback(arguments[arguments.length - 1]); | ||
fs.open = function(path, flags, mode, callback_) { | ||
var callback = makeCallback(arguments[arguments.length - 1]); | ||
mode = modeNum(mode, 0o666); | ||
|
||
if (!nullCheck(path, callback)) return; | ||
|
@@ -584,10 +583,12 @@ fs.read = function(fd, buffer, offset, length, position, callback) { | |
|
||
fs.readSync = function(fd, buffer, offset, length, position) { | ||
var legacy = false; | ||
var encoding; | ||
|
||
if (!(buffer instanceof Buffer)) { | ||
// legacy string interface (fd, length, position, encoding, callback) | ||
legacy = true; | ||
var encoding = arguments[3]; | ||
encoding = arguments[3]; | ||
|
||
assertEncoding(encoding); | ||
|
||
|
@@ -622,14 +623,14 @@ fs.write = function(fd, buffer, offset, length, position, callback) { | |
callback(err, written || 0, buffer); | ||
} | ||
|
||
var req = new FSReqWrap(); | ||
if (buffer instanceof Buffer) { | ||
// if no position is passed then assume null | ||
if (typeof position === 'function') { | ||
callback = position; | ||
position = null; | ||
} | ||
callback = maybeCallback(callback); | ||
var req = new FSReqWrap(); | ||
req.oncomplete = strWrapper; | ||
return binding.writeBuffer(fd, buffer, offset, length, position, req); | ||
} | ||
|
@@ -646,7 +647,6 @@ fs.write = function(fd, buffer, offset, length, position, callback) { | |
length = 'utf8'; | ||
} | ||
callback = maybeCallback(position); | ||
var req = new FSReqWrap(); | ||
req.oncomplete = bufWrapper; | ||
return binding.writeString(fd, buffer, offset, length, req); | ||
}; | ||
|
@@ -720,8 +720,10 @@ fs.truncateSync = function(path, len) { | |
} | ||
// allow error to be thrown, but still close fd. | ||
var fd = fs.openSync(path, 'r+'); | ||
var ret; | ||
|
||
try { | ||
var ret = fs.ftruncateSync(fd, len); | ||
ret = fs.ftruncateSync(fd, len); | ||
} finally { | ||
fs.closeSync(fd); | ||
} | ||
|
@@ -874,7 +876,7 @@ function preprocessSymlinkDestination(path, type, linkPath) { | |
} | ||
} | ||
|
||
fs.symlink = function(destination, path, type_, callback) { | ||
fs.symlink = function(destination, path, type_, callback_) { | ||
var type = (typeof type_ === 'string' ? type_ : null); | ||
var callback = makeCallback(arguments[arguments.length - 1]); | ||
|
||
|
@@ -967,9 +969,9 @@ if (constants.hasOwnProperty('O_SYMLINK')) { | |
|
||
// prefer to return the chmod error, if one occurs, | ||
// but still try to close, and report closing errors if they occur. | ||
var err, err2; | ||
var err, err2, ret; | ||
try { | ||
var ret = fs.fchmodSync(fd, mode); | ||
ret = fs.fchmodSync(fd, mode); | ||
} catch (er) { | ||
err = er; | ||
} | ||
|
@@ -1087,8 +1089,8 @@ fs.futimesSync = function(fd, atime, mtime) { | |
binding.futimes(fd, atime, mtime); | ||
}; | ||
|
||
function writeAll(fd, buffer, offset, length, position, callback) { | ||
callback = maybeCallback(arguments[arguments.length - 1]); | ||
function writeAll(fd, buffer, offset, length, position, callback_) { | ||
var callback = maybeCallback(arguments[arguments.length - 1]); | ||
|
||
// write(fd, buffer, offset, length, position, callback) | ||
fs.write(fd, buffer, offset, length, position, function(writeErr, written) { | ||
|
@@ -1111,15 +1113,15 @@ function writeAll(fd, buffer, offset, length, position, callback) { | |
}); | ||
} | ||
|
||
fs.writeFile = function(path, data, options, callback) { | ||
fs.writeFile = function(path, data, options, callback_) { | ||
var callback = maybeCallback(arguments[arguments.length - 1]); | ||
|
||
if (!options || typeof options === 'function') { | ||
options = { encoding: 'utf8', mode: 0o666, flag: 'w' }; | ||
} else if (typeof options === 'string') { | ||
options = { encoding: options, mode: 0o666, flag: 'w' }; | ||
} else if (typeof options !== 'object') { | ||
throw new TypeError('Bad arguments'); | ||
throwOptionsError(options); | ||
} | ||
|
||
assertEncoding(options.encoding); | ||
|
@@ -1143,7 +1145,7 @@ fs.writeFileSync = function(path, data, options) { | |
} else if (typeof options === 'string') { | ||
options = { encoding: options, mode: 0o666, flag: 'w' }; | ||
} else if (typeof options !== 'object') { | ||
throw new TypeError('Bad arguments'); | ||
throwOptionsError(options); | ||
} | ||
|
||
assertEncoding(options.encoding); | ||
|
@@ -1178,7 +1180,7 @@ fs.appendFile = function(path, data, options, callback_) { | |
} else if (typeof options === 'string') { | ||
options = { encoding: options, mode: 0o666, flag: 'a' }; | ||
} else if (typeof options !== 'object') { | ||
throw new TypeError('Bad arguments'); | ||
throwOptionsError(options); | ||
} | ||
|
||
if (!options.flag) | ||
|
@@ -1192,7 +1194,7 @@ fs.appendFileSync = function(path, data, options) { | |
} else if (typeof options === 'string') { | ||
options = { encoding: options, mode: 0o666, flag: 'a' }; | ||
} else if (typeof options !== 'object') { | ||
throw new TypeError('Bad arguments'); | ||
throwOptionsError(options); | ||
} | ||
if (!options.flag) | ||
options = util._extend({ flag: 'a' }, options); | ||
|
@@ -1300,12 +1302,7 @@ StatWatcher.prototype.stop = function() { | |
}; | ||
|
||
|
||
var statWatchers = {}; | ||
function inStatWatchers(filename) { | ||
return Object.prototype.hasOwnProperty.call(statWatchers, filename) && | ||
statWatchers[filename]; | ||
} | ||
|
||
const statWatchers = new Map(); | ||
|
||
fs.watchFile = function(filename) { | ||
nullCheck(filename); | ||
|
@@ -1332,22 +1329,24 @@ fs.watchFile = function(filename) { | |
throw new Error('watchFile requires a listener function'); | ||
} | ||
|
||
if (inStatWatchers(filename)) { | ||
stat = statWatchers[filename]; | ||
} else { | ||
stat = statWatchers[filename] = new StatWatcher(); | ||
stat = statWatchers.get(filename); | ||
|
||
if (stat === undefined) { | ||
stat = new StatWatcher(); | ||
stat.start(filename, options.persistent, options.interval); | ||
statWatchers.set(filename, stat); | ||
} | ||
|
||
stat.addListener('change', listener); | ||
return stat; | ||
}; | ||
|
||
fs.unwatchFile = function(filename, listener) { | ||
nullCheck(filename); | ||
filename = pathModule.resolve(filename); | ||
if (!inStatWatchers(filename)) return; | ||
var stat = statWatchers.get(filename); | ||
|
||
var stat = statWatchers[filename]; | ||
if (stat === undefined) return; | ||
|
||
if (typeof listener === 'function') { | ||
stat.removeListener('change', listener); | ||
|
@@ -1357,7 +1356,7 @@ fs.unwatchFile = function(filename, listener) { | |
|
||
if (EventEmitter.listenerCount(stat, 'change') === 0) { | ||
stat.stop(); | ||
statWatchers[filename] = undefined; | ||
statWatchers.delete(filename); | ||
} | ||
}; | ||
|
||
|
@@ -1629,8 +1628,8 @@ function ReadStream(path, options) { | |
this.flags = options.flags === undefined ? 'r' : options.flags; | ||
this.mode = options.mode === undefined ? 0o666 : options.mode; | ||
|
||
this.start = options.start === undefined ? undefined : options.start; | ||
this.end = options.end === undefined ? undefined : options.end; | ||
this.start = options.start; | ||
this.end = options.end; | ||
this.autoClose = options.autoClose === undefined ? true : options.autoClose; | ||
this.pos = undefined; | ||
|
||
|
@@ -1792,7 +1791,7 @@ function WriteStream(path, options) { | |
this.flags = options.flags === undefined ? 'w' : options.flags; | ||
this.mode = options.mode === undefined ? 0o666 : options.mode; | ||
|
||
this.start = options.start === undefined ? undefined : options.start; | ||
this.start = options.start; | ||
this.pos = undefined; | ||
this.bytesWritten = 0; | ||
|
||
|
@@ -1879,8 +1878,11 @@ util.inherits(SyncWriteStream, Stream); | |
|
||
|
||
// Export | ||
fs.SyncWriteStream = SyncWriteStream; | ||
|
||
Object.defineProperty(fs, 'SyncWriteStream', { | ||
configurable: true, | ||
writable: true, | ||
value: SyncWriteStream | ||
}); | ||
|
||
SyncWriteStream.prototype.write = function(data, arg1, arg2) { | ||
var encoding, cb; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
glad there are others around to clean up my mess. thanks for catching it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trevnorris Got your back ;-)