Adding more verbose documentation for std::fmt::Write
#100255
Conversation
rustbot
added
the
T-libs
|
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
library/core/src/fmt/mod.rs
Outdated
| @@ -119,6 +119,13 @@ pub trait Write { | |||
| /// | |||
| /// This function will return an instance of [`Error`] on error. | |||
| /// | |||
| /// Though it is possible for implementors of this trait to return an error, at the time | |||
| /// of writing these docs, no implementation of [`std::fmt::Write`] in the standard library | |||
| /// returns such an error. | |||
There was a problem hiding this comment.
This claim is false. The internal implementation of std::fmt::Write for an Adapter that wraps a std::io::Write produces a fmt::Error when the underlying destination returns a std::io::Error (while saving that error internally and returning it after the formatting is cancelled).
This can be observed in user code by implementing a formatting trait which is then used with io::Write::write_fmt() on a destination that errors.
I think better advice to give here is something like “The purpose of std::fmt::Error is to abort the formatting operation when the underlying destination encounters some error preventing it from accepting more text; it should generally be propagated rather than handled, at least when implementing formatting traits.”
There was a problem hiding this comment.
Oh, I missed that! Thanks for the heads up. I'll make the change with your suggestion, run the checks, and push the commit.
|
@kpreid did you need anything further here? |
Oh, sorry — I don't have any authority with the project and cannot approve your PR; @joshtriplett is your assigned reviewer. I only commented because the PR (and the issue) came to my attention and I noticed a problem. That said, if you want further input: I think it'd be better if all 3 methods of the trait had some kind of note rather than only |
|
I'm going to go ahead and merge this, but I do agree that the other methods could use a similar note. @bors r+ rollup |
bors
added
S-waiting-on-bors
…triplett Adding more verbose documentation for `std::fmt::Write` Attempts to address rust-lang#98861
Noted - I'll circle back on Monday and add in similar notes for other methods as well if that works for you? @kpreid sorry about that - didn't mean to drag you in! My mistake. But thanks for your help and feedback 😄 |
Rollup of 9 pull requests Successful merges: - rust-lang#100022 (Optimize thread ID generation) - rust-lang#100030 (cleanup code w/ pointers in std a little) - rust-lang#100229 (add -Zextra-const-ub-checks to enable more UB checking in const-eval) - rust-lang#100247 (Generalize trait object generic param check to aliases.) - rust-lang#100255 (Adding more verbose documentation for `std::fmt::Write`) - rust-lang#100366 (errors: don't fail on broken primary translations) - rust-lang#100396 (Suggest const and static for global variable) - rust-lang#100409 (rustdoc: don't generate DOM element for operator) - rust-lang#100443 (Add two let else regression tests) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Attempts to address #98861