changeset 46500:184e46550dc8

rhg: replace command structs with functions

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.

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

--- a/rust/rhg/src/commands.rs	Mon Feb 08 11:13:56 2021 +0100
+++ b/rust/rhg/src/commands.rs	Mon Feb 08 20:33:04 2021 +0100
@@ -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>;
-}

--- a/rust/rhg/src/commands/cat.rs	Mon Feb 08 11:13:56 2021 +0100
+++ b/rust/rhg/src/commands/cat.rs	Mon Feb 08 20:33:04 2021 +0100
@@ -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()),
-        }
-    }
-}

--- a/rust/rhg/src/commands/debugdata.rs	Mon Feb 08 11:13:56 2021 +0100
+++ b/rust/rhg/src/commands/debugdata.rs	Mon Feb 08 20:33:04 2021 +0100
@@ -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(())
 }

--- a/rust/rhg/src/commands/debugrequirements.rs	Mon Feb 08 11:13:56 2021 +0100
+++ b/rust/rhg/src/commands/debugrequirements.rs	Mon Feb 08 20:33:04 2021 +0100
@@ -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(())
-    }
-}

--- a/rust/rhg/src/commands/files.rs	Mon Feb 08 11:13:56 2021 +0100
+++ b/rust/rhg/src/commands/files.rs	Mon Feb 08 20:33:04 2021 +0100
@@ -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(())
 }

--- a/rust/rhg/src/commands/root.rs	Mon Feb 08 11:13:56 2021 +0100
+++ b/rust/rhg/src/commands/root.rs	Mon Feb 08 20:33:04 2021 +0100
@@ -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(())
-    }
-}

--- a/rust/rhg/src/main.rs	Mon Feb 08 11:13:56 2021 +0100
+++ b/rust/rhg/src/main.rs	Mon Feb 08 20:33:04 2021 +0100
@@ -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))
-    }
-}