From 79d7b8949028288dfa69de6ee30aa6130dc24313 Mon Sep 17 00:00:00 2001 From: Andrew Lilley Brinker Date: Fri, 15 Nov 2024 14:47:21 -0800 Subject: [PATCH] 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 * 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 --------- Signed-off-by: Andrew Lilley Brinker --- omnibor-cli/src/cli.rs | 63 +++++++++---------- .../snapshots/test__artifact_no_args.snap | 2 +- .../tests/snapshots/test__debug_no_args.snap | 2 +- .../snapshots/test__manifest_no_args.snap | 2 +- .../tests/snapshots/test__no_args.snap | 2 +- 5 files changed, 34 insertions(+), 37 deletions(-) diff --git a/omnibor-cli/src/cli.rs b/omnibor-cli/src/cli.rs index e926162..acd6a4a 100644 --- 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 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 { + 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 { - let ignore_case = true; - ::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 { - let ignore_case = true; - ::from_str(s, ignore_case) +fn possible_value(value: Option) -> String { + match value { + Some(value) => value.get_name().to_string(), + None => String::from(""), } } diff --git a/omnibor-cli/tests/snapshots/test__artifact_no_args.snap b/omnibor-cli/tests/snapshots/test__artifact_no_args.snap index ff2f992..c6bb8d8 100644 --- 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 Output format [env: OMNIBOR_FORMAT=] [possible values: plain, json, short] + -f, --format Output format [env: OMNIBOR_FORMAT=] [possible values: plain, short, json] -H, --hash Hash algorithm to use when parsing Artifact IDs [env: OMNIBOR_HASH=] [possible values: sha256] -d, --dir Directory to store manifests [env: OMNIBOR_DIR=] -c, --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 index 6c443be..85cc208 100644 --- 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 Output format [env: OMNIBOR_FORMAT=] [possible values: plain, json, short] + -f, --format Output format [env: OMNIBOR_FORMAT=] [possible values: plain, short, json] -H, --hash Hash algorithm to use when parsing Artifact IDs [env: OMNIBOR_HASH=] [possible values: sha256] -d, --dir Directory to store manifests [env: OMNIBOR_DIR=] -c, --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 index b5f7604..160ea30 100644 --- 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 Output format [env: OMNIBOR_FORMAT=] [possible values: plain, json, short] + -f, --format Output format [env: OMNIBOR_FORMAT=] [possible values: plain, short, json] -H, --hash Hash algorithm to use when parsing Artifact IDs [env: OMNIBOR_HASH=] [possible values: sha256] -d, --dir Directory to store manifests [env: OMNIBOR_DIR=] -c, --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 index 279e1f4..970d221 100644 --- 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 Output format [env: OMNIBOR_FORMAT=] [possible values: plain, json, short] + -f, --format Output format [env: OMNIBOR_FORMAT=] [possible values: plain, short, json] -H, --hash Hash algorithm to use when parsing Artifact IDs [env: OMNIBOR_HASH=] [possible values: sha256] -d, --dir Directory to store manifests [env: OMNIBOR_DIR=] -c, --config Path to a configuration file [env: OMNIBOR_CONFIG=]