This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7604

rust-hg-path: implement more readable custom Debug for HgPath{,Buf}
ClosedPublic

Authored by martinvonz on Dec 11 2019, 12:51 PM.

Details

Reviewers
kevincox
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHG4b3c8df189bc: rust-hg-path: implement more readable custom Debug for HgPath{,Buf}
Summary

The default prints the vector of bytes as a list of integers. I
considered instead getting rid of the Debug trait, but we use the
Debug format in lots of derived Debug instances, so we probably do
want to implement it.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Dec 11 2019, 12:51 PM
kevincox accepted this revision.Dec 11 2019, 12:57 PM
kevincox added inline comments.
rust/hg-core/src/utils/hg_path.rs
186

I would recommend "HgPath({:?})" so that the result is unambiguous and nicely escaped.

martinvonz marked an inline comment as done.Dec 11 2019, 1:05 PM
martinvonz updated this revision to Diff 18618.
martinvonz added inline comments.Dec 11 2019, 1:05 PM
rust/hg-core/src/utils/hg_path.rs
186

Good point.

Alphare accepted this revision.Dec 11 2019, 2:28 PM
pulkit accepted this revision.Dec 17 2019, 3:56 AM
This revision is now accepted and ready to land.Dec 17 2019, 3:56 AM

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/utils/hg_path.rs (16 lines)

Diff 18616

rust/hg-core/src/utils/hg_path.rs

// //
// FIXME: (adapted from a comment in the stdlib) // FIXME: (adapted from a comment in the stdlib)
// `HgPath::new()` current implementation relies on `Slice` being // `HgPath::new()` current implementation relies on `Slice` being
// layout-compatible with `[u8]`. // layout-compatible with `[u8]`.
// When attribute privacy is implemented, `Slice` should be annotated as // When attribute privacy is implemented, `Slice` should be annotated as
// `#[repr(transparent)]`. // `#[repr(transparent)]`.
// Anyway, `Slice` representation and layout are considered implementation // Anyway, `Slice` representation and layout are considered implementation
// detail, are not documented and must not be relied upon. // detail, are not documented and must not be relied upon.
#[derive(Eq, Ord, PartialEq, PartialOrd, Debug, Hash)] #[derive(Eq, Ord, PartialEq, PartialOrd, Hash)]
pub struct HgPath { pub struct HgPath {
inner: [u8], inner: [u8],
} }
impl HgPath { impl HgPath {
pub fn new<S: AsRef<[u8]> + ?Sized>(s: &S) -> &Self { pub fn new<S: AsRef<[u8]> + ?Sized>(s: &S) -> &Self {
unsafe { &*(s.as_ref() as *const [u8] as *const Self) } unsafe { &*(s.as_ref() as *const [u8] as *const Self) }
} }
#[cfg(test)] #[cfg(test)]
/// Only usable during tests to force developers to handle invalid states /// Only usable during tests to force developers to handle invalid states
fn is_valid(&self) -> bool { fn is_valid(&self) -> bool {
self.check_state().is_ok() self.check_state().is_ok()
} }
} }
impl fmt::Debug for HgPath {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "HgPath({})", String::from_utf8_lossy(&self.inner))
kevincoxUnsubmitted
Done

I would recommend "HgPath({:?})" so that the result is unambiguous and nicely escaped.

kevincox: I would recommend `"HgPath({:?})"` so that the result is unambiguous and nicely escaped.
martinvonzAuthorUnsubmitted
Done

Good point.

martinvonz: Good point.
}
}
impl fmt::Display for HgPath { impl fmt::Display for HgPath {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", String::from_utf8_lossy(&self.inner)) write!(f, "{}", String::from_utf8_lossy(&self.inner))
} }
} }
#[derive(Eq, Ord, Clone, PartialEq, PartialOrd, Debug, Hash)] #[derive(Eq, Ord, Clone, PartialEq, PartialOrd, Hash)]
pub struct HgPathBuf { pub struct HgPathBuf {
inner: Vec<u8>, inner: Vec<u8>,
} }
impl HgPathBuf { impl HgPathBuf {
pub fn new() -> Self { pub fn new() -> Self {
Self { inner: Vec::new() } Self { inner: Vec::new() }
} }
pub fn push(&mut self, byte: u8) { pub fn push(&mut self, byte: u8) {
self.inner.push(byte); self.inner.push(byte);
} }
pub fn from_bytes(s: &[u8]) -> HgPathBuf { pub fn from_bytes(s: &[u8]) -> HgPathBuf {
HgPath::new(s).to_owned() HgPath::new(s).to_owned()
} }
pub fn into_vec(self) -> Vec<u8> { pub fn into_vec(self) -> Vec<u8> {
self.inner self.inner
} }
pub fn as_ref(&self) -> &[u8] { pub fn as_ref(&self) -> &[u8] {
self.inner.as_ref() self.inner.as_ref()
} }
} }
impl fmt::Debug for HgPathBuf {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "HgPathBuf({})", String::from_utf8_lossy(&self.inner))
}
}
impl fmt::Display for HgPathBuf { impl fmt::Display for HgPathBuf {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", String::from_utf8_lossy(&self.inner)) write!(f, "{}", String::from_utf8_lossy(&self.inner))
} }
} }
impl Deref for HgPathBuf { impl Deref for HgPathBuf {
type Target = HgPath; type Target = HgPath;