D6626: rust-dirstate: create dirstate submodule in hg-cpython

[kevincox (Kevin Cox)]

kevincox added inline comments.
kevincox accepted this revision.

INLINE COMMENTS

> dirs_multiset.rs:34
> +            skip_state = Some(skip.extract::<PyBytes>(py)?.data(py)[0] as i8);
> +        }
> +        let dirs_map;

let skip_state = skip.map(|skip| skip.extract::<PyBytes>(py)?.data(py)[0] as i8);

> dirs_multiset.rs:35
> +        }
> +        let dirs_map;
> +

let dirs_map = if ...

> dirs_multiset.rs:68
> +        )
> +            .and(Ok(py.None()))
> +            .or_else(|e| {

.map(py.None())

> mod.rs:119
> +            let state = stats.get_item(py, 0)?.extract::<PyBytes>(py)?;
> +            let state = state.data(py)[0] as i8;
> +            let mode = stats.get_item(py, 1)?.extract(py)?;

Should there be a check that this element exists?

> mod.rs:178
> +                },
> +            ) in new_dirstate_vec
> +            {

I think I would find it more readable as `for (filename, dirstate) in ...`. If you want to unpack it I would do that in the first line of the loop body. However it might also be best just to use `dirstate.state`. I guess the downside there is you don't get a compile error if a new field is added.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6626/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6626

To: Alphare, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel