Docs: fixes filehandle.read docs to show default buffer value length as 0

[rbrishabh]

Fixes: #39034

Hey @Linkgoron, @Ayase-252 - how does this look?
Let me know if this needs any changes! Thanks

[Linkgoron]

IMO there should no default values mentioned at all (for any of the paramaters), as the function arguments are not optional. This is true for both filehandle.read(buffer, offset, length, position) and fs.read(fd, buffer, offset, length, position, callback). For example, if length is not provided, the function doesn't actually read anything (as it is converted to 0), which is probably not what anyone meant.

[rbrishabh]

makes sense! I'll make the changes

[rbrishabh]

I've made the changes! thoughts @Linkgoron?

[rbrishabh]

my bad @Linkgoron, I should have been more careful. Done! :)

[lydell]

Is it worth mentioning that fileHandle.read differs compared to fs.readSync (no parameter defaults)? Seems like an easy mistake to make.

[rbrishabh]

Happy to make changes for any suggestions you have @lydell

[lydell]

My bad, fs.readSync isn’t documented to have parameter defaults either:

https://nodejs.org/api/fs.html#fs_fs_readsync_fd_buffer_offset_length_position

I think I’ve finally figured out what confused me: For fs.readSync, buffer is always the first argument. So fs.readSync(fd, buffer) triggers the fs.readSync(fd, buffer, [options]) overload, and options has defaults. But fileHandle.read() takes buffer inside the options object. So I can’t say filehandle.read(buffer) – it has to be filehandle.read({buffer}). Oh well.

I think this PR is correct – none of the 3 read variants take parameter defaults (only the overloads with options objects have defaults). And I don’t think any additional text is needed.

[rbrishabh]

Makes complete sense @lydell! Thanks for your review :)

[subson]

@rbrishabh - Can you amend the commit message to less than 72 characters, thats the check failing for this PR.

[rbrishabh]

Hi @subson! done! let me know if this needs any other changes! thx

[subson]

Hey @Linkgoron @lydell, can you approve this PR again to kick of the workflows as commit message has been updated?

[Linkgoron]

You need to change the subsystem to doc instead of Docs in the commit message.

[jasnell]

You need to change the subsystem to doc instead of Docs in the commit message.

FWIW, that can be done by whomever lands the PR really.

[subson]

@rbrishabh - Hey, Need to update commit again to doc: instead of Docs:.

[rbrishabh]

Sure! I can do that

[rbrishabh]

Hi @subson! done! let me know if this needs any other changes! thank you

[github-actions]

Landed in 7153d25...479658b

[lilit-harutyunyan]

🤒