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

rust/rhg/src/commands.rs

@@ -1,15 +1,5 b''
 pub mod cat;
 pub mod debugdata;
 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>;
-}

rust/rhg/src/commands/cat.rs

@@ -1,58 +1,51 b''
-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;
 use hg::utils::hg_path::HgPathBuf;
 use micro_timer::timed;
 use std::convert::TryFrom;
 
 pub const HELP_TEXT: &str = "
 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()),
-        }
-    }
-}

rust/rhg/src/commands/debugdata.rs

@@ -1,37 +1,42 b''
-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;
 use micro_timer::timed;
 
 pub const HELP_TEXT: &str = "
 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(())
 }

rust/rhg/src/commands/debugrequirements.rs

@@ -1,32 +1,26 b''
-use crate::commands::Command;
 use crate::error::CommandError;
 use crate::ui::Ui;
+use clap::ArgMatches;
 use hg::config::Config;
 use hg::repo::Repo;
 
 pub const HELP_TEXT: &str = "
 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(())
-    }
-}

rust/rhg/src/commands/files.rs

@@ -1,62 +1,55 b''
-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;
 use hg::repo::Repo;
 use hg::utils::files::{get_bytes_from_path, relativize_path};
 use hg::utils::hg_path::{HgPath, HgPathBuf};
 
 pub const HELP_TEXT: &str = "
 List tracked files.
 
 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(())
 }

rust/rhg/src/commands/root.rs

@@ -1,30 +1,24 b''
-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;
 use hg::utils::files::get_bytes_from_path;
 
 pub const HELP_TEXT: &str = "
 Print the root directory of the current repository.
 
 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(())
-    }
-}

rust/rhg/src/main.rs

@@ -1,194 +1,137 b''
 extern crate log;
 use clap::App;
 use clap::AppSettings;
 use clap::Arg;
 use clap::ArgGroup;
 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() {
     env_logger::init();
     let app = App::new("rhg")
         .setting(AppSettings::AllowInvalidUtf8)
         .setting(AppSettings::SubcommandRequired)
         .setting(AppSettings::VersionlessSubcommands)
         .version("0.0.1")
         .subcommand(
             SubCommand::with_name("root").about(commands::root::HELP_TEXT),
         )
         .subcommand(
             SubCommand::with_name("files")
                 .arg(
                     Arg::with_name("rev")
                         .help("search the repository as it is in REV")
                         .short("-r")
                         .long("--revision")
                         .value_name("REV")
                         .takes_value(true),
                 )
                 .about(commands::files::HELP_TEXT),
         )
         .subcommand(
             SubCommand::with_name("cat")
                 .arg(
                     Arg::with_name("rev")
                         .help("search the repository as it is in REV")
                         .short("-r")
                         .long("--revision")
                         .value_name("REV")
                         .takes_value(true),
                 )
                 .arg(
                     clap::Arg::with_name("files")
                         .required(true)
                         .multiple(true)
                         .empty_values(false)
                         .value_name("FILE")
                         .help("Activity to start: activity@category"),
                 )
                 .about(commands::cat::HELP_TEXT),
         )
         .subcommand(
             SubCommand::with_name("debugdata")
                 .about(commands::debugdata::HELP_TEXT)
                 .arg(
                     Arg::with_name("changelog")
                         .help("open changelog")
                         .short("-c")
                         .long("--changelog"),
                 )
                 .arg(
                     Arg::with_name("manifest")
                         .help("open manifest")
                         .short("-m")
                         .long("--manifest"),
                 )
                 .group(
                     ArgGroup::with_name("")
                         .args(&["changelog", "manifest"])
                         .required(true),
                 )
                 .arg(
                     Arg::with_name("rev")
                         .help("revision")
                         .required(true)
                         .value_name("REV"),
                 ),
         )
         .subcommand(
             SubCommand::with_name("debugrequirements")
                 .about(commands::debugrequirements::HELP_TEXT),
         );
 
     let matches = app.clone().get_matches_safe().unwrap_or_else(|err| {
         let _ = ui::Ui::new().writeln_stderr_str(&err.message);
         std::process::exit(exitcode::UNIMPLEMENTED)
     });
 
     let ui = ui::Ui::new();
 
     let command_result = match_subcommand(matches, &ui);
 
     let exit_code = match command_result {
         Ok(_) => exitcode::OK,
 
         // Exit with a specific code and no error message to let a potential
         // wrapper script fallback to Python-based Mercurial.
         Err(CommandError::Unimplemented) => exitcode::UNIMPLEMENTED,
 
         Err(CommandError::Abort { message }) => {
             if !message.is_empty() {
                 // Ignore errors when writing to stderr, we’re already exiting
                 // with failure code so there’s not much more we can do.
                 let _ =
                     ui.write_stderr(&format_bytes!(b"abort: {}\n", message));
             }
             exitcode::ABORT
         }
     };
     std::process::exit(exit_code)
 }
 
 fn match_subcommand(
     matches: ArgMatches,
     ui: &ui::Ui,
 ) -> Result<(), CommandError> {
     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))
-    }
-}