From 633531f6c9b0dc1349741e09cd212415731c7812 Mon Sep 17 00:00:00 2001
From: Adrian Moreno <amorenoz@redhat.com>
Date: Mon, 25 Sep 2023 14:48:32 +0200
Subject: [PATCH] cli: return errors when building from strings

Fixes: #259
build_from() should only be used directly by unit tests.
It would be private if it wasn't needed in other modules for testing
purposes.

It always returns an error. However, build(), which is called by main(),
exits the program based on that error using clap.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 src/cli/cli.rs | 22 ++++++++++------------
 src/main.rs    |  2 +-
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/src/cli/cli.rs b/src/cli/cli.rs
index 939e53363..7f33e7bbd 100644
--- a/src/cli/cli.rs
+++ b/src/cli/cli.rs
@@ -242,11 +242,14 @@ impl ThinCli {
 
     /// Build a FullCli by running a first round of CLI parsing without subcommand argument
     /// validation.
-    pub(crate) fn build(self) -> Result<FullCli, ClapError> {
-        self.build_from(env::args_os())
+    /// If clap reports an error (including "--help" and "--version"), print the message and
+    /// exit the program.
+    pub(crate) fn build(self) -> FullCli {
+        self.build_from(env::args_os()).unwrap_or_else(|e| e.exit())
     }
 
     /// Build a FullCli by running a first round of CLI parsing with the given list of arguments.
+    /// This function should be only used directly by unit tests.
     pub(crate) fn build_from<I, T>(mut self, args: I) -> Result<FullCli, ClapError>
     where
         I: IntoIterator<Item = T>,
@@ -272,7 +275,7 @@ impl ThinCli {
         let matches = command
             .clone()
             .ignore_errors(true)
-            .get_matches_from(args.iter());
+            .try_get_matches_from(args.iter())?;
 
         let ran_subcommand = matches.subcommand_name();
 
@@ -282,16 +285,11 @@ impl ThinCli {
                 .get(&ran_subcommand.unwrap().to_string())
                 .is_none()
         {
-            // There is no subcommand or it's invalid. Let clap handle this error since it prints
-            // nicer error messages and knows where they should be printed to.
-            let err = command
+            // There is no subcommand or it's invalid. Re-run the match to generate
+            // the right clap error that to be printed nicely.
+            return Err(command
                 .try_get_matches_from_mut(args.iter())
-                .expect_err("clap should fail with no arguments");
-
-            match cfg!(test) {
-                true => return Err(err),
-                false => err.exit(),
-            };
+                .expect_err("clap should fail with no arguments"));
         }
 
         // Get main config.
diff --git a/src/main.rs b/src/main.rs
index fef26ba68..f56185280 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -17,7 +17,7 @@ use retis_derive::*;
 const VERSION_NAME: &str = "pizza margherita";
 
 fn main() -> Result<()> {
-    let mut cli = get_cli()?.build()?;
+    let mut cli = get_cli()?.build();
 
     let log_level = match cli.main_config.log_level.as_str() {
         "error" => LevelFilter::Error,