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

clarify the position argument of fs.read #14610

Closed
wants to merge 1 commit into from
Closed

clarify the position argument of fs.read #14610

wants to merge 1 commit into from

Conversation

dcharbonnier
Copy link
Contributor

@dcharbonnier dcharbonnier commented Aug 3, 2017

What happen to the file position after a read using a position null or integer was not clear and you can assume that the pointer to the file position is updated even if position is an integer. close #8397

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

What happen to the file position after a read using a position null or integer was not clear and you can assume that the pointer to the file position is updated even if position is an integer. close #8397
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. labels Aug 3, 2017
@@ -789,6 +789,7 @@ const defaults = {
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.
.
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems this line is unneeded.

@@ -1639,8 +1640,8 @@ Read data from the file specified by `fd`.

`length` is an integer specifying the number of bytes to read.

`position` is an integer specifying where to begin reading from in the file.
If `position` is `null`, data will be read from the current file position.
`position` is an integer specifying where to begin reading from in the file, in this case the current file position is not updated.
Copy link
Contributor

Choose a reason for hiding this comment

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

Style guide advises wrapping lines at 80 characters.

@dcharbonnier dcharbonnier deleted the patch-4 branch August 3, 2017 16:45
@dcharbonnier
Copy link
Contributor Author

yes, I agree with you comments, but I just saw what happen to my last PR so I don't think it worse the effort to try to help on this project, keep going like that guys

@refack refack self-assigned this Aug 3, 2017
@refack
Copy link
Contributor

refack commented Aug 3, 2017

@dcharbonnier don't give up on this project just because of some pushback. Documentation changes seem like they should be easy, but because we can't "test" them people give them extra attention and make a lot of comments.

Give this PR a second chance, it's a good improvement and IMHO it will land.

@vsemozhetbyt
Copy link
Contributor

@dcharbonnier I am very sorry if my comments were abrupt or inappropriate for you. I am grateful for your effort and time as all the other collaborators. Excuse me for the tone or other possible flaws.

@dcharbonnier
Copy link
Contributor Author

@vsemozhetbyt this PR is ok ! Your comments are perfectly justified and I should read before sending this, I'm wasting your time with my mistakes.
I was referring to an other PR with an attitude that was for me inappropriate. But on this PR I'm 100% wrong and you are right @vsemozhetbyt :-)

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Aug 3, 2017

@dcharbonnier I am still sorry I was not more sympathetic and less dry with my nit comments and did not express gratitude before those small requests. I hope you will not infer any bad conclusions about Node.js community from my personal indiscretion.

@refack refack removed their assignment Oct 20, 2018
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fs.read at position 0 followed by position null reads wrong data
4 participants