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

Jump to bottom

Document that Display entails ToString and should be lossless and infallible #94488

Closed
wants to merge 2 commits into from
Closed

Document that Display entails ToString and should be lossless and infallible #94488

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
714bc47
Document that Display entails ToString and should be lossless and inf…
Mar 1, 2022
0dcec03
Merge branch 'rust-lang:master' into issue-92941-fix
JKAnderson409 Jun 18, 2022
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
14 changes: 12 additions & 2 deletions library/core/src/fmt/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,11 +703,21 @@ pub use macros::Debug;

/// Format trait for an empty format, `{}`.
///
/// `Display` is similar to [`Debug`], but `Display` is for user-facing
/// output, and so cannot be derived.
/// `Display` is similar to [`Debug`], but `Display` is for user-facing output,
/// and so cannot be derived. When `Display` is implemented, the [`ToString`]
Comment on lines +706 to +707
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// `Display` is similar to [`Debug`], but `Display` is for user-facing output,
/// and so cannot be derived. When `Display` is implemented, the [`ToString`]
/// `Display` is similar to [`Debug`], but `Display` is for user-facing output.
/// When `Display` is implemented, the [`ToString`]

As long as we're fixing this documentation: this conclusion doesn't actually automatically follow, and might not always be true.

/// trait is implemented automatically, adding the `to_string` method to all
/// `Display` types.
///
/// A `Display` implementation should be lossless and infallible, otherwise it
Copy link

Choose a reason for hiding this comment

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

How do you define lossless here? Any definition I can think of feels way too strict..

Copy link
Member

Choose a reason for hiding this comment

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

"doesn't lose information from its internal data" and then we can use Path as an example?

/// does not fit the API. For example, converting a path to a [`String`] is
/// potentially lossy or fallible, so [`Path`] doesn’t implement `Display`
/// directly (but offers a .display() method).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// directly (but offers a .display() method).
/// directly (but offers a `.display()` method).

///
/// For more information on formatters, see [the module-level documentation][module].
///
/// [`Path`]: ../../std/path/struct.Path.html
/// [`ToString`]: ../../std/string/trait.ToString.html
/// [`String`]: ../../std/string/struct.String.html
/// [module]: ../../std/fmt/index.html
///
/// # Examples
Expand Down