Improve std::net docs #40838

chordowl · 2017-03-26T15:51:01Z

Fixes #29363

Summary:

Added a lot of missing links, both to other types/methods and to IETF RFCs, and changed occurences of just "RFC" to "IETF RFC"
Expanded a bunch of top-level docs, specifically the module docs & the docs for TcpListener, TcpStream, UdpSocket, IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6,
Expanded method docs for SocketAddrV6, AddrParseError,
Various edits for clarity, consistency, and accuracy

See the commit descriptions for more detail.

Things not done quite as laid out in the task list in #29363:

AddrParseError still doesn't have examples, but I wasn't quite sure how to do them; other FromStr error types don't have any, either
I didn't link to an IETF RFC in IpAddr, but in Ipv4Addr and Ipv6Addr and linked tho those from IpAddr; this seems more appropriate to me
Similarly, I didn't really exand SocketAddr's docs, but elaborated on SocketAddrV4 and SocketAddrV6's and linked to them from SocketAddr

Theres definitely still room for improvement, but this should be a good first effort 😄

part of #29363

Part of #29363

part of #29363

Part of #29363 In the section about the default implementations of ToSocketAddrs, I moved the bulletpoint of SocketAddrV4 & SocketAddrV6 to the one stating that SocketAddr is constructed trivially, as this is what's actually the case

Additionally changed the summary sentence to be more consistent with most of the other FromStr implementations' error types.

Part of #29363 Expanded top-level documentation & linked to relevant IETF RFCs. Added a bunch of links (to true/false/Ipv4Addr/etc.) throughout the docs.

Relative links in trait methods don't resolve in e.g. std/primitive.tuple.html :(

…AddrV6} docs Part of #29363 Changed summary sentences of SocketAddr and IpAddr for consistency Linked to SocketAddrV4 and SocketAddrV6 from SocketAddr, moving explaination there Expanded top-level docs for SocketAddrV4 and SocketAddrV6, linking to some relevant IETF RFCs, and linking back to SocketAddr Changed some of the method summaries to third person as per RFC 1574; added links to IETF RFCs where appropriate

…docs

Part of #29363

rust-highfive · 2017-03-26T15:51:14Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

chordowl · 2017-03-26T15:51:28Z

r? @steveklabnik
(oops)

GuillaumeGomez · 2017-03-26T16:49:04Z

src/libstd/net/addr.rs

 #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
 #[stable(feature = "rust1", since = "1.0.0")]
 pub enum SocketAddr {
-    /// An IPv4 socket address which is a (ip, port) combination.
+    /// An IPv4 socket address.


For those (like me) who aren't much into network, the original sentence made more sense.

I'll probably address this by mentioning this in SocketAddr's top-level docs again, since it's also (mostly) true for the V6 variant (unless someone has a better solution)

I still think it's fine; it's a summary line, not a full explanation.

GuillaumeGomez · 2017-03-26T16:49:44Z

src/libstd/net/addr.rs

    #[stable(feature = "rust1", since = "1.0.0")]
    V4(#[stable(feature = "rust1", since = "1.0.0")] SocketAddrV4),
    /// An IPv6 socket address.
    #[stable(feature = "rust1", since = "1.0.0")]
    V6(#[stable(feature = "rust1", since = "1.0.0")] SocketAddrV6),
 }

-/// An IPv4 socket address which is a (ip, port) combination.
+/// An IPv4 socket address.


I like this small introduction better but I think the removed part should be put somewhere under.

It being a combination of IP address and port is already stated in the next sentence. Should I make that more explicit?

GuillaumeGomez · 2017-03-26T16:52:28Z

src/libstd/net/addr.rs

-    /// Returns true if the IP in this `SocketAddr` is a valid IPv4 address,
-    /// false if it's a valid IPv6 address.
+    /// Returns [`true`] if the [IP address] in this `SocketAddr` is an
+    /// [IPv4 address] and [`false`] if it's an [IPv6 address].


I wonder if "or false" wouldn't be better than "and false". Anyone has a thought on this?

I'd put a comma before the "and" (but keep the "and")

One formulation I thought about was , `false` otherwise, maybe that's better?

GuillaumeGomez · 2017-03-26T16:52:46Z

src/libstd/net/addr.rs

-    /// Returns true if the IP in this `SocketAddr` is a valid IPv6 address,
-    /// false if it's a valid IPv4 address.
+    /// Returns [`true`] if the [IP address] in this `SocketAddr` is an
+    /// [IPv6 address] and [`false`] if it's an [IPv4 address].


Same comment as above.

GuillaumeGomez · 2017-03-26T16:55:41Z

Except for the few (very) little nits I mentionned, it's awesome!

killercup

Very nice.

killercup · 2017-03-26T17:09:02Z

src/libstd/net/addr.rs

-    /// Returns true if the IP in this `SocketAddr` is a valid IPv4 address,
-    /// false if it's a valid IPv6 address.
+    /// Returns [`true`] if the [IP address] in this `SocketAddr` is an
+    /// [IPv4 address] and [`false`] if it's an [IPv6 address].


I'd put a comma before the "and" (but keep the "and")

killercup · 2017-03-26T17:11:37Z

src/libstd/net/addr.rs

-    /// corresponding to the `sin6_flowinfo` field in C.
+    /// Returns the flow information associated with this address.
+    ///
+    /// This information corresponds to the `sin6_flowinfo` field in C, as specified in


"in C" → "in C's netinet/in.h"

killercup · 2017-03-26T17:15:46Z

src/libstd/net/addr.rs

@@ -632,7 +746,9 @@ pub trait ToSocketAddrs {
    ///
    /// # Errors
    ///
-    /// Any errors encountered during resolution will be returned as an `Err`.
+    /// Any errors encountered during resolution will be returned as an [`Err`].


This is obvious from the function signature with Result that it returns an Err on error… Or you would have added a "Panics" section instead 😉 Maybe instead mention which Err variants it will return? Apparently one/some of std::io::Error?

I looked around and it seems like the std::io::ErrorKinds that might (commonly) come in the provided implementations are InvalidInput, PermissionDenied, and NotFound.
I will add this instance with this PR, but this is kind of an issue in general throughout the docs (#40322) and out of scope for what I wanted to achieve with this PR. Thanks for the pointer tho, this does sound rather silly!
(Sidenote: I will have to test this myself, but it seems like from Windows' decode_error_kind implementation that this might return an io:Error with ErrorKind::Other more often then it should/could on Windows??)

So yes, agreed with both: this does feel a little redundant right now, but also, we do want to start describing which errors actually can happen.

You could also just remove this section for the purposes of this PR.

chordowl

Noticed some bona fide errors here and there, will correct them later

chordowl · 2017-03-26T18:01:25Z

src/libstd/net/mod.rs

+//! [`SocketAddrV6`]: ../../std/net/struct.SocketAddrV6.html
+//! [`TcpListener`]: ../../std/net/struct.TcpListener.html
+//! [`TcpStream`]: ../../std/net/struct.TcpStream.html
+//! [`ToSocketAddr`]: ../../std/net/trait.ToSocketAddr.html


should be ../../std/net/trait.ToSocketAddrs.html (cought by linkchecker)

chordowl · 2017-03-26T18:01:56Z

src/libstd/net/mod.rs

+//!   [`Ipv6Addr`] are respectively IPv4 and IPv6 addresses
+//! * [`SocketAddr`] represents socket addresses of either IPv4 or IPv6; [`SocketAddrV4`]
+//!   and [`SocketAddrV6`] are respectively IPv4 and IPv6 socket addresses
+//! * [`ToSocketAddr`] is a trait that used for generic address resolution interacting


missing a "when" before "interacting"

ollie27 · 2017-03-26T19:37:55Z

src/libstd/net/ip.rs

+///
+/// # Examples
+///
+/// ```


This example is missing the closing ```.

steveklabnik

Wonderful PR. Thanks so much!

steveklabnik · 2017-03-27T13:57:34Z

src/libstd/net/addr.rs

@@ -632,7 +746,9 @@ pub trait ToSocketAddrs {
    ///
    /// # Errors
    ///
-    /// Any errors encountered during resolution will be returned as an `Err`.
+    /// Any errors encountered during resolution will be returned as an [`Err`].


So yes, agreed with both: this does feel a little redundant right now, but also, we do want to start describing which errors actually can happen.

You could also just remove this section for the purposes of this PR.

* Fixed spelling ToSocketAddr -> ToSocketAddrs in module docs (which also fixes a link) * Added missing "when" before "interacting" in module docs * Changed SocketAddr's top-level docs to explicitly state what socket addresses consist of, making them more consistent with SocketAddrV4's and SocketAddrV6's docs * Changed "in C" -> "in C's `netinet/in.h`" * Changed wording in is_ipv4/is_ipv6 methods to ", `false` otherwise" * Add missing closing ` ``` ` in Ipv6Addr's examples * Removed "Errors" section in ToSocketAddrs' to_socket_addrs method as it was rather redundant

chordowl · 2017-03-27T14:45:28Z

Addressed requested changes; see commit description for more details.

steveklabnik · 2017-03-27T16:56:03Z

Looks great to me!

GuillaumeGomez · 2017-03-28T02:53:01Z

Since everyone approved, I'll start the merge process. Don't hesitate to r- if there is anything wrong.
Thanks again @lukaramu!

@bors: r+ rollup

bors · 2017-03-28T02:53:02Z

📌 Commit b8cbc5d has been approved by GuillaumeGomez

…omez Improve std::net docs Fixes rust-lang#29363 Summary: * Added a _lot_ of missing links, both to other types/methods and to IETF RFCs, and changed occurences of just "RFC" to "IETF RFC" * Expanded a bunch of top-level docs, specifically the module docs & the docs for `TcpListener`, `TcpStream`, `UdpSocket`, `IpAddr`, `Ipv4Addr`, `Ipv6Addr`, `SocketAddr`, `SocketAddrV4`, `SocketAddrV6`, * Expanded method docs for `SocketAddrV6`, `AddrParseError`, * Various edits for clarity, consistency, and accuracy See the commit descriptions for more detail. Things not done quite as laid out in the task list in rust-lang#29363: * `AddrParseError` still doesn't have examples, but I wasn't quite sure how to do them; other `FromStr` error types don't have any, either * I didn't link to an IETF RFC in `IpAddr`, but in `Ipv4Addr` and `Ipv6Addr` and linked tho those from `IpAddr`; this seems more appropriate to me * Similarly, I didn't really exand `SocketAddr`'s docs, but elaborated on `SocketAddrV4` and `SocketAddrV6`'s and linked to them from `SocketAddr` Theres definitely still room for improvement, but this should be a good first effort 😄

Rollup of 5 pull requests - Successful merges: #40682, #40731, #40783, #40838, #40864 - Failed merges:

chordowl added 12 commits March 26, 2017 14:35

added missing links in std::net TCP docs

169facf

part of #29363

Update std::net:Incoming's docs to use standard iterator boilerplate

76d08ed

Part of #29363

std::net docs: changed occurences of "RFC" to say "IETF RFC"

0df7398

part of #29363

Added links to std::net::AddrParseError's documentation

0d5baba

Additionally changed the summary sentence to be more consistent with most of the other FromStr implementations' error types.

Expanded and added links to std::net::{IpAddr,Ipv4Addr,Ipv6Addr} docs

347b709

Part of #29363 Expanded top-level documentation & linked to relevant IETF RFCs. Added a bunch of links (to true/false/Ipv4Addr/etc.) throughout the docs.

Removed link in std::net::ToSocketAddr's summary sentence

be713fa

Relative links in trait methods don't resolve in e.g. std/primitive.tuple.html :(

Added examples to std::net::{SocketAddr, SocketAddrV4, SocketAddrV6} …

1a9c8ba

…docs

Added links to std::net::Shutdown docs and made them more consistent

ad816f8

Part of #29363

Expanded top-level docs for std::net{TcpListener,TcpStream,UdpSocket}

597bcec

Part of #29363

Expanded std::net module docs

577677d

Part of #29363

rust-highfive assigned alexcrichton Mar 26, 2017

rust-highfive assigned steveklabnik and unassigned alexcrichton Mar 26, 2017

fix trailing whitespace

c2601fd

GuillaumeGomez requested changes Mar 26, 2017

View reviewed changes

killercup approved these changes Mar 26, 2017

View reviewed changes

chordowl commented Mar 26, 2017

View reviewed changes

ollie27 reviewed Mar 26, 2017

View reviewed changes

src/libstd/net/ip.rs

///

/// # Examples

///

/// ```

Copy link

Member

ollie27 Mar 26, 2017

Choose a reason for hiding this comment

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

This example is missing the closing ```.

steveklabnik reviewed Mar 27, 2017

View reviewed changes

GuillaumeGomez approved these changes Mar 28, 2017

View reviewed changes

frewsxcv mentioned this pull request Mar 29, 2017

Rollup of 8 pull requests #40884

Closed

frewsxcv mentioned this pull request Mar 29, 2017

Rollup of 8 pull requests #40886

Closed

frewsxcv mentioned this pull request Mar 29, 2017

Rollup of 5 pull requests #40889

Merged

bors added a commit that referenced this pull request Mar 29, 2017

Auto merge of #40889 - frewsxcv:rollup, r=frewsxcv

4465f22

Rollup of 5 pull requests - Successful merges: #40682, #40731, #40783, #40838, #40864 - Failed merges:

bors merged commit b8cbc5d into rust-lang:master Mar 29, 2017

chordowl deleted the std-net-docs branch March 29, 2017 10:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve std::net docs #40838

Improve std::net docs #40838

chordowl commented Mar 26, 2017

rust-highfive commented Mar 26, 2017

chordowl commented Mar 26, 2017

GuillaumeGomez Mar 26, 2017

chordowl Mar 26, 2017

steveklabnik Mar 27, 2017

GuillaumeGomez Mar 26, 2017

chordowl Mar 26, 2017

GuillaumeGomez Mar 26, 2017

killercup Mar 26, 2017

chordowl Mar 26, 2017

GuillaumeGomez Mar 26, 2017

GuillaumeGomez commented Mar 26, 2017

killercup left a comment

killercup Mar 26, 2017

killercup Mar 26, 2017

killercup Mar 26, 2017

chordowl Mar 26, 2017

steveklabnik Mar 27, 2017

chordowl left a comment

chordowl Mar 26, 2017

chordowl Mar 26, 2017

ollie27 Mar 26, 2017

steveklabnik left a comment

steveklabnik Mar 27, 2017

chordowl commented Mar 27, 2017

steveklabnik commented Mar 27, 2017

GuillaumeGomez commented Mar 28, 2017

bors commented Mar 28, 2017

Improve std::net docs #40838

Improve std::net docs #40838

Conversation

chordowl commented Mar 26, 2017

rust-highfive commented Mar 26, 2017

chordowl commented Mar 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Mar 26, 2017

killercup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chordowl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steveklabnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chordowl commented Mar 27, 2017

steveklabnik commented Mar 27, 2017

GuillaumeGomez commented Mar 28, 2017

bors commented Mar 28, 2017