Add isolate_post_checkout color warning messages

Add isolate_post_checkout colored warning messages in the isolate
process so that users would be provided useful information about the
isolate_post_checkout hook.

The approach was simply adding the warning messages to the isolate
command so that the would appear when used via any of the other
commands, e.g. isolate, request-review, integrate. To do this I first
needed to define the print_warn() function which then turned into
extracting the ps::private::utils into a directory module rather than a
single file. Once that was complete I was able to use the new
print_warn() funtion to actually print the warning messages.

This relates to issue #79.

[changelog]
added: isolate_post_checkout color warning messages

ps-id: 389b2a94-2e3e-4e55-9085-e0a6d12b7469

src/commands/integrate.rs

@@ -1,7 +1,7 @@
 use gps as ps;
 
-pub fn integrate(patch_index: usize, force: bool, keep_branch: bool, branch_name: Option<String>) {
-  match ps::integrate::integrate(patch_index, force, keep_branch, branch_name) {
+pub fn integrate(patch_index: usize, force: bool, keep_branch: bool, branch_name: Option<String>, color: bool) {
+  match ps::integrate::integrate(patch_index, force, keep_branch, branch_name, color) {
     Ok(_) => {},
     Err(e) => eprintln!("Error: {:?}", e)
   }

src/commands/isolate.rs

@@ -1,9 +1,9 @@
 use gps as ps;
+use super::utils::print_err;
 
-pub fn isolate(patch_index: Option<usize>) {
-  let res = ps::isolate(patch_index);
-  match res {
+pub fn isolate(patch_index: Option<usize>, color: bool) {
+  match ps::isolate(patch_index, color) {
     Ok(_) => {},
-    Err(e) => eprintln!("Error: {:?}", e)
+    Err(e) => print_err(color, format!("\nError: {:?}\n", e).as_str())
   }
 }

src/commands/request_review.rs

@@ -9,7 +9,7 @@ use gps as ps;
 use super::utils::print_err;
 
 pub fn request_review(patch_index: usize, branch_name: Option<String>, color: bool) {
-  match ps::request_review(patch_index, branch_name) {
+match ps::request_review(patch_index, branch_name, color) {
     Ok(_) => {},
     Err(ps::RequestReviewError::PostSyncHookNotFound) => {
       print_err(color,
@@ -55,6 +55,6 @@ r#"
 "#, path_str, path_str);
       print_err(color, &msg);
     },
-    Err(e) => print_err(color, format!("\n{}\n", e).as_str())
+    Err(e) => print_err(color, format!("\nError: {}\n", e).as_str())
   };
 }

src/commands/utils.rs

@@ -7,4 +7,3 @@ pub fn print_err(color: bool, message: &str) {
     eprintln!("{}", message)
   }
 }
-

src/main.rs

@@ -21,14 +21,14 @@ fn main() {
     match opt.command {
         cli::Command::Branch(opts) => commands::branch::branch(opts.start_patch_index, opts.end_patch_index, opts.branch_name),
         cli::Command::RequestReviewBranch(opts) => commands::request_review_branch::request_review_branch(opts.patch_index, opts.branch_name),
-        cli::Command::Integrate(opts) => commands::integrate::integrate(opts.patch_index, opts.force, opts.keep_branch, opts.branch_name),
+        cli::Command::Integrate(opts) => commands::integrate::integrate(opts.patch_index, opts.force, opts.keep_branch, opts.branch_name, opt.color),
         cli::Command::List => commands::list::list(opt.color),
         cli::Command::Rebase(opts) => commands::rebase::rebase(opts.r#continue),
         cli::Command::Pull => commands::pull::pull(opt.color),
         cli::Command::RequestReview(opts) => commands::request_review::request_review(opts.patch_index, opts.branch_name, opt.color),
         cli::Command::Show(opts) => commands::show::show(opts.patch_index),
         cli::Command::Sync(opts) => commands::sync::sync(opts.patch_index, opts.branch_name),
-        cli::Command::Isolate(opts) => commands::isolate::isolate(opts.patch_index),
+        cli::Command::Isolate(opts) => commands::isolate::isolate(opts.patch_index, opt.color),
         cli::Command::Checkout(opts) => commands::checkout::checkout(opts.patch_index),
         cli::Command::CreatePatch => commands::create_patch::create_patch(),
         cli::Command::AmendPatch => commands::amend_patch::amend_patch(),

src/ps/private/utils.rs → src/ps/private/utils/execute.rs

@@ -1,18 +1,8 @@
-// This is the `utils` module. It is responsible for housing generic utility
-// functionality that this application needs. This should be functionality
-// that is generic enough that other applications in theory would find it
-// useful when tackeling a completely different problem space than Git Patch
-// Stack. All code fitting that description belongs here.
-
 use std::os::unix::prelude::ExitStatusExt;
 use std::process::Command;
 use std::io;
 use std::result::Result;
 
-pub trait Mergable {
-  fn merge(&self, b: &Self) -> Self;
-}
-
 #[derive(Debug)]
 pub enum ExecuteError {
   SpawnFailure(io::Error),
@@ -27,22 +17,22 @@ pub enum ExecuteError {
 /// exit status.
 pub fn execute(exe: &str, args: &[&str]) -> Result<(), ExecuteError> {
   match Command::new(exe).args(args).spawn() {
-    Err(e) => return Err(ExecuteError::SpawnFailure(e)),
+    Err(e) => Err(ExecuteError::SpawnFailure(e)),
     Ok(mut child) => match child.wait() {
-      Err(e) => return Err(ExecuteError::Failure(e)),
+      Err(e) => Err(ExecuteError::Failure(e)),
       Ok(status) => {
         if status.success() {
-          return Ok(())
+          Ok(())
         } else {
           match status.code() {
-            Some(code) => return Err(ExecuteError::ExitStatus(code)),
+            Some(code) => Err(ExecuteError::ExitStatus(code)),
             None => match status.signal() {
-              Some(signal) => return Err(ExecuteError::ExitSignal(signal)),
-              None => return Err(ExecuteError::ExitMissingSignal)
+              Some(signal) => Err(ExecuteError::ExitSignal(signal)),
+              None => Err(ExecuteError::ExitMissingSignal)
             }
           }
         }
       }
     }
-  };
+  }
 }

src/ps/private/utils/mergable.rs

@@ -0,0 +1,3 @@
+pub trait Mergable {
+  fn merge(&self, b: &Self) -> Self;
+}

src/ps/private/utils/mod.rs

@@ -0,0 +1,7 @@
+mod execute;
+mod mergable;
+mod print_warn;
+
+pub use execute::{execute, ExecuteError};
+pub use mergable::Mergable;
+pub use print_warn::print_warn;

src/ps/private/utils/print_warn.rs

@@ -0,0 +1,10 @@
+use ansi_term::Colour::Yellow;
+
+pub fn print_warn(color: bool, message: &str) {
+  if color {
+    eprintln!("{}", Yellow.paint(message))
+  } else {
+    eprintln!("{}", message)
+  }
+}
+

src/ps/private/verify_isolation.rs

@@ -8,9 +8,9 @@ pub enum VerifyIsolationError {
   IsolateResetFailed(IsolateError)
 }
 
-pub fn verify_isolation(patch_index: usize) -> Result<(), VerifyIsolationError> {
-  match isolate::isolate(Some(patch_index)) {
-    Ok(_) => Ok(isolate::isolate(None).map_err(VerifyIsolationError::IsolateResetFailed)?),
+pub fn verify_isolation(patch_index: usize, color: bool) -> Result<(), VerifyIsolationError> {
+  match isolate::isolate(Some(patch_index), color) {
+    Ok(_) => Ok(isolate::isolate(None, color).map_err(VerifyIsolationError::IsolateResetFailed)?),
     Err(e) => match e {
       // pre-checkout errors
       IsolateError::OpenGitRepositoryFailed(_)
@@ -30,7 +30,7 @@ pub fn verify_isolation(patch_index: usize) -> Result<(), VerifyIsolationError>
         | IsolateError::FailedToCheckout(_) => Err(VerifyIsolationError::IsolateFailed(e)),
       // post-checkout errors
       _ => {
-        isolate::isolate(None).map_err(VerifyIsolationError::IsolateResetFailed)?;
+        isolate::isolate(None, color).map_err(VerifyIsolationError::IsolateResetFailed)?;
         Err(VerifyIsolationError::IsolateFailed(e))
       }
     }

src/ps/public/integrate.rs

@@ -47,7 +47,7 @@ pub enum IntegrateError {
   ShowFailed(show::ShowError)
 }
 
-pub fn integrate(patch_index: usize, force: bool, keep_branch: bool, given_branch_name_option: Option<String>) -> Result<(), IntegrateError> {
+pub fn integrate(patch_index: usize, force: bool, keep_branch: bool, given_branch_name_option: Option<String>, color: bool) -> Result<(), IntegrateError> {
   let repo = git::create_cwd_repo().map_err(|_| IntegrateError::RepositoryNotFound)?;
 
   // verify that the patch-index has a corresponding commit
@@ -59,7 +59,7 @@ pub fn integrate(patch_index: usize, force: bool, keep_branch: bool, given_branc
   let config = config::get_config(repo_root_str).map_err(IntegrateError::GetConfigFailed)?;
 
   if config.integrate.verify_isolation {
-    verify_isolation::verify_isolation(patch_index).map_err(IntegrateError::IsolationVerificationFailed)?;
+    verify_isolation::verify_isolation(patch_index, color).map_err(IntegrateError::IsolationVerificationFailed)?;
   }
 
   if force {

src/ps/public/isolate.rs

@@ -32,7 +32,7 @@ pub enum IsolateError {
   DeleteIsolateBranchFailed(git2::Error)
 }
 
-pub fn isolate(patch_index_optional: Option<usize>) -> Result<(), IsolateError> {
+pub fn isolate(patch_index_optional: Option<usize>, color: bool) -> Result<(), IsolateError> {
   let isolate_branch_name = "ps/tmp/isolate";
   let repo = ps::private::git::create_cwd_repo().map_err(IsolateError::OpenGitRepositoryFailed)?;
   let config = git2::Config::open_default().map_err(IsolateError::OpenGitConfigFailed)?;
@@ -68,8 +68,44 @@ pub fn isolate(patch_index_optional: Option<usize>) -> Result<(), IsolateError>
       let repo_root_str = repo_root_path.to_str().ok_or(IsolateError::PathNotUtf8)?;
       match hooks::find_hook(repo_root_str, "isolate_post_checkout") {
         Ok(hook_path) => utils::execute(hook_path.to_str().ok_or(IsolateError::PathNotUtf8)?, &[]).map_err(IsolateError::HookExecutionFailed)?,
-        Err(hooks::FindHookError::NotFound) => eprintln!("Warning: isolate_post_checkout hook not found. Skipping hook execution."),
-        Err(hooks::FindHookError::NotExecutable(hook_path)) => eprintln!("Warning: hook {} found, but is not executable. Skipping hook execution.", hook_path.to_str().ok_or(IsolateError::PathNotUtf8)?),
+        Err(hooks::FindHookError::NotFound) => {
+          utils::print_warn(color,
+r#"
+  The isolate_post_checkout hook was not found!
+
+  This hook is NOT required but it is strongly recommended that you set it
+  up. It is executed after the temporary isolation branch has been created,
+  the patch cherry-picked into it and the isolation branch checked out.
+
+  It is intended to be used to further verify patch isolation by verifying
+  that your code bases build succeeds and your test suite passes.
+
+  You can effectively have it do whatever you want as it is just a hook.
+  An exit status of 0, success, informs gps that the further isolation
+  verification was successful. Any non-zero exit status will indicate failure
+  and cause gps to abort.
+
+  You can find more information and examples of this hook and others at
+  the following.
+
+  https://github.com/uptech/git-ps-rs#hooks
+"#)
+        },
+        Err(hooks::FindHookError::NotExecutable(hook_path)) => {
+          let path_str = hook_path.to_str().unwrap_or("unknow path");
+          let msg = format!(
+r#"
+  The isolate_post_checkout hook was found at
+
+    {}
+
+  but it is NOT executable. Due to this the hook is being skipped. Generally
+  this can be corrected with the following.
+
+    chmod u+x {}
+"#, path_str, path_str);
+          utils::print_warn(color, &msg);
+        },
         Err(e) => return Err(IsolateError::HookNotFound(e))
       }
 

src/ps/public/request_review.rs

@@ -69,7 +69,7 @@ impl fmt::Display for RequestReviewError {
   }
 }
 
-pub fn request_review(patch_index: usize, given_branch_name: Option<String>) -> Result<(), RequestReviewError> {
+pub fn request_review(patch_index: usize, given_branch_name: Option<String>, color: bool) -> Result<(), RequestReviewError> {
   // check if post_request_review hook exists
   let repo = git::create_cwd_repo().map_err(RequestReviewError::OpenRepositoryFailed)?;
 
@@ -84,7 +84,7 @@ pub fn request_review(patch_index: usize, given_branch_name: Option<String>) ->
   let config = config::get_config(repo_root_str).map_err(RequestReviewError::GetConfigFailed)?;
 
   if config.request_review.verify_isolation {
-    verify_isolation::verify_isolation(patch_index).map_err(RequestReviewError::IsolationVerificationFailed)?;
+    verify_isolation::verify_isolation(patch_index, color).map_err(RequestReviewError::IsolationVerificationFailed)?;
   }
 
   // sync patch up to remote