From 6aa805d581981adf4fe9c910fc4b2bb9e3a5ed04 Mon Sep 17 00:00:00 2001
From: Thomas Eizinger <thomas@eizinger.io>
Date: Fri, 13 Oct 2023 16:23:11 +1100
Subject: [PATCH] refactor: make debug-print of `StreamProtocol` more concise

The` fmt::Debug` implementation of a type should in most cases reveal its internal structure. `StreamProtocol` is likely to be debug-printed a lot and in many cases, the only contract is the `fmt::Debug` impl. The internals of `StreamProtocol` only exist for performance reasons to avoid allocations for statically-known protocol strings. Revealing this implementation detail isn't particularly beneficial to end users. At the same time, the current implementation is very noise.

Previously, the `protocols` field of an `identify::Info` would e.g. read as:

```
protocols: [StreamProtocol { inner: Right("/ipfs/id/1.0.0") }, StreamProtocol { inner: Right("/ipfs/id/push/1.0.0") }, StreamProtocol { inner: Right("/ipfs/kad/1.0.0") }]
```

With this patch, it reads as:

```
protocols: ["/ipfs/id/1.0.0", "/ipfs/kad/1.0.0", "/ipfs/id/push/1.0.0"]
```

Pull-Request: #4631.
---
 swarm/CHANGELOG.md           |  2 ++
 swarm/src/stream_protocol.rs | 30 +++++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/swarm/CHANGELOG.md b/swarm/CHANGELOG.md
index d0567f7d..3e4ccd00 100644
--- a/swarm/CHANGELOG.md
+++ b/swarm/CHANGELOG.md
@@ -4,6 +4,8 @@
   Most users should use `libp2p::SwarmBuilder`.
   In some special cases, users may need to use `Swarm::new` and `Config` instead of the new `libp2p::SwarmBuilder`.
   See [PR 4120].
+- Make the `Debug` implementation of `StreamProtocol` more concise.
+  See [PR 4631](https://github.com/libp2p/rust-libp2p/pull/4631).
 
 [PR 4120]: https://github.com/libp2p/rust-libp2p/pull/4120
 
diff --git a/swarm/src/stream_protocol.rs b/swarm/src/stream_protocol.rs
index bce0ec51..f746429a 100644
--- a/swarm/src/stream_protocol.rs
+++ b/swarm/src/stream_protocol.rs
@@ -7,7 +7,7 @@ use std::sync::Arc;
 ///
 /// libp2p nodes use stream protocols to negotiate what to do with a newly opened stream.
 /// Stream protocols are string-based and must start with a forward slash: `/`.
-#[derive(Debug, Clone, Eq)]
+#[derive(Clone, Eq)]
 pub struct StreamProtocol {
     inner: Either<&'static str, Arc<str>>,
 }
@@ -50,6 +50,12 @@ impl AsRef<str> for StreamProtocol {
     }
 }
 
+impl fmt::Debug for StreamProtocol {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        either::for_both!(&self.inner, s => s.fmt(f))
+    }
+}
+
 impl fmt::Display for StreamProtocol {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         self.inner.fmt(f)
@@ -102,3 +108,25 @@ impl fmt::Display for InvalidProtocol {
 }
 
 impl std::error::Error for InvalidProtocol {}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn stream_protocol_print() {
+        let protocol = StreamProtocol::new("/foo/bar/1.0.0");
+
+        let debug = format!("{protocol:?}");
+        let display = format!("{protocol}");
+
+        assert_eq!(
+            debug, r#""/foo/bar/1.0.0""#,
+            "protocol to debug print as string with quotes"
+        );
+        assert_eq!(
+            display, "/foo/bar/1.0.0",
+            "protocol to display print as string without quotes"
+        );
+    }
+}