D7058: rust-dirstate-status: add first Rust implementation of `dirstate.status`

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Tue Oct 15 13:11:42 EDT 2019


martinvonz added inline comments.

INLINE COMMENTS

> status.rs:24
> +    files: Vec<HgPathBuf>,
> +    dmap: &mut DirstateMap,
> +    root_dir: P,

Do we need to borrow it mutably?

> status.rs:49
> +            (normalized, Ok(stat)) => {
> +                if stat.is_file() {
> +                    results.insert(

Should this be `stat.file_type().is_file() || stat.file_type().is_symlink()` as below?

> status.rs:156-160
> +            None if match state {
> +                EntryState::Normal
> +                | EntryState::Merged
> +                | EntryState::Added => true,
> +                _ => false,

This nested match was hard for me to read. I think it would be simpler if you merged this one with the next one inside a simple `None => ` match block, like this:

  None => {
      match state {
          EntryState::Normal
          | EntryState::Merged
          | EntryState::Added => {
              deleted.push(filename);
          },
          EntryState::Removed => {
              removed.push(filename);
          },
          _ => {}
      }
  }

REPOSITORY
  rHG Mercurial

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

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

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


More information about the Mercurial-devel mailing list