hg-core: simplify `list_tracked_files` operation

hg-core: simplify `list_tracked_files` operation

Use directly `ListDirstateTrackedFiles` rather than having an operation builder.

Differential Revision: https://phab.mercurial-scm.org/D9013

--- a/rust/hg-core/src/operations/list_tracked_files.rs	Fri Sep 18 16:52:08 2020 +0200
+++ b/rust/hg-core/src/operations/list_tracked_files.rs	Wed Sep 09 12:12:11 2020 +0200
@@ -5,7 +5,6 @@
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
-use super::find_root;
 use crate::dirstate::parsers::parse_dirstate;
 use crate::utils::hg_path::HgPath;
 use crate::{DirstateParseError, EntryState};
@@ -13,74 +12,67 @@
 use std::convert::From;
 use std::fmt;
 use std::fs;
-use std::io;
+use std::ops::Deref;
 use std::path::{Path, PathBuf};
 
-/// Kind of error encoutered by ListTrackedFiles
+/// Kind of error encountered by `ListDirstateTrackedFiles`
 #[derive(Debug)]
-pub enum ListTrackedFilesErrorKind {
+pub enum ListDirstateTrackedFilesErrorKind {
+    /// Error when reading the `dirstate` file
+    IoError(std::io::Error),
+    /// Error when parsing the `dirstate` file
     ParseError(DirstateParseError),
 }
 
-/// A ListTrackedFiles error
+/// A `ListDirstateTrackedFiles` error
 #[derive(Debug)]
-pub struct ListTrackedFilesError {
-    /// Kind of error encoutered by ListTrackedFiles
-    pub kind: ListTrackedFilesErrorKind,
+pub struct ListDirstateTrackedFilesError {
+    /// Kind of error encountered by `ListDirstateTrackedFiles`
+    pub kind: ListDirstateTrackedFilesErrorKind,
 }
 
-impl std::error::Error for ListTrackedFilesError {}
+impl std::error::Error for ListDirstateTrackedFilesError {}
 
-impl fmt::Display for ListTrackedFilesError {
+impl fmt::Display for ListDirstateTrackedFilesError {
     fn fmt(&self, _f: &mut fmt::Formatter<'_>) -> fmt::Result {
         unimplemented!()
     }
 }
 
-impl From<ListTrackedFilesErrorKind> for ListTrackedFilesError {
-    fn from(kind: ListTrackedFilesErrorKind) -> Self {
-        ListTrackedFilesError { kind }
+impl From<ListDirstateTrackedFilesErrorKind>
+    for ListDirstateTrackedFilesError
+{
+    fn from(kind: ListDirstateTrackedFilesErrorKind) -> Self {
+        ListDirstateTrackedFilesError { kind }
     }
 }
 
-/// List files under Mercurial control in the working directory
-pub struct ListTrackedFiles {
-    root: PathBuf,
-}
-
-impl ListTrackedFiles {
-    pub fn new() -> Result<Self, find_root::FindRootError> {
-        let root = find_root::FindRoot::new().run()?;
-        Ok(ListTrackedFiles { root })
-    }
-
-    /// Load the tracked files data from disk
-    pub fn load(&self) -> Result<ListDirstateTrackedFiles, io::Error> {
-        let dirstate = &self.root.join(".hg/dirstate");
-        let content = fs::read(&dirstate)?;
-        Ok(ListDirstateTrackedFiles { content })
-    }
-
-    /// Returns the repository root directory
-    /// TODO I think this is a crutch that creates a dependency that should not
-    /// be there. Operations that need the root of the repository should get
-    /// it themselves, probably in a lazy fashion. But this would make the
-    /// current series even larger, so this is simplified for now.
-    pub fn get_root(&self) -> &Path {
-        &self.root
+impl From<std::io::Error> for ListDirstateTrackedFilesError {
+    fn from(err: std::io::Error) -> Self {
+        let kind = ListDirstateTrackedFilesErrorKind::IoError(err);
+        ListDirstateTrackedFilesError { kind }
     }
 }
 
 /// List files under Mercurial control in the working directory
 /// by reading the dirstate
 pub struct ListDirstateTrackedFiles {
+    /// The `dirstate` content.
     content: Vec<u8>,
 }
 
 impl ListDirstateTrackedFiles {
-    pub fn run(&self) -> Result<Vec<&HgPath>, ListTrackedFilesError> {
+    pub fn new(root: &PathBuf) -> Result<Self, ListDirstateTrackedFilesError> {
+        let dirstate = root.join(".hg/dirstate");
+        let content = fs::read(&dirstate)?;
+        Ok(Self { content })
+    }
+
+    pub fn run(
+        &mut self,
+    ) -> Result<Vec<&HgPath>, ListDirstateTrackedFilesError> {
         let (_, entries, _) = parse_dirstate(&self.content)
-            .map_err(ListTrackedFilesErrorKind::ParseError)?;
+            .map_err(ListDirstateTrackedFilesErrorKind::ParseError)?;
         let mut files: Vec<&HgPath> = entries
             .into_iter()
             .filter_map(|(path, entry)| match entry.state {

--- a/rust/hg-core/src/operations/mod.rs	Fri Sep 18 16:52:08 2020 +0200
+++ b/rust/hg-core/src/operations/mod.rs	Wed Sep 09 12:12:11 2020 +0200
@@ -11,7 +11,8 @@
 };
 pub use find_root::{FindRoot, FindRootError, FindRootErrorKind};
 pub use list_tracked_files::{
-    ListTrackedFiles, ListTrackedFilesError, ListTrackedFilesErrorKind,
+    ListDirstateTrackedFiles, ListDirstateTrackedFilesError,
+    ListDirstateTrackedFilesErrorKind,
 };
 
 // TODO add an `Operation` trait when GAT have landed (rust #44265):

--- a/rust/rhg/src/commands/files.rs	Fri Sep 18 16:52:08 2020 +0200
+++ b/rust/rhg/src/commands/files.rs	Wed Sep 09 12:12:11 2020 +0200
@@ -1,7 +1,12 @@
 use crate::commands::Command;
 use crate::error::{CommandError, CommandErrorKind};
+use crate::ui::utf8_to_local;
 use crate::ui::Ui;
-use hg::operations::{ListTrackedFiles, ListTrackedFilesErrorKind};
+use hg::operations::FindRoot;
+use hg::operations::{
+    ListDirstateTrackedFiles, ListDirstateTrackedFilesError,
+    ListDirstateTrackedFilesErrorKind,
+};
 use hg::utils::files::{get_bytes_from_path, relativize_path};
 use hg::utils::hg_path::HgPathBuf;
 
@@ -21,27 +26,15 @@
 
 impl Command for FilesCommand {
     fn run(&self, ui: &Ui) -> Result<(), CommandError> {
-        let operation_builder = ListTrackedFiles::new()?;
-        let operation = operation_builder.load().map_err(|err| {
-            CommandErrorKind::Abort(Some(
-                [b"abort: ", err.to_string().as_bytes(), b"\n"]
-                    .concat()
-                    .to_vec(),
-            ))
-        })?;
-        let files = operation.run().map_err(|err| match err.kind {
-            ListTrackedFilesErrorKind::ParseError(_) => {
-                CommandErrorKind::Abort(Some(
-                    // TODO find a better error message
-                    b"abort: parse error\n".to_vec(),
-                ))
-            }
-        })?;
+        let root = FindRoot::new().run()?;
+        let mut operation = ListDirstateTrackedFiles::new(&root)
+            .map_err(map_dirstate_error)?;
+        let files = operation.run().map_err(map_dirstate_error)?;
 
         let cwd = std::env::current_dir()
             .or_else(|e| Err(CommandErrorKind::CurrentDirNotFound(e)))?;
         let rooted_cwd = cwd
-            .strip_prefix(operation_builder.get_root())
+            .strip_prefix(&root)
             .expect("cwd was already checked within the repository");
         let rooted_cwd = HgPathBuf::from(get_bytes_from_path(rooted_cwd));
 
@@ -52,7 +45,25 @@
             stdout.write_all(b"\n")?;
         }
         stdout.flush()?;
-
         Ok(())
     }
 }
+
+/// Convert operation errors to command errors
+fn map_dirstate_error(err: ListDirstateTrackedFilesError) -> CommandError {
+    CommandError {
+        kind: match err.kind {
+            ListDirstateTrackedFilesErrorKind::IoError(err) => {
+                CommandErrorKind::Abort(Some(
+                    utf8_to_local(&format!("abort: {}\n", err)).into(),
+                ))
+            }
+            ListDirstateTrackedFilesErrorKind::ParseError(_) => {
+                CommandErrorKind::Abort(Some(
+                    // TODO find a better error message
+                    b"abort: parse error\n".to_vec(),
+                ))
+            }
+        },
+    }
+}