Skip to content
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

Omission in the fs doc #7099

Closed
vsemozhetbyt opened this issue Jun 2, 2016 · 4 comments
Closed

Omission in the fs doc #7099

vsemozhetbyt opened this issue Jun 2, 2016 · 4 comments
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.

Comments

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Jun 2, 2016

  • Version: 6.2.0
  • Platform: Windows 7
  • Subsystem: doc, fs

Sometimes I use the same file descriptor for writing and then reading by readline (possible case: write some data line by line, then reread it into an array to sort and rewrite).

It seems fs doc lacks for some point concerning these cases.

fs.createReadStream(path[, options]):

options can include start and end values to read a range of bytes from the file instead of the entire file. Both start and end are inclusive and start at 0.

Maybe it should be mentioned that start is set by default to the file end (or to the write position) for fd that has been previously written into.

Test:

const fs = require('fs');
const rl = require('readline');

const file = fs.openSync('test.txt', 'w+');

fs.writeSync(file, '1\n2\n3\n');

rl.createInterface({
  input: fs.createReadStream(null, {fd: file})
}).on('line', line => {
  console.log(line);
}).on('close', () => {
  console.log('Closed');
});

The output is only

Closed
const fs = require('fs');
const rl = require('readline');

const file = fs.openSync('test.txt', 'w+');

fs.writeSync(file, '1\n2\n3\n');

rl.createInterface({
  input: fs.createReadStream(null, {fd: file, start: 0})
}).on('line', line => {
  console.log(line);
}).on('close', () => {
  console.log('Closed');
});

The output contains all the file:

1
2
3
Closed
@Fishrock123 Fishrock123 added doc Issues and PRs related to the documentations. arm Issues and PRs related to the ARM platform. labels Jun 2, 2016
@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jun 2, 2016

Is arm label added by mistake?

@addaleax addaleax added fs Issues and PRs related to the fs subsystem / file system. and removed arm Issues and PRs related to the ARM platform. labels Jun 2, 2016
@bnoordhuis
Copy link
Member

Both start and end are inclusive and start at 0.

I think that should be understood as, when set, they start counting from zero, not one.

Maybe it should be mentioned that start is set by default to the file end (or to the write position) for fd that has been previously written into.

I agree. @nodejs/documentation: when fd is specified and start is omitted or undefined, fs.createReadStream() reads sequentially from the current file position. start corresponds to the position argument to fs.read(), which is documented to behave that way.

Mixing sequential reads and writes with concurrent asynchronous file operations is usually unwise because there is no telling in which order they run. Not an issue in the OP's example but replacing fs.writeSync() with fs.write(), for example, would introduce randomness.

@jasnell jasnell added the good first issue Issues that are suitable for first-time contributors. label Sep 9, 2016
@WesTyler
Copy link
Contributor

WesTyler commented Dec 1, 2016

@bnoordhuis / @nodejs/documentation: before I commit/PR, can I get feedback on wording updates? How's this?

Updated wording below:

options can include start and end values to read a range of bytes from
the file instead of the entire file. Both start and end are inclusive and
start counting at 0. If fd is specified and start is omitted or undefined,
fs.createReadStream() reads sequentially from the current file position.
The encoding can be any one of those accepted by [Buffer][].

@bnoordhuis
Copy link
Member

@WesTyler Sounds good to me.

@sam-github sam-github added doc Issues and PRs related to the documentations. and removed doc Issues and PRs related to the documentations. docs-requested labels Dec 1, 2016
addaleax pushed a commit that referenced this issue Dec 5, 2016
* start/end start *counting* at 0
* If fd is specified and start is omitted or undefined,
  fs.createReadStream() reads sequentially from the current file
  position.

Fixes: #7099
PR-URL: #10078
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
jmdarling pushed a commit to jmdarling/node that referenced this issue Dec 8, 2016
* start/end start *counting* at 0
* If fd is specified and start is omitted or undefined,
  fs.createReadStream() reads sequentially from the current file
  position.

Fixes: nodejs#7099
PR-URL: nodejs#10078
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
MylesBorins pushed a commit that referenced this issue Dec 20, 2016
* start/end start *counting* at 0
* If fd is specified and start is omitted or undefined,
  fs.createReadStream() reads sequentially from the current file
  position.

Fixes: #7099
PR-URL: #10078
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
MylesBorins pushed a commit that referenced this issue Dec 20, 2016
* start/end start *counting* at 0
* If fd is specified and start is omitted or undefined,
  fs.createReadStream() reads sequentially from the current file
  position.

Fixes: #7099
PR-URL: #10078
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
MylesBorins pushed a commit that referenced this issue Dec 21, 2016
* start/end start *counting* at 0
* If fd is specified and start is omitted or undefined,
  fs.createReadStream() reads sequentially from the current file
  position.

Fixes: #7099
PR-URL: #10078
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
MylesBorins pushed a commit that referenced this issue Dec 21, 2016
* start/end start *counting* at 0
* If fd is specified and start is omitted or undefined,
  fs.createReadStream() reads sequentially from the current file
  position.

Fixes: #7099
PR-URL: #10078
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

No branches or pull requests

7 participants