-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
feat: make fs.read params optional #31402
Changes from 10 commits
95f03ef
478d2f0
10d3902
dfdd3b4
cf320a0
418b6fd
e3835c2
128aeab
2c5d3e7
6433727
3923975
f6f01b6
55a2356
f86bdc1
a4c3763
e321b88
cb8c0ef
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 |
---|---|---|
|
@@ -2760,6 +2760,26 @@ The callback is given the three arguments, `(err, bytesRead, buffer)`. | |
If this method is invoked as its [`util.promisify()`][]ed version, it returns | ||
a `Promise` for an `Object` with `bytesRead` and `buffer` properties. | ||
|
||
## `fs.read(fd, options, callback)` | ||
<!-- YAML | ||
added: REPLACEME | ||
changes: | ||
- version: REPLACEME | ||
pr-url: https://github.com/nodejs/node/pull/31402 | ||
description: Options object can be passed in | ||
to make Buffer, offset, lenght and position optional | ||
--> | ||
* `fd` {integer} | ||
* `options` {Object} | ||
* `buffer` {Buffer|TypedArray|DataView} **Default:** `Buffer.alloc(16384)` | ||
* `offset` {integer} **Default:** `0` | ||
* `length` {integer} **Default:** `buffer.length` | ||
* `position` {integer} **Default:** `null` | ||
* `callback` {Function} | ||
* `err` {Error} | ||
* `bytesRead` {integer} | ||
* `buffer` {Buffer} | ||
|
||
## `fs.readdir(path[, options], callback)` | ||
<!-- YAML | ||
added: v0.1.8 | ||
|
@@ -4342,6 +4362,17 @@ Following successful read, the `Promise` is resolved with an object with a | |
`bytesRead` property specifying the number of bytes read, and a `buffer` | ||
property that is a reference to the passed in `buffer` argument. | ||
|
||
#### `filehandle.read(options)` | ||
<!-- YAML | ||
added: REPLACEME | ||
--> | ||
* `options` {Object} | ||
* `buffer` {Buffer|Uint8Array} | ||
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. I assume the defaults here are the same as the 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. Also, I might be missing it but is the Promise version tested? 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. i don't think you are missing anything. i think i forgot to add a test for the promise based call 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.
Right, added them and a test for the promise in the latest 2 commits Also, I noticed that the buffer types for the |
||
* `offset` {integer} | ||
* `length` {integer} | ||
* `position` {integer} | ||
* Returns: {Promise} | ||
ronag marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
#### `filehandle.readFile(options)` | ||
<!-- YAML | ||
added: v10.0.0 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
'use strict'; | ||
|
||
const common = require('../common'); | ||
const fixtures = require('../common/fixtures'); | ||
const fs = require('fs'); | ||
const assert = require('assert'); | ||
const filepath = fixtures.path('x.txt'); | ||
const fd = fs.openSync(filepath, 'r'); | ||
|
||
const expected = Buffer.from('xyz\n'); | ||
const defaultBufferAsync = Buffer.alloc(16384); | ||
|
||
// Optional buffer, offset, length, position | ||
// fs.read(fd, callback); | ||
fs.read(fd, {}, common.mustCall((err, bytesRead, buffer) => { | ||
assert.strictEqual(bytesRead, expected.length); | ||
assert.deepStrictEqual(defaultBufferAsync.length, buffer.length); | ||
})); | ||
ronag marked this conversation as resolved.
Show resolved
Hide resolved
|
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.
same here