Deduplicate and rework the signature notation argument handling.

src/cli/encrypt.rs

@@ -124,20 +124,8 @@ pub struct Command {
                                  SignerDoc>,
 
 
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the signature.",
-        long_help = "Add a notation to the signature.  \
-            A user-defined notation's name must be of the form \
-            `name@a.domain.you.control.org`. If the notation's name starts \
-            with a `!`, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable.",
-    )]
-    pub signature_notations: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 
     #[clap(
         long = "encrypt-for",

src/cli/key/revoke.rs

@@ -105,22 +105,8 @@ future.`",
     )]
     pub message: String,
 
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the revocation",
-        long_help = "\
-Add a notation to the revocation.
-
-A user-defined notation's name must be of the form \
-`name@a.domain.you.control.org`.  If the notation's name starts with a \
-`!`, then the notation is marked as being critical.  If a consumer of \
-a signature doesn't understand a critical notation, then it will \
-ignore the signature.  The notation is marked as being human \
-readable."
-    )]
-    pub notation: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 
     #[clap(
         long,

src/cli/key/subkey/revoke.rs

@@ -132,22 +132,8 @@ created a new subkey, please refresh the certificate.\"",
     )]
     pub message: String,
 
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the revocation.",
-        long_help = "\
-Add a notation to the revocation.
-
-A user-defined notation's name must be of the form \
-`name@a.domain.you.control.org`.  If the notation's name starts with a \
-`!`, then the notation is marked as being critical.  If a consumer of \
-a signature doesn't understand a critical notation, then it will \
-ignore the signature.  The notation is marked as being human \
-readable."
-    )]
-    pub notation: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 
     #[clap(
         long,

src/cli/key/userid.rs

@@ -252,22 +252,8 @@ has left the organization, it might say who to contact instead.",
     )]
     pub message: String,
 
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the certification.",
-        long_help = "\
-Add a notation to the certification.
-
-A user-defined notation's name must be of the form \
-`name@a.domain.you.control.org`.  If the notation's name starts with a \
-`!`, then the notation is marked as being critical.  If a consumer of \
-a signature doesn't understand a critical notation, then it will \
-ignore the signature.  The notation is marked as being human \
-readable."
-    )]
-    pub notation: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 
     #[clap(
         long,

src/cli/pki/link.rs

@@ -222,20 +222,8 @@ to force the signature to be re-created anyway.",
     )]
     pub recreate: bool,
 
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the certification.",
-        long_help = "Add a notation to the certification.  \
-            A user-defined notation's name must be of the form \
-            `name@a.domain.you.control.org`. If the notation's name starts \
-            with a `!`, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable."
-    )]
-    pub notation: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 }
 
 const ADD_EXAMPLES: Actions = Actions {
@@ -436,20 +424,8 @@ to force the signature to be recreated anyway.",
     )]
     pub recreate: bool,
 
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the certification.",
-        long_help = "Add a notation to the certification.  \
-            A user-defined notation's name must be of the form \
-            `name@a.domain.you.control.org`. If the notation's name starts \
-            with a `!`, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable."
-    )]
-    pub notation: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 }
 
 const AUTHORIZE_EXAMPLES: Actions = Actions {
@@ -516,20 +492,8 @@ particular time instead of the current time.
     after_help = RETRACT_EXAMPLES,
 )]
 pub struct RetractCommand {
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the certification.",
-        long_help = "Add a notation to the certification.  \
-            A user-defined notation's name must be of the form \
-            `name@a.domain.you.control.org`. If the notation's name starts \
-            with a !, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable."
-    )]
-    pub notation: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 
     #[clap(
         long = "recreate",

src/cli/pki/vouch/add.rs

@@ -137,20 +137,8 @@ pub struct Command {
     )]
     pub non_revocable: bool,
 
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the certification.",
-        long_help = "Add a notation to the certification.  \
-            A user-defined notation's name must be of the form \
-            `name@a.domain.you.control.org`. If the notation's name starts \
-            with a !, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable."
-    )]
-    pub notation: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 
     #[clap(
         help = FileOrStdout::HELP_OPTIONAL,

src/cli/pki/vouch/authorize.rs

@@ -202,20 +202,8 @@ of power, you have to opt in to this behavior explicitly.",
     #[clap(skip)]
     pub expiration_source: Option<clap::parser::ValueSource>,
 
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the certification.",
-        long_help = "Add a notation to the certification.  \
-            A user-defined notation's name must be of the form \
-            `name@a.domain.you.control.org`. If the notation's name starts \
-            with a !, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable."
-    )]
-    pub notation: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 
     #[clap(
         help = FileOrStdout::HELP_OPTIONAL,

src/cli/sign.rs

@@ -165,24 +165,8 @@ may change line endings.  In doubt, create binary signatures.",
                                  OptionalValue,
                                  SignerDoc>,
 
-    #[clap(
-        long = "signature-notation",
-        value_names = &["NAME", "VALUE"],
-        number_of_values = 2,
-        help = "Add a notation to the signature.",
-        conflicts_with = "merge",
-        long_help = "Add a notation to the signature.  \
-            A user-defined notation's name must be of the form \
-            `name@a.domain.you.control.org`. If the notation's name starts \
-            with a `!`, then the notation is marked as being critical.  If a \
-            consumer of a signature doesn't understand a critical notation, \
-            then it will ignore the signature.  The notation is marked as \
-            being human readable."
-    )]
-    // TODO: Is there a better way to express that one notation consists of two arguments, and
-    // there may be multiple notations? Like something like Vec<(String, String)>.
-    // TODO: Also, no need for the Option
-    pub notation: Vec<String>,
+    #[command(flatten)]
+    pub signature_notations: crate::cli::types::SignatureNotationsArg,
 }
 
 /// Documentation for signer arguments.

src/cli/types.rs

@@ -41,6 +41,8 @@ pub use expiration::Expiration;
 pub use expiration::ExpirationArg;
 pub mod profile;
 pub use profile::Profile;
+pub mod signature_notations;
+pub use signature_notations::SignatureNotationsArg;
 pub mod special_names;
 pub use special_names::SpecialName;
 pub mod time;

src/cli/types/signature_notations.rs

@@ -0,0 +1,58 @@
+//! Common argument for signature notations.
+
+use anyhow::Result;
+
+use sequoia_openpgp::packet::signature::subpacket::{
+    NotationData,
+    NotationDataFlags,
+};
+
+#[derive(Debug, clap::Args)]
+pub struct SignatureNotationsArg {
+    #[clap(
+        long = "signature-notation",
+        value_names = &["NAME", "VALUE"],
+        number_of_values = 2,
+        help = "Add a notation to the signature",
+        long_help = "Add a notation to the signature
+
+A user-defined notation's name must be of the form \
+`name@a.domain.you.control.org`. If the notation's name starts \
+with a `!`, then the notation is marked as being critical.  If a \
+consumer of a signature doesn't understand a critical notation, \
+then it will ignore the signature.  The notation is marked as \
+being human readable.",
+    )]
+    pub signature_notations: Vec<String>,
+}
+
+impl SignatureNotationsArg {
+    /// Parses the notations.
+    pub fn parse(&self) -> Result<Vec<(bool, NotationData)>> {
+	let n = &self.signature_notations;
+	assert_eq!(n.len() % 2, 0, "notations must be pairs of key and value");
+
+	// Each --notation takes two values.  Iterate over them in chunks of 2.
+	let notations: Vec<(bool, NotationData)> = n
+            .chunks(2)
+            .map(|arg_pair| {
+		let name = &arg_pair[0];
+		let value = &arg_pair[1];
+
+		let (critical, name) = match name.strip_prefix('!') {
+                    Some(name) => (true, name),
+                    None => (false, name.as_str()),
+		};
+
+		let notation_data = NotationData::new(
+                    name,
+                    value,
+                    NotationDataFlags::empty().set_human_readable(),
+		);
+		(critical, notation_data)
+            })
+            .collect();
+
+	Ok(notations)
+    }
+}

src/commands/encrypt.rs

@@ -59,8 +59,7 @@ pub fn dispatch(sq: Sq, command: cli::encrypt::Command) -> Result<()> {
                                  sequoia_wot::FULLY_TRUSTED)?;
     let signers = sq.get_signing_keys(&signers, None)?;
 
-    let notations =
-        crate::parse_notations(command.signature_notations)?;
+    let notations = command.signature_notations.parse()?;
 
     if signers.is_empty() && ! notations.is_empty() {
         return Err(anyhow::anyhow!("--signature-notation requires signers, \

src/commands/key/revoke.rs

@@ -10,7 +10,6 @@ use crate::Sq;
 use crate::cli::key::revoke::Command;
 use crate::common::get_secret_signer;
 use crate::common::RevocationOutput;
-use crate::parse_notations;
 
 /// Handle the revocation of a certificate
 struct CertificateRevocation {
@@ -95,7 +94,7 @@ pub fn certificate_revoke(
         Some(sq.resolve_cert(&command.revoker, sequoia_wot::FULLY_TRUSTED)?.0)
     };
 
-    let notations = parse_notations(command.notation)?;
+    let notations = command.signature_notations.parse()?;
 
     let revocation = CertificateRevocation::new(
         &sq,

src/commands/key/subkey/revoke.rs

@@ -16,7 +16,6 @@ use crate::Sq;
 use crate::common::NULL_POLICY;
 use crate::common::RevocationOutput;
 use crate::common::get_secret_signer;
-use crate::parse_notations;
 
 /// Handle the revocation of a subkey
 struct SubkeyRevocation {
@@ -131,7 +130,7 @@ pub fn dispatch(sq: Sq, command: crate::cli::key::subkey::revoke::Command)
         Some(sq.resolve_cert(&command.revoker, sequoia_wot::FULLY_TRUSTED)?.0)
     };
 
-    let notations = parse_notations(command.notation)?;
+    let notations = command.signature_notations.parse()?;
 
     let revocation = SubkeyRevocation::new(
         &sq,

src/commands/key/userid.rs

@@ -33,7 +33,6 @@ use crate::common::userid::{
     lint_names,
     lint_userids,
 };
-use crate::parse_notations;
 
 /// Handle the revocation of a User ID
 struct UserIDRevocation {
@@ -324,7 +323,7 @@ pub fn userid_revoke(
         Some(sq.resolve_cert(&command.revoker, sequoia_wot::FULLY_TRUSTED)?.0)
     };
 
-    let notations = parse_notations(command.notation)?;
+    let notations = command.signature_notations.parse()?;
 
     let revocation = UserIDRevocation::new(
         &sq,

src/commands/pki/link.rs

@@ -16,7 +16,6 @@ use cert_store::{LazyCert, Store};
 use crate::Sq;
 use crate::commands::active_certification;
 use crate::common::NULL_POLICY;
-use crate::parse_notations;
 
 use crate::cli::pki::link;
 use crate::cli::types::Expiration;
@@ -46,7 +45,7 @@ pub fn add(sq: Sq, c: link::AddCommand)
     let vc = cert.with_policy(sq.policy, Some(sq.time))?;
     let userids = c.userids.resolve(&vc)?;
 
-    let notations = parse_notations(c.notation)?;
+    let notations = c.signature_notations.parse()?;
 
     let templates: Vec<(TrustAmount<_>, Expiration)> = if c.temporary {
         // Make the partially trusted link one second younger.  When
@@ -97,7 +96,7 @@ pub fn authorize(sq: Sq, c: link::AuthorizeCommand)
     let vc = cert.with_policy(sq.policy, Some(sq.time))?;
     let userids = c.userids.resolve(&vc)?;
 
-    let notations = parse_notations(c.notation)?;
+    let notations = c.signature_notations.parse()?;
 
     crate::common::pki::certify::certify(
         &mut std::io::stdout(),
@@ -130,7 +129,7 @@ pub fn retract(sq: Sq, c: link::RetractCommand)
     let vc = cert.with_policy(NULL_POLICY, Some(sq.time))?;
     let userids = c.userids.resolve(&vc)?;
 
-    let notations = parse_notations(c.notation)?;
+    let notations = c.signature_notations.parse()?;
 
     crate::common::pki::certify::certify(
         &mut std::io::stdout(),

src/commands/pki/vouch/add.rs

@@ -4,7 +4,6 @@ use openpgp::Result;
 use crate::Sq;
 use crate::cli::pki::vouch::add;
 use crate::commands::FileOrStdout;
-use crate::parse_notations;
 
 pub fn add(sq: Sq, mut c: add::Command)
     -> Result<()>
@@ -24,7 +23,7 @@ pub fn add(sq: Sq, mut c: add::Command)
     let vc = cert.with_policy(sq.policy, Some(sq.time))?;
     let userids = c.userids.resolve(&vc)?;
 
-    let notations = parse_notations(&c.notation)?;
+    let notations = c.signature_notations.parse()?;
     let expiration =
         sq.config.pki_vouch_expiration(&c.expiration, c.expiration_source);
 

src/commands/pki/vouch/authorize.rs

@@ -4,7 +4,6 @@ use openpgp::Result;
 use crate::Sq;
 use crate::cli::pki::vouch::authorize;
 use crate::commands::FileOrStdout;
-use crate::parse_notations;
 
 pub fn authorize(sq: Sq, mut c: authorize::Command)
     -> Result<()>
@@ -24,7 +23,7 @@ pub fn authorize(sq: Sq, mut c: authorize::Command)
     let vc = cert.with_policy(sq.policy, Some(sq.time))?;
     let userids = c.userids.resolve(&vc)?;
 
-    let notations = parse_notations(&c.notation)?;
+    let notations = c.signature_notations.parse()?;
     let expiration =
         sq.config.pki_vouch_expiration(&c.expiration, c.expiration_source);
 

src/commands/sign.rs

@@ -25,7 +25,6 @@ use openpgp::types::SignatureType;
 
 
 use crate::Sq;
-use crate::parse_notations;
 
 use crate::cli;
 use crate::cli::sign::Mode;
@@ -55,7 +54,7 @@ pub fn dispatch(sq: Sq, command: cli::sign::Command) -> Result<()> {
         return Err(anyhow::anyhow!("No signing keys found"));
     }
 
-    let notations = parse_notations(command.notation)?;
+    let notations = command.signature_notations.parse()?;
 
     if let Some(merge) = command.merge {
         let output = output.create_pgp_safe(

src/main.rs

@@ -20,8 +20,6 @@ use sequoia_openpgp as openpgp;
 use openpgp::Result;
 use openpgp::Cert;
 use openpgp::parse::Parse;
-use openpgp::packet::signature::subpacket::NotationData;
-use openpgp::packet::signature::subpacket::NotationDataFlags;
 use openpgp::cert::prelude::*;
 
 use clap::FromArgMatches;
@@ -421,37 +419,6 @@ fn real_main() -> Result<()> {
     }
 }
 
-fn parse_notations<N>(n: N) -> Result<Vec<(bool, NotationData)>>
-where
-    N: AsRef<[String]>,
-{
-    let n = n.as_ref();
-    assert_eq!(n.len() % 2, 0, "notations must be pairs of key and value");
-
-    // Each --notation takes two values.  Iterate over them in chunks of 2.
-    let notations: Vec<(bool, NotationData)> = n
-        .chunks(2)
-        .map(|arg_pair| {
-            let name = &arg_pair[0];
-            let value = &arg_pair[1];
-
-            let (critical, name) = match name.strip_prefix('!') {
-                Some(name) => (true, name),
-                None => (false, name.as_str()),
-            };
-
-            let notation_data = NotationData::new(
-                name,
-                value,
-                NotationDataFlags::empty().set_human_readable(),
-            );
-            (critical, notation_data)
-        })
-        .collect();
-
-    Ok(notations)
-}
-
 // Sometimes the same error cascades, e.g.:
 //
 // ```