D9013: hg-core: simplify `list_tracked_files` operation
acezar (Antoine Cezar)
phabricator at mercurial-scm.org
Fri Sep 11 07:53:50 UTC 2020
acezar created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REVISION SUMMARY
Use directly `ListDirstateTrackedFiles` rather than having an operation builder.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D9013
AFFECTED FILES
rust/hg-core/src/operations/list_tracked_files.rs
rust/hg-core/src/operations/mod.rs
rust/rhg/src/commands/files.rs
CHANGE DETAILS
diff --git a/rust/rhg/src/commands/files.rs b/rust/rhg/src/commands/files.rs
--- a/rust/rhg/src/commands/files.rs
+++ b/rust/rhg/src/commands/files.rs
@@ -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(),
+ ))
+ }
+ },
+ }
+}
diff --git a/rust/hg-core/src/operations/mod.rs b/rust/hg-core/src/operations/mod.rs
--- a/rust/hg-core/src/operations/mod.rs
+++ b/rust/hg-core/src/operations/mod.rs
@@ -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):
diff --git a/rust/hg-core/src/operations/list_tracked_files.rs b/rust/hg-core/src/operations/list_tracked_files.rs
--- a/rust/hg-core/src/operations/list_tracked_files.rs
+++ b/rust/hg-core/src/operations/list_tracked_files.rs
@@ -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 {
To: acezar, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
More information about the Mercurial-devel
mailing list