D9967: rhg: replace command structs with functions
SimonSapin
phabricator at mercurial-scm.org
Mon Feb 8 22:44:53 UTC 2021
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REVISION SUMMARY
The `Command` trait was not used in any generic context,
and the struct where nothing more than holders for values parsed from CLI
arguments to be available to a `run` method.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D9967
AFFECTED FILES
rust/rhg/src/commands.rs
rust/rhg/src/commands/cat.rs
rust/rhg/src/commands/debugdata.rs
rust/rhg/src/commands/debugrequirements.rs
rust/rhg/src/commands/files.rs
rust/rhg/src/commands/root.rs
rust/rhg/src/main.rs
CHANGE DETAILS
diff --git a/rust/rhg/src/main.rs b/rust/rhg/src/main.rs
--- a/rust/rhg/src/main.rs
+++ b/rust/rhg/src/main.rs
@@ -6,14 +6,11 @@
use clap::ArgMatches;
use clap::SubCommand;
use format_bytes::format_bytes;
-use hg::operations::DebugDataKind;
-use std::convert::TryFrom;
mod commands;
mod error;
mod exitcode;
mod ui;
-use commands::Command;
use error::CommandError;
fn main() {
@@ -126,69 +123,15 @@
let config = hg::config::Config::load()?;
match matches.subcommand() {
- ("root", _) => commands::root::RootCommand::new().run(&ui, &config),
- ("files", Some(matches)) => {
- commands::files::FilesCommand::try_from(matches)?.run(&ui, &config)
- }
- ("cat", Some(matches)) => {
- commands::cat::CatCommand::try_from(matches)?.run(&ui, &config)
+ ("root", Some(matches)) => commands::root::run(ui, &config, matches),
+ ("files", Some(matches)) => commands::files::run(ui, &config, matches),
+ ("cat", Some(matches)) => commands::cat::run(ui, &config, matches),
+ ("debugdata", Some(matches)) => {
+ commands::debugdata::run(ui, &config, matches)
}
- ("debugdata", Some(matches)) => {
- commands::debugdata::DebugDataCommand::try_from(matches)?
- .run(&ui, &config)
- }
- ("debugrequirements", _) => {
- commands::debugrequirements::DebugRequirementsCommand::new()
- .run(&ui, &config)
+ ("debugrequirements", Some(matches)) => {
+ commands::debugrequirements::run(ui, &config, matches)
}
_ => unreachable!(), // Because of AppSettings::SubcommandRequired,
}
}
-
-impl<'a> TryFrom<&'a ArgMatches<'_>> for commands::files::FilesCommand<'a> {
- type Error = CommandError;
-
- fn try_from(args: &'a ArgMatches) -> Result<Self, Self::Error> {
- let rev = args.value_of("rev");
- Ok(commands::files::FilesCommand::new(rev))
- }
-}
-
-impl<'a> TryFrom<&'a ArgMatches<'_>> for commands::cat::CatCommand<'a> {
- type Error = CommandError;
-
- fn try_from(args: &'a ArgMatches) -> Result<Self, Self::Error> {
- let rev = args.value_of("rev");
- let files = match args.values_of("files") {
- Some(files) => files.collect(),
- None => vec![],
- };
- Ok(commands::cat::CatCommand::new(rev, files))
- }
-}
-
-impl<'a> TryFrom<&'a ArgMatches<'_>>
- for commands::debugdata::DebugDataCommand<'a>
-{
- type Error = CommandError;
-
- fn try_from(args: &'a ArgMatches) -> Result<Self, Self::Error> {
- let rev = args
- .value_of("rev")
- .expect("rev should be a required argument");
- let kind = match (
- args.is_present("changelog"),
- args.is_present("manifest"),
- ) {
- (true, false) => DebugDataKind::Changelog,
- (false, true) => DebugDataKind::Manifest,
- (true, true) => {
- unreachable!("Should not happen since options are exclusive")
- }
- (false, false) => {
- unreachable!("Should not happen since options are required")
- }
- };
- Ok(commands::debugdata::DebugDataCommand::new(rev, kind))
- }
-}
diff --git a/rust/rhg/src/commands/root.rs b/rust/rhg/src/commands/root.rs
--- a/rust/rhg/src/commands/root.rs
+++ b/rust/rhg/src/commands/root.rs
@@ -1,6 +1,6 @@
-use crate::commands::Command;
use crate::error::CommandError;
use crate::ui::Ui;
+use clap::ArgMatches;
use format_bytes::format_bytes;
use hg::config::Config;
use hg::repo::Repo;
@@ -12,19 +12,13 @@
Returns 0 on success.
";
-pub struct RootCommand {}
-
-impl RootCommand {
- pub fn new() -> Self {
- RootCommand {}
- }
+pub fn run(
+ ui: &Ui,
+ config: &Config,
+ _args: &ArgMatches,
+) -> Result<(), CommandError> {
+ let repo = Repo::find(config)?;
+ let bytes = get_bytes_from_path(repo.working_directory_path());
+ ui.write_stdout(&format_bytes!(b"{}\n", bytes.as_slice()))?;
+ Ok(())
}
-
-impl Command for RootCommand {
- fn run(&self, ui: &Ui, config: &Config) -> Result<(), CommandError> {
- let repo = Repo::find(config)?;
- let bytes = get_bytes_from_path(repo.working_directory_path());
- ui.write_stdout(&format_bytes!(b"{}\n", bytes.as_slice()))?;
- Ok(())
- }
-}
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,6 +1,6 @@
-use crate::commands::Command;
use crate::error::CommandError;
use crate::ui::Ui;
+use clap::ArgMatches;
use hg::config::Config;
use hg::operations::list_rev_tracked_files;
use hg::operations::Dirstate;
@@ -14,49 +14,42 @@
Returns 0 on success.
";
-pub struct FilesCommand<'a> {
- rev: Option<&'a str>,
-}
-
-impl<'a> FilesCommand<'a> {
- pub fn new(rev: Option<&'a str>) -> Self {
- FilesCommand { rev }
- }
+pub fn run(
+ ui: &Ui,
+ config: &Config,
+ args: &ArgMatches,
+) -> Result<(), CommandError> {
+ let rev = args.value_of("rev");
- fn display_files(
- &self,
- ui: &Ui,
- repo: &Repo,
- files: impl IntoIterator<Item = &'a HgPath>,
- ) -> Result<(), CommandError> {
- let cwd = hg::utils::current_dir()?;
- let rooted_cwd = cwd
- .strip_prefix(repo.working_directory_path())
- .expect("cwd was already checked within the repository");
- let rooted_cwd = HgPathBuf::from(get_bytes_from_path(rooted_cwd));
-
- let mut stdout = ui.stdout_buffer();
-
- for file in files {
- stdout.write_all(relativize_path(file, &rooted_cwd).as_ref())?;
- stdout.write_all(b"\n")?;
- }
- stdout.flush()?;
- Ok(())
+ let repo = Repo::find(config)?;
+ if let Some(rev) = rev {
+ let files =
+ list_rev_tracked_files(&repo, rev).map_err(|e| (e, rev))?;
+ display_files(ui, &repo, files.iter())
+ } else {
+ let distate = Dirstate::new(&repo)?;
+ let files = distate.tracked_files()?;
+ display_files(ui, &repo, files)
}
}
-impl<'a> Command for FilesCommand<'a> {
- fn run(&self, ui: &Ui, config: &Config) -> Result<(), CommandError> {
- let repo = Repo::find(config)?;
- if let Some(rev) = self.rev {
- let files =
- list_rev_tracked_files(&repo, rev).map_err(|e| (e, rev))?;
- self.display_files(ui, &repo, files.iter())
- } else {
- let distate = Dirstate::new(&repo)?;
- let files = distate.tracked_files()?;
- self.display_files(ui, &repo, files)
- }
+fn display_files<'a>(
+ ui: &Ui,
+ repo: &Repo,
+ files: impl IntoIterator<Item = &'a HgPath>,
+) -> Result<(), CommandError> {
+ let cwd = hg::utils::current_dir()?;
+ let rooted_cwd = cwd
+ .strip_prefix(repo.working_directory_path())
+ .expect("cwd was already checked within the repository");
+ let rooted_cwd = HgPathBuf::from(get_bytes_from_path(rooted_cwd));
+
+ let mut stdout = ui.stdout_buffer();
+
+ for file in files {
+ stdout.write_all(relativize_path(file, &rooted_cwd).as_ref())?;
+ stdout.write_all(b"\n")?;
}
+ stdout.flush()?;
+ Ok(())
}
diff --git a/rust/rhg/src/commands/debugrequirements.rs b/rust/rhg/src/commands/debugrequirements.rs
--- a/rust/rhg/src/commands/debugrequirements.rs
+++ b/rust/rhg/src/commands/debugrequirements.rs
@@ -1,6 +1,6 @@
-use crate::commands::Command;
use crate::error::CommandError;
use crate::ui::Ui;
+use clap::ArgMatches;
use hg::config::Config;
use hg::repo::Repo;
@@ -8,25 +8,19 @@
Print the current repo requirements.
";
-pub struct DebugRequirementsCommand {}
-
-impl DebugRequirementsCommand {
- pub fn new() -> Self {
- DebugRequirementsCommand {}
+pub fn run(
+ ui: &Ui,
+ config: &Config,
+ _args: &ArgMatches,
+) -> Result<(), CommandError> {
+ let repo = Repo::find(config)?;
+ let mut output = String::new();
+ let mut requirements: Vec<_> = repo.requirements().iter().collect();
+ requirements.sort();
+ for req in requirements {
+ output.push_str(req);
+ output.push('\n');
}
+ ui.write_stdout(output.as_bytes())?;
+ Ok(())
}
-
-impl Command for DebugRequirementsCommand {
- fn run(&self, ui: &Ui, config: &Config) -> Result<(), CommandError> {
- let repo = Repo::find(config)?;
- let mut output = String::new();
- let mut requirements: Vec<_> = repo.requirements().iter().collect();
- requirements.sort();
- for req in requirements {
- output.push_str(req);
- output.push('\n');
- }
- ui.write_stdout(output.as_bytes())?;
- Ok(())
- }
-}
diff --git a/rust/rhg/src/commands/debugdata.rs b/rust/rhg/src/commands/debugdata.rs
--- a/rust/rhg/src/commands/debugdata.rs
+++ b/rust/rhg/src/commands/debugdata.rs
@@ -1,6 +1,6 @@
-use crate::commands::Command;
use crate::error::CommandError;
use crate::ui::Ui;
+use clap::ArgMatches;
use hg::config::Config;
use hg::operations::{debug_data, DebugDataKind};
use hg::repo::Repo;
@@ -10,28 +10,33 @@
Dump the contents of a data file revision
";
-pub struct DebugDataCommand<'a> {
- rev: &'a str,
- kind: DebugDataKind,
-}
-
-impl<'a> DebugDataCommand<'a> {
- pub fn new(rev: &'a str, kind: DebugDataKind) -> Self {
- DebugDataCommand { rev, kind }
- }
-}
+#[timed]
+pub fn run(
+ ui: &Ui,
+ config: &Config,
+ args: &ArgMatches,
+) -> Result<(), CommandError> {
+ let rev = args
+ .value_of("rev")
+ .expect("rev should be a required argument");
+ let kind =
+ match (args.is_present("changelog"), args.is_present("manifest")) {
+ (true, false) => DebugDataKind::Changelog,
+ (false, true) => DebugDataKind::Manifest,
+ (true, true) => {
+ unreachable!("Should not happen since options are exclusive")
+ }
+ (false, false) => {
+ unreachable!("Should not happen since options are required")
+ }
+ };
-impl<'a> Command for DebugDataCommand<'a> {
- #[timed]
- fn run(&self, ui: &Ui, config: &Config) -> Result<(), CommandError> {
- let repo = Repo::find(config)?;
- let data = debug_data(&repo, self.rev, self.kind)
- .map_err(|e| (e, self.rev))?;
+ let repo = Repo::find(config)?;
+ let data = debug_data(&repo, rev, kind).map_err(|e| (e, rev))?;
- let mut stdout = ui.stdout_buffer();
- stdout.write_all(&data)?;
- stdout.flush()?;
+ let mut stdout = ui.stdout_buffer();
+ stdout.write_all(&data)?;
+ stdout.flush()?;
- Ok(())
- }
+ Ok(())
}
diff --git a/rust/rhg/src/commands/cat.rs b/rust/rhg/src/commands/cat.rs
--- a/rust/rhg/src/commands/cat.rs
+++ b/rust/rhg/src/commands/cat.rs
@@ -1,6 +1,6 @@
-use crate::commands::Command;
use crate::error::CommandError;
use crate::ui::Ui;
+use clap::ArgMatches;
use hg::config::Config;
use hg::operations::cat;
use hg::repo::Repo;
@@ -12,47 +12,40 @@
Output the current or given revision of files
";
-pub struct CatCommand<'a> {
- rev: Option<&'a str>,
- files: Vec<&'a str>,
-}
+#[timed]
+pub fn run(
+ ui: &Ui,
+ config: &Config,
+ args: &ArgMatches,
+) -> Result<(), CommandError> {
+ let rev = args.value_of("rev");
+ let file_args = match args.values_of("files") {
+ Some(files) => files.collect(),
+ None => vec![],
+ };
-impl<'a> CatCommand<'a> {
- pub fn new(rev: Option<&'a str>, files: Vec<&'a str>) -> Self {
- Self { rev, files }
+ let repo = Repo::find(config)?;
+ let cwd = hg::utils::current_dir()?;
+
+ let mut files = vec![];
+ for file in file_args.iter() {
+ // TODO: actually normalize `..` path segments etc?
+ let normalized = cwd.join(&file);
+ let stripped = normalized
+ .strip_prefix(&repo.working_directory_path())
+ // TODO: error message for path arguments outside of the repo
+ .map_err(|_| CommandError::abort(""))?;
+ let hg_file = HgPathBuf::try_from(stripped.to_path_buf())
+ .map_err(|e| CommandError::abort(e.to_string()))?;
+ files.push(hg_file);
}
- fn display(&self, ui: &Ui, data: &[u8]) -> Result<(), CommandError> {
- ui.write_stdout(data)?;
- Ok(())
+ match rev {
+ Some(rev) => {
+ let data = cat(&repo, rev, &files).map_err(|e| (e, rev))?;
+ ui.write_stdout(&data)?;
+ Ok(())
+ }
+ None => Err(CommandError::Unimplemented.into()),
}
}
-
-impl<'a> Command for CatCommand<'a> {
- #[timed]
- fn run(&self, ui: &Ui, config: &Config) -> Result<(), CommandError> {
- let repo = Repo::find(config)?;
- let cwd = hg::utils::current_dir()?;
-
- let mut files = vec![];
- for file in self.files.iter() {
- // TODO: actually normalize `..` path segments etc?
- let normalized = cwd.join(&file);
- let stripped = normalized
- .strip_prefix(&repo.working_directory_path())
- // TODO: error message for path arguments outside of the repo
- .map_err(|_| CommandError::abort(""))?;
- let hg_file = HgPathBuf::try_from(stripped.to_path_buf())
- .map_err(|e| CommandError::abort(e.to_string()))?;
- files.push(hg_file);
- }
-
- match self.rev {
- Some(rev) => {
- let data = cat(&repo, rev, &files).map_err(|e| (e, rev))?;
- self.display(ui, &data)
- }
- None => Err(CommandError::Unimplemented.into()),
- }
- }
-}
diff --git a/rust/rhg/src/commands.rs b/rust/rhg/src/commands.rs
--- a/rust/rhg/src/commands.rs
+++ b/rust/rhg/src/commands.rs
@@ -3,13 +3,3 @@
pub mod debugrequirements;
pub mod files;
pub mod root;
-use crate::error::CommandError;
-use crate::ui::Ui;
-use hg::config::Config;
-
-/// The common trait for rhg commands
-///
-/// Normalize the interface of the commands provided by rhg
-pub trait Command {
- fn run(&self, ui: &Ui, config: &Config) -> Result<(), CommandError>;
-}
To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
More information about the Mercurial-devel
mailing list