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

doc: reword ambigous docs for socket.setNoDelay #7995

Closed
wants to merge 1 commit into from
Closed

doc: reword ambigous docs for socket.setNoDelay #7995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion doc/api/http.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1521,7 +1521,7 @@ There are a few special headers that should be noted.
[`response.writeContinue()`]: #http_response_writecontinue
[`response.writeHead()`]: #http_response_writehead_statuscode_statusmessage_headers
[`socket.setKeepAlive()`]: net.html#net_socket_setkeepalive_enable_initialdelay
[`socket.setNoDelay()`]: net.html#net_socket_setnodelay_nodelay
[`socket.setNoDelay()`]: net.html#net_socket_setnodelay_enable
[`socket.setTimeout()`]: net.html#net_socket_settimeout_timeout_callback
[`TypeError`]: errors.html#errors_class_typeerror
[`url.parse()`]: url.html#url_url_parse_urlstring_parsequerystring_slashesdenotehost
Expand Down
11 changes: 6 additions & 5 deletions doc/api/net.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,15 +625,16 @@ initialDelay will leave the value unchanged from the default

Returns `socket`.

### socket.setNoDelay([noDelay])
### socket.setNoDelay([enable])
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

That said, I'm not sure if it really makes sense to rename the argument to enable.

Thoughs?

Copy link
Author

Choose a reason for hiding this comment

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

it is too ambiguous otherwise and parameter name in code is enable: https://github.com/nodejs/node/blob/master/lib/net.js#L337

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@sam-github sam-github Dec 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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.

<!-- YAML
added: v0.1.90
-->

Disables the Nagle algorithm. By default TCP 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.
`noDelay` defaults to `true`.
Sets `noDelay` option, which disables the Nagle algorithm. By default TCP
connections use the Nagle algorithm, they buffer data before sending it off.
Setting `true` for `noDelay` will immediately fire off data each time
Copy link
Member

Choose a reason for hiding this comment

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

Note that it's still called noDelay here.

Copy link
Author

@Xerkus Xerkus Aug 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

i understand it as noDelay socket option as compared to noDelay parameter

`socket.write()` is called.
`enable` defaults to `true`.
Copy link
Member

Choose a reason for hiding this comment

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

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`.


Returns `socket`.

Expand Down