Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 Read trait blocking behavior #625
Clarify Read trait blocking behavior #625
Changes from 1 commit
9b1e754
45fcad0
92dbaf2
f3558ac
d745418
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
minor nit: this line looks like it applies only to the "if there are bytes available to read" case, while it should apply to both. Perhaps split it to a new paragraph and remove the
-
.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.
Hmmm, but it is applicable only to the "if there are bytes available to read" case, as it talks about - if there are bytes available to read in the first place - how many of those would be returned - i.e. not necessarily all of them (because (a) the buffer might actually be shorter than what is available (b) because the implementation just decides so, for whatever other reasons).
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.
This applies to the first case as well, doesn't it? In "Once at least one (or more) bytes become available", one could also ask if it's guaranteed that all available bytes are returned. The answer is "it is not guaranteed", but it's not completely obvious from the description as it is written now.
But as Dirbaio wrote, it is a "minor nit". Personally, I would not expect to have such a guarantee when reading the text you proposed.
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.
Not sure how come the first case is unclear that only a subset of the just-read bytes are returned, given that it says (emphasis mine):
"
buf
, and the amount is returned..."
"a non-zero amount of those". Do we really need to be more explicit than that?
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.
Ah be it. If two folks find it unclear then it is probably unclear. :(
I'll re-arrange so that the problematic text is a separate paragraph.
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.
@jannic I've re-arranged it now so that the problematic text is a separate paragraph. If you can take a quick look - thanks.
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.
IMHO the only possible source of confusion was the asymmetry between the two cases. Yes, in general I agree that the wording "a non-zero amount of those" was sufficiently clear. But then the second case, in addition to that, explicitly stated that it's allowed to return less than available. For the first case, that sentence was missing. Readers may wonder why? Is there a difference in guarantees?
Again, this was a very minor detail. But if we can avoid this possible confusion by just putting the last sentence in a separate paragraph, as you now did, we should do it.
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.
Argh, don't want to over-complicate it, but now - with the problematic text being a separate paragraph - the next paragraph that starts with "This waiting behavior is important for the cases where
Read
..." becomes unclear... in that readers might be confused that it only refers to the newly-born "Note..." paragraph, while it refers to everything mentioned above.Any brilliant ideas? Or is it just my mind that finds the new arrangement also problematic?