kevincox added inline comments.
kevincox accepted this revision.
INLINE COMMENTS
> node.rs:14
> +/// Binary revisions SHA
> +pub type Node = [u8; 20];
> +
I would recommend making this a proper type because almost no one should be
poking at the bytes of the hash.
struct Node([u8; 20]);
You can make the field `pub` if necessary.
> node.rs:31
> + for i in 0..20 {
> + node[i] = u8::from_str_radix(&hex[i * 2..i * 2 + 2], 16)?
> + }
I find progressing through the string easier to understand than this slicing.
WDYT.
for byte in node.iter_mut() {
*byte = u8::from_str_radix(&hex[..2], 16)?;
hex = &hex[2..];
}
> node.rs:39
> + as_vec.join("")
> +}
> +
If we want to avoid a number of allocations we can do:
pub fn node_to_hex(n: &Node) -> String {
let mut r = String::with_capacity(n.len() * 2);
for byte in n {
write!(r, "{:02x}", byte).unwrap();
}
debug_assert_eq!(r.len(), n.len() * 2);
r
}
The generated code for `write!()` doesn't look great but if hex printing shows
up in benchmarks it would be trivial to write a custom hex-formatter.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7788/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7788
To: gracinet, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
