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.

Sign up for GitHub

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

Jump to bottom

doc: fix filehandle.truncate sample codes #20913

Closed
wants to merge 4 commits into from
Closed

doc: fix filehandle.truncate sample codes #20913

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
32107d0
doc: fix filehandle.truncate sample codes
May 23, 2018
89fc020
doc: fix filehandle.truncate sample codes to call filehandle.close
May 23, 2018
dda739e
doc: remove unneeded empty line in fs.md
May 24, 2018
01a67c1
doc: fix sample codes for filehandle.truncate
May 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions doc/api/fs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3572,8 +3572,8 @@ console.log(fs.readFileSync('temp.txt', 'utf8'));
// Prints: Node.js

async function doTruncate() {
const fd = await fsPromises.open('temp.txt', 'r+');
await fsPromises.ftruncate(fd, 4);
const filehandle = await fsPromises.open('temp.txt', 'r+');
await filehandle.truncate(4);
console.log(fs.readFileSync('temp.txt', 'utf8')); // Prints: Node
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should really be....

let filehandle;
try {
  const filehandle = await fsPromises.open('temp.txt', 'r+');
  await filehandle.truncate(4);
} finally {
  await filehandle.close();
}

While the filehandle will close automatically on garbage collection in order to prevent the leak, it should be closed manually. A process warning would be emitted if it is allowed to close on gc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell sorry for the tangent: we really need a better resources story in general - if this is tricky consider how hard this is with 3 concurrent handles.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that might work here API wise is a disposer:

await fsPromises.open('temp.txt', 'r+', async handle => {
  await handle.truncate(4);
});

This doesn't solve the harder problem of dealing with multiple resources at once though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I certainly don't disagree :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell
Thank you for your comment. The codes were fixed with filehandle.close. Please check them again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell - what do you think the right avenue to discuss such a cross-cutting concern is? I'm not sure what the right team/working-group/avenue is.

I'm a little lost here :)

}

Expand All @@ -3591,12 +3591,13 @@ console.log(fs.readFileSync('temp.txt', 'utf8'));
// Prints: Node.js

async function doTruncate() {
const fd = await fsPromises.open('temp.txt', 'r+');
await fsPromises.ftruncate(fd, 10);
const filehandle = await fsPromises.open('temp.txt', 'r+');
await filehandle.truncate(10);
console.log(fs.readFileSync('temp.txt', 'utf8')); // Prints Node.js\0\0\0
}

doTruncate().catch(console.error);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: unneeded empty line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vsemozhetbyt Fixed it. Thanks.

```

The last three bytes are null bytes (`'\0'`), to compensate the over-truncation.
Expand Down