[Updated] D8612: rhg: add RootCommand using hg-core FindRoot operation to prepare `hg root`

[marmoute (Pierre-Yves David)]

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

INLINE COMMENTS

> root.rs:24
> +            .write_all(&[bytes.as_slice(), b"\n"].concat())
> +            .expect("Should be able to write to stdout");
> +

If stdout is closed, this will fails. We probably want a better error than that eventually.

> root.rs:34
> +                    Some(path) => path,
> +                    None => panic!("This should not happen"),
> +                };

Maybe we use a bit more explicit error, like: "Error is RootNotFound, but no path provided" or soemthing in that flavor.

> root.rs:50
> +                    )
> +                    .expect("Should be able to write to stderr");
> +            }

Same feedback, there wil be legit cas were we cannot write to stdout and we should improve the errors here (and test theses case)

> root.rs:60
> +                        "Current directory does not exists",
> +                        "or permissions are insufficient to get access to it"
> +                    )

What kind of error does Mercurial issue in this case ? Could we keep the same behavior ?

REPOSITORY
  rHG Mercurial

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

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

To: acezar, #hg-reviewers, marmoute
Cc: marmoute, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200609/0ba57ff8/attachment-0002.html>