D7788: rust-node: binary Node and conversion utilities

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Fri Jan 17 20:24:57 EST 2020


This revision now requires changes to proceed.
martinvonz added inline comments.
martinvonz requested changes to this revision.

INLINE COMMENTS

> dirs_multiset.rs:111
>                          path.as_ref().to_owned(),
> -                    ))
> +                    ));
>                  }

please revert unrelated change

> kevincox wrote in node.rs:31
> 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..];
>   }

Or even use `hex::decode()` from the `hex` crate? Can also use `hex::encode()` in `node_to_hex` if we're okay with that.

> node.rs:41-51
> +/// Retrieve the `i`th half-byte from a bytes slice
> +///
> +/// This is also the `i`th hexadecimal digit in numeric form,
> +/// also called a [nybble](https://en.wikipedia.org/wiki/Nibble).
> +pub fn get_nybble(i: usize, s: &[u8]) -> u8 {
> +    if i % 2 == 0 {
> +        s[i / 2] >> 4

This feels like something that will only be used in nodemap.rs, so put it there instead? Or do you think (or know) that it will be used elsewhere soon?

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, martinvonz
Cc: martinvonz, durin42, kevincox, mercurial-devel


More information about the Mercurial-devel mailing list