From fefdffe1e90278f61a6a0ef4722b3ecb87c4f5cc Mon Sep 17 00:00:00 2001
From: Leonard Steppy <jonlaurem@gmail.com>
Date: Wed, 8 Jan 2025 10:24:15 +0100
Subject: [PATCH] Use run function for commands where possible

---
 src/command.rs |  1 +
 src/main.rs    | 51 ++++++++++++++++++++------------------------------
 2 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/src/command.rs b/src/command.rs
index be7431f..529ce24 100644
--- a/src/command.rs
+++ b/src/command.rs
@@ -7,6 +7,7 @@ use std::iter::once;
 use std::process::{Command, ExitStatus, Stdio};
 
 pub trait LogRunnable {
+  //TODO add function to query output on success, could further improve code readability
   fn run(&mut self, logger: &Logger) -> Result<(), SpecificExecutionError>;
 }
 
diff --git a/src/main.rs b/src/main.rs
index e775d74..ef7d7b9 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,11 +1,12 @@
 mod action;
+mod command;
 mod file;
 mod logger;
 mod os_string_builder;
 mod server;
-mod command;
 
 use crate::action::{Action, FileAction, ServerActions};
+use crate::command::LogRunnable;
 use crate::file::{FileMatcher, FileNameInfo};
 use crate::logger::{LogLevel, Logger};
 use crate::os_string_builder::ReplaceWithOsStr;
@@ -160,7 +161,7 @@ fn main() -> Result<(), String> {
       file_name,
     } => {
       require_non_empty_servers(&servers)?;
-      start_ssh_agent()?;
+      start_ssh_agent(&logger)?;
 
       let file_name_info =
         FileNameInfo::try_from(file.clone()).map_err(|e| format!("bad file: {e}"))?;
@@ -275,19 +276,15 @@ fn main() -> Result<(), String> {
               ShellCmd::new("scp")
                 .arg(file.clone())
                 .arg(osf!(&server.ssh_name) + ":" + &server_actions.working_directory)
-                .spawn()
-                .map_err(|e| format!("failed to upload file: {e}"))?
-                .wait()
-                .map_err(|e| format!("failed to wait for upload: {e}"))?;
+                .run(&logger)
+                .map_err(|e| format!("upload failure: {e}"))?;
             }
             Action::Delete => {
               ShellCmd::new("ssh")
                 .arg(&server.ssh_name)
                 .arg(osf!("cd ") + &server_actions.working_directory + "; rm " + &file_action.file)
-                .spawn()
-                .map_err(|e| format!("failed to send delete command: {e}"))?
-                .wait()
-                .map_err(|e| format!("failed to wait for delete command: {e}"))?;
+                .run(&logger)
+                .map_err(|e| format!("failed to delete old version: {e}"))?;
             }
             Action::Rename { new_name } => {
               ShellCmd::new("ssh")
@@ -300,10 +297,8 @@ fn main() -> Result<(), String> {
                     + " "
                     + new_name,
                 )
-                .spawn()
-                .map_err(|e| format!("failed to send rename command: {e}"))?
-                .wait()
-                .map_err(|e| format!("failed to wait for rename command: {e}"))?;
+                .run(&logger)
+                .map_err(|e| format!("failed to rename: {e}"))?;
             }
           }
         }
@@ -312,17 +307,15 @@ fn main() -> Result<(), String> {
       log!(logger, "Done!");
     }
     Command::Command { command } => {
-      start_ssh_agent()?;
+      start_ssh_agent(&logger)?;
       require_non_empty_servers(&servers)?;
       for server in servers {
         log!(logger, "Running command on '{}'...", server.ssh_name);
         ShellCmd::new("ssh")
           .arg(server.ssh_name)
           .arg(osf!("cd ") + server.server_directory_path + "; " + &command)
-          .spawn()
-          .map_err(|_| "failed to start ssh command".to_string())?
-          .wait()
-          .map_err(|e| format!("failed to wait for ssh command completion: {e}"))?;
+          .run(&logger)
+          .map_err(|e| format!("{e}"))?;
       }
       log!(logger, "Done!");
     }
@@ -358,15 +351,15 @@ fn main() -> Result<(), String> {
       }
 
       require_non_empty_servers(&servers)?;
-      start_ssh_agent()?;
+      start_ssh_agent(&logger)?;
 
       for server in servers {
         log!(logger, "Downloading file from {}...", server.ssh_name);
         ShellCmd::new("scp")
           .arg(osf!(&server.ssh_name) + ":" + server.server_directory_path.join(&file))
           .arg(&working_directory)
-          .status()
-          .map_err(|e| format!("failed to download file: {e}"))?;
+          .run(&logger)
+          .map_err(|e| format!("download failure: {e}"))?;
 
         //open file in editor
         let mut shell_args = shell_words::split(&editor)
@@ -378,15 +371,15 @@ fn main() -> Result<(), String> {
         let command = shell_args.remove(0);
         ShellCmd::new(command)
           .args(shell_args)
-          .status()
+          .run(&logger)
           .map_err(|e| format!("failed to open file in editor: {e}"))?;
 
         //upload file again
         ShellCmd::new("scp")
           .arg(working_directory.join(file_name))
           .arg(osf!(&server.ssh_name) + ":" + server.server_directory_path.join(&file))
-          .status()
-          .map_err(|e| format!("failed to upload file again: {e}"))?;
+          .run(&logger)
+          .map_err(|e| format!("failed to re-upload file: {e}"))?;
       }
 
       log!(logger, "Done!");
@@ -404,7 +397,7 @@ fn require_non_empty_servers(servers: &[Server]) -> Result<(), String> {
   }
 }
 
-fn start_ssh_agent() -> Result<(), String> {
+fn start_ssh_agent(logger: &Logger) -> Result<(), String> {
   //start the ssh agent
   let agent_output = ShellCmd::new("ssh-agent")
     .arg("-s")
@@ -424,11 +417,7 @@ fn start_ssh_agent() -> Result<(), String> {
   }
 
   //add the ssh key
-  ShellCmd::new("ssh-add")
-    .spawn()
-    .map_err(|e| format!("failed to add ssh key: {}", e))?
-    .wait()
-    .expect("failed to wait on ssh-add");
+  ShellCmd::new("ssh-add").run(logger).map_err(|e| format!("failed to add ssh-key: {e}"))?;
   Ok(())
 }