From 07e61cdc514234beeacc6d954e204b8d31e81c34 Mon Sep 17 00:00:00 2001 From: boats Date: Wed, 11 Apr 2018 02:52:03 -0700 Subject: [PATCH 1/3] Deprecate `Path::display`, implement `Display` for `Path` In [RFC #474][rfc474], the issue of how to handle Displaying a Path was left as an open question. The problem is that a Path may contain non-UTF-8 data on most platforms. In the implementation of the RFC, a `display` method was added, which returns an adapter that implements `Display` by replacing non-UTF8 data with a unicode replacement character. Though I can't find a record of the discussion around this issue, I believe there were two primary reasons not to just implement this behavior as the `Display` impl of `Path`: 1. The adapter allocated in the non-UTF8 case, and Rust as a rule tries to avoid allocations that are not explicit in code. 2. The user may prefer an alternative solution than using the unicode replacement character for handling non-UTF8 data. In my view, the choice to provide an adapter rather than implement Display has had a high cost in terms of user experience: * I almost never remember that I need an adapter, forcing me to go back and edit my code after compiling it and getting an error. * It makes my code more noisy to have the display adapter; this detail is rarely important. * It is extremely uncommon to actually do something other than call the display adapter when trying to display a path (I have never wanted anything else myself). * For new users, it is Yet Another Compiler Error that they have to figure out how to solve, contributing to the sense that Rust nags nags and obstructs rather than assists & guides. Therefore, I think time has shown that this has been a detriment to user experience, rather than a helpful reminder. That leaves only the first reason not to implement this: implicit allocations. That problem was happily resolved in June 2017: #42613 provided an alternative implementation which efficiently avoids allocations. Given that, I think it is time that we implement `Display` for both `Path` and `PathBuf` and deprecate the `Path::display` method. r? @alexcrichton cc @rust-lang/libs [rfc474]: https://github.com/rust-lang/rfcs/blob/master/text/0474-path-reform.md) --- src/libstd/path.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/libstd/path.rs b/src/libstd/path.rs index ec96157547383..b6216560d538f 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -1493,6 +1493,13 @@ impl fmt::Debug for PathBuf { } } +#[stable(feature = "rust1", since = "1.27.0")] +impl fmt::Display for PathBuf { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + fmt::Display::fmt(&Display { path: &**self }, formatter) + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl ops::Deref for PathBuf { type Target = Path; @@ -2250,6 +2257,10 @@ impl Path { /// Returns an object that implements [`Display`] for safely printing paths /// that may contain non-Unicode data. /// + /// This method has been deprecated since version 1.27.0, because paths now + /// implement [`Display`] directly, with the same outcome as using this + /// adapter. + /// /// [`Display`]: ../fmt/trait.Display.html /// /// # Examples @@ -2262,6 +2273,7 @@ impl Path { /// println!("{}", path.display()); /// ``` #[stable(feature = "rust1", since = "1.0.0")] + #[rustc_deprecated(since = "1.27.0", reason = "Path and PathBuf implement Display directly")] pub fn display(&self) -> Display { Display { path: self } } @@ -2480,6 +2492,13 @@ impl AsRef for Path { } } +#[stable(feature = "rust1", since = "1.27.0")] +impl fmt::Display for Path { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + fmt::Display::fmt(&Display { path: self }, formatter) + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl fmt::Debug for Path { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { From a2044876982040886794d62717cad2a1b75f9bdf Mon Sep 17 00:00:00 2001 From: boats Date: Wed, 11 Apr 2018 06:02:27 -0700 Subject: [PATCH 2/3] Remove deprecation. --- src/libstd/path.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libstd/path.rs b/src/libstd/path.rs index b6216560d538f..107b7ef136977 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -2257,10 +2257,6 @@ impl Path { /// Returns an object that implements [`Display`] for safely printing paths /// that may contain non-Unicode data. /// - /// This method has been deprecated since version 1.27.0, because paths now - /// implement [`Display`] directly, with the same outcome as using this - /// adapter. - /// /// [`Display`]: ../fmt/trait.Display.html /// /// # Examples @@ -2273,7 +2269,6 @@ impl Path { /// println!("{}", path.display()); /// ``` #[stable(feature = "rust1", since = "1.0.0")] - #[rustc_deprecated(since = "1.27.0", reason = "Path and PathBuf implement Display directly")] pub fn display(&self) -> Display { Display { path: self } } From 980c5ecec3ecc4c8794593871a77ba277a917719 Mon Sep 17 00:00:00 2001 From: boats Date: Wed, 11 Apr 2018 06:10:34 -0700 Subject: [PATCH 3/3] Correct feature name. --- src/libstd/path.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/path.rs b/src/libstd/path.rs index 107b7ef136977..e9ecca6869d15 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -1493,7 +1493,7 @@ impl fmt::Debug for PathBuf { } } -#[stable(feature = "rust1", since = "1.27.0")] +#[stable(feature = "display_path", since = "1.27.0")] impl fmt::Display for PathBuf { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&Display { path: &**self }, formatter) @@ -2487,7 +2487,7 @@ impl AsRef for Path { } } -#[stable(feature = "rust1", since = "1.27.0")] +#[stable(feature = "display_for_path", since = "1.27.0")] impl fmt::Display for Path { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&Display { path: self }, formatter)