Skip to content

Commit

Permalink
refactor: make debug-print of StreamProtocol more concise
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
thomaseizinger authored Oct 13, 2023
1 parent 7947a9f commit 6aa805d
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
2 changes: 2 additions & 0 deletions swarm/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
30 changes: 29 additions & 1 deletion swarm/src/stream_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"
);
}
}

0 comments on commit 6aa805d

Please sign in to comment.