Some minor cleanup (#220)

* chore: Delegate FromStr and Display to ValueEnum This cleans up some logic for ValueEnum types in the CLI by delegating their Display and FromStr impls entirely to use functions from ValueEnum. Signed-off-by: Andrew Lilley Brinker <[email protected]> * chore: Reordered formatting options to put JSON last I think this should hopefully make the JSON option clearer, as I generally think putting something in the middle makes it easier to miss. Signed-off-by: Andrew Lilley Brinker <[email protected]> --------- Signed-off-by: Andrew Lilley Brinker <[email protected]>
omnibor · Nov 15, 2024 · 79d7b89 · 79d7b89
1 parent f654deb
commit 79d7b89
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 37 deletions.
diff --git a/omnibor-cli/src/cli.rs b/omnibor-cli/src/cli.rs
@@ -1,6 +1,7 @@
 //! Defines the Command Line Interface.
 
 use crate::error::Error;
+use clap::{builder::PossibleValue, ValueEnum};
 use clap_verbosity_flag::{InfoLevel, Verbosity};
 use omnibor::{hashes::Sha256, ArtifactId, IntoArtifactId};
 use pathbuf::pathbuf;
@@ -295,35 +296,39 @@ impl IntoArtifactId<Sha256> for IdentifiableArg {
     }
 }
 
+// Helper macro, generates `Display` and `FromStr` impls for any type that
+// implements `clap::ValueEnum`, delegating to `ValueEnum` functions.
+macro_rules! to_and_from_string {
+    ($name:ident) => {
+        impl Display for $name {
+            fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+                write!(f, "{}", possible_value(self.to_possible_value()))
+            }
+        }
+
+        impl FromStr for $name {
+            type Err = String;
+
+            fn from_str(s: &str) -> Result<Self, Self::Err> {
+                let ignore_case = true;
+                ValueEnum::from_str(s, ignore_case)
+            }
+        }
+    };
+}
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Default, clap::ValueEnum)]
 pub enum Format {
     /// A human-readable plaintext format
     #[default]
     Plain,
-    /// JSON format
-    Json,
     /// Shortest possible format (ideal for piping to other commands)
     Short,
+    /// JSON format
+    Json,
 }
 
-impl Display for Format {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Format::Plain => write!(f, "plain"),
-            Format::Json => write!(f, "json"),
-            Format::Short => write!(f, "short"),
-        }
-    }
-}
-
-impl FromStr for Format {
-    type Err = String;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let ignore_case = true;
-        <Self as clap::ValueEnum>::from_str(s, ignore_case)
-    }
-}
+to_and_from_string!(Format);
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Default, clap::ValueEnum)]
 pub enum SelectedHash {
@@ -332,19 +337,11 @@ pub enum SelectedHash {
     Sha256,
 }
 
-impl Display for SelectedHash {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        match self {
-            SelectedHash::Sha256 => write!(f, "sha256"),
-        }
-    }
-}
-
-impl FromStr for SelectedHash {
-    type Err = String;
+to_and_from_string!(SelectedHash);
 
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        let ignore_case = true;
-        <Self as clap::ValueEnum>::from_str(s, ignore_case)
+fn possible_value(value: Option<PossibleValue>) -> String {
+    match value {
+        Some(value) => value.get_name().to_string(),
+        None => String::from("<skipped>"),
     }
 }
diff --git a/omnibor-cli/tests/snapshots/test__artifact_no_args.snap b/omnibor-cli/tests/snapshots/test__artifact_no_args.snap
@@ -26,7 +26,7 @@ Options:
   -V, --version     Print version
 
 General Flags:
-  -f, --format <FORMAT>  Output format [env: OMNIBOR_FORMAT=] [possible values: plain, json, short]
+  -f, --format <FORMAT>  Output format [env: OMNIBOR_FORMAT=] [possible values: plain, short, json]
   -H, --hash <HASH>      Hash algorithm to use when parsing Artifact IDs [env: OMNIBOR_HASH=] [possible values: sha256]
   -d, --dir <DIR>        Directory to store manifests [env: OMNIBOR_DIR=]
   -c, --config <CONFIG>  Path to a configuration file [env: OMNIBOR_CONFIG=]

diff --git a/omnibor-cli/tests/snapshots/test__debug_no_args.snap b/omnibor-cli/tests/snapshots/test__debug_no_args.snap
@@ -25,7 +25,7 @@ Options:
   -V, --version     Print version
 
 General Flags:
-  -f, --format <FORMAT>  Output format [env: OMNIBOR_FORMAT=] [possible values: plain, json, short]
+  -f, --format <FORMAT>  Output format [env: OMNIBOR_FORMAT=] [possible values: plain, short, json]
   -H, --hash <HASH>      Hash algorithm to use when parsing Artifact IDs [env: OMNIBOR_HASH=] [possible values: sha256]
   -d, --dir <DIR>        Directory to store manifests [env: OMNIBOR_DIR=]
   -c, --config <CONFIG>  Path to a configuration file [env: OMNIBOR_CONFIG=]

diff --git a/omnibor-cli/tests/snapshots/test__manifest_no_args.snap b/omnibor-cli/tests/snapshots/test__manifest_no_args.snap
@@ -27,7 +27,7 @@ Options:
   -V, --version     Print version
 
 General Flags:
-  -f, --format <FORMAT>  Output format [env: OMNIBOR_FORMAT=] [possible values: plain, json, short]
+  -f, --format <FORMAT>  Output format [env: OMNIBOR_FORMAT=] [possible values: plain, short, json]
   -H, --hash <HASH>      Hash algorithm to use when parsing Artifact IDs [env: OMNIBOR_HASH=] [possible values: sha256]
   -d, --dir <DIR>        Directory to store manifests [env: OMNIBOR_DIR=]
   -c, --config <CONFIG>  Path to a configuration file [env: OMNIBOR_CONFIG=]

diff --git a/omnibor-cli/tests/snapshots/test__no_args.snap b/omnibor-cli/tests/snapshots/test__no_args.snap
@@ -26,7 +26,7 @@ Options:
   -V, --version     Print version
 
 General Flags:
-  -f, --format <FORMAT>  Output format [env: OMNIBOR_FORMAT=] [possible values: plain, json, short]
+  -f, --format <FORMAT>  Output format [env: OMNIBOR_FORMAT=] [possible values: plain, short, json]
   -H, --hash <HASH>      Hash algorithm to use when parsing Artifact IDs [env: OMNIBOR_HASH=] [possible values: sha256]
   -d, --dir <DIR>        Directory to store manifests [env: OMNIBOR_DIR=]
   -c, --config <CONFIG>  Path to a configuration file [env: OMNIBOR_CONFIG=]