doc: reword ambigous docs for socket.setNoDelay#7995
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nodejs-github-bot added doc 
doc/api/net.md Outdated
There was a problem hiding this comment.
This breaks the links, I presume.
The following line should also be changed:
doc/api/http.md:1524:[`socket.setNoDelay()`]: net.html#net_socket_setnodelay_nodelay There was a problem hiding this comment.
That said, I'm not sure if it really makes sense to rename the argument to enable.
Thoughs?
There was a problem hiding this comment.
it is too ambiguous otherwise and parameter name in code is enable: https://github.com/nodejs/node/blob/master/lib/net.js#L337
There was a problem hiding this comment.
This doesn't help, we used to have two concepts, "no delay" and "true/false", and we could tell that "nodelay" (the param name and method name) could be set to true.
Now, you enable (something?) and when something is enabled, "no delay" is... what? true? false? So I have to map enable to true, and true to "no delay" being on, which is a hop too many.
There was a problem hiding this comment.
or maybe when "enable" is false, nagle is disabled... so "no delay" is true? In which case, the meaning of true/false to this function would be inverted, so that's an API change, and we would never do it.
| connections use the Nagle algorithm, they buffer data before sending it off. | ||
| Setting `true` for `noDelay` will immediately fire off data each time | ||
| `socket.write()` is called. | ||
| `enable` defaults to `true`. |
There was a problem hiding this comment.
Another way this could be reworded is:
By default, TCP connections use the Nagle algorithm to buffer data before sending. Calling `socket.setNoDelay(true)` disables such buffering so that data is sent each time `socket.write()` is called. The `enable` argument defaults to `true`. 
silverwind commented Sep 2, 2016
Also to note, the assumption |
rvaggforce-pushed the master branch 2 times, most recently from c133999 to 83c7a88Compare
jasnell commented Mar 1, 2017
Still want to pursue this? |
Xerkus commented Mar 1, 2017
I am not a native speaker, and there were objections to the change. I am not sure if i was wrong here. |
jasnell commented Mar 1, 2017
@Xerkus ... no problem. I'll keep it open and will take a look to see what else we can do here. |
fhinkel commented May 26, 2017
@silverwind and @sam-github can you make suggestions how you'd rather have the documentation worded so we can change it and land it? Thanks. |
We did make suggestions, didn't we? EDIT: yes, there were specific suggestions made, including text by @jasnell, that the OP has not followed up on. |
jasnell commented Aug 24, 2017
Closing due to lack of forward progress on this. It's likely still a change that should be made tho. |
chrisbutler commented Feb 1, 2018
please please please can we fix this madness? i'd be happy to take a stab at it |
8 participants
Checklist
Affected core subsystem(s)
doc
Description of change
Reworded description of
socket.setNoDelayto make it clear that function parameter defaults to true and not thenoDelayoption itself.See nodejs/node-v0.x-archive#8929