sq: Unify parse_notations.

3 files changed, 45 insertions, 112 deletions

diff --git a/sq/src/commands/certify.rs b/sq/src/commands/certify.rs
index 7f612f67..b0d6f34a 100644
--- a/sq/src/commands/certify.rs
+++ b/sq/src/commands/certify.rs
@@ -13,6 +13,7 @@ use openpgp::types::SignatureType;
 
 use crate::Config;
 use crate::parse_duration;
+use crate::parse_notations;
 use crate::SECONDS_IN_YEAR;
 use crate::commands::get_certification_keys;
 use crate::commands::GetKeysOptions;
@@ -136,28 +137,14 @@ pub fn certify(config: Config, c: CertifyCommand)
         (Some(_), Some(_)) => unreachable!("conflicting args"),
     }
 
-    // TODO Extract this function, it is used in sign and some revoke commands, too.
-    // Each --notation takes two values.  The iterator returns them
-    // one at a time, however.
-    if let Some(n) = c.notation {
-        let mut n = n.iter();
-        while let Some(name) = n.next() {
-            let value = n.next().unwrap();
-
-            let (critical, name) =
-                if let Some(name) = name.strip_prefix('!') {
-                    (true, name)
-                } else {
-                    (false, name.as_str())
-                };
-
-            builder = builder.add_notation(
-                name,
-                value,
-                NotationDataFlags::empty().set_human_readable(),
-                critical)?;
-        }
-    }
+    let notations = parse_notations(c.notation.unwrap_or_default())?;
+    for (critical, n) in notations {
+        builder = builder.add_notation(
+            n.name(),
+            n.value(),
+            NotationDataFlags::empty().set_human_readable(),
+            critical)?;
+    };
 
     let mut options = Vec::new();
     if c.allow_not_alive_certifier {
diff --git a/sq/src/commands/revoke.rs b/sq/src/commands/revoke.rs
index c6b3207f..5b04b8a6 100644
--- a/sq/src/commands/revoke.rs
+++ b/sq/src/commands/revoke.rs
@@ -7,7 +7,6 @@ use openpgp::cert::prelude::*;
 use openpgp::KeyHandle;
 use openpgp::Packet;
 use openpgp::packet::signature::subpacket::NotationData;
-use openpgp::packet::signature::subpacket::NotationDataFlags;
 use openpgp::packet::UserID;
 use openpgp::parse::Parse;
 use openpgp::policy::NullPolicy;
@@ -20,6 +19,7 @@ use crate::{
     Config,
     load_certs,
     open_or_stdin,
+    parse_notations,
 };
 
 const NP: &NullPolicy = &NullPolicy::new();
@@ -68,29 +68,7 @@ pub fn revoke_certificate(config: Config, c: RevokeCertificateCommand) -> Result
 
     let time = c.time.map(|t| t.time.into());
 
-
-    // Each --notation takes two values.  The iterator
-    // returns them one at a time, however.
-    let mut notations: Vec<(bool, NotationData)> = Vec::new();
-    if let Some(n) = c.notation {
-        let mut n = n.iter();
-        while let Some(name) = n.next() {
-            let value = n.next().unwrap();
-
-            let (critical, name) =
-                if let Some(name) = name.strip_prefix('!') {
-                    (true, name)
-                } else {
-                    (false, name.as_ref())
-                };
-
-            notations.push(
-                (critical,
-                 NotationData::new(
-                     name, value,
-                     NotationDataFlags::empty().set_human_readable())));
-        }
-    }
+    let notations = parse_notations(c.notation.unwrap_or_default())?;
 
     revoke(
         config,
@@ -125,28 +103,7 @@ pub fn revoke_subkey(config: Config, c: RevokeSubkeyCommand) -> Result<()> {
 
     let time = c.time.map(|t| t.time.into());
 
-    // Each --notation takes two values.  The iterator
-    // returns them one at a time, however.
-    let mut notations: Vec<(bool, NotationData)> = Vec::new();
-    if let Some(n) = c.notation {
-        let mut n = n.iter();
-        while let Some(name) = n.next() {
-            let value = n.next().unwrap();
-
-            let (critical, name) =
-                if let Some(name) = name.strip_prefix('!') {
-                    (true, name)
-                } else {
-                    (false, name.as_ref())
-                };
-
-            notations.push(
-                (critical,
-                 NotationData::new(
-                     name, value,
-                     NotationDataFlags::empty().set_human_readable())));
-        }
-    }
+    let notations = parse_notations(c.notation.unwrap_or_default())?;
 
     revoke(
         config,
@@ -173,28 +130,7 @@ pub fn revoke_userid(config: Config, c: RevokeUseridCommand) -> Result<()> {
 
     let time = c.time.map(|t| t.time.into());
 
-    // Each --notation takes two values.  The iterator
-    // returns them one at a time, however.
-    let mut notations: Vec<(bool, NotationData)> = Vec::new();
-    if let Some(n) = c.notation {
-        let mut n = n.iter();
-        while let Some(name) = n.next() {
-            let value = n.next().unwrap();
-
-            let (critical, name) =
-                if let Some(name) = name.strip_prefix('!') {
-                    (true, name)
-                } else {
-                    (false, name.as_ref())
-                };
-
-            notations.push(
-                (critical,
-                 NotationData::new(
-                     name, value,
-                     NotationDataFlags::empty().set_human_readable())));
-        }
-    }
+    let notations = parse_notations(c.notation.unwrap_or_default())?;
 
     revoke(
         config,
diff --git a/sq/src/sq.rs b/sq/src/sq.rs
index 8e5f2d9f..51d4527a 100644
--- a/sq/src/sq.rs
+++ b/sq/src/sq.rs
@@ -483,28 +483,7 @@ fn main() -> Result<()> {
                 load_certs(command.secret_key_file.iter().map(|s| s.as_ref()))?;
             let time = command.time.map(|t| t.time.into());
 
-            // Each --notation takes two values.  The iterator
-            // returns them one at a time, however.
-            let mut notations: Vec<(bool, NotationData)> = Vec::new();
-            if let Some(n) = command.notation {
-                let mut n = n.iter();
-                while let Some(name) = n.next() {
-                    let value = n.next().unwrap();
-
-                    let (critical, name) =
-                        if let Some(name) = name.strip_prefix('!') {
-                            (true, name)
-                        } else {
-                            (false, name.as_str())
-                        };
-
-                    notations.push(
-                        (critical,
-                         NotationData::new(
-                             name, value,
-                             NotationDataFlags::empty().set_human_readable())));
-                }
-            }
+            let notations = parse_notations(command.notation.unwrap_or_default())?;
 
             if let Some(merge) = command.merge {
                 let output = config.create_or_stdout_pgp(output, binary,
@@ -741,6 +720,37 @@ fn main() -> Result<()> {
     Ok(())
 }
 
+fn parse_notations(n: Vec<String>) -> Result<Vec<(bool, NotationData)>> {
+
+    assert_eq!(n.len() % 2, 0);
+
+    // Each --notation takes two values.  The iterator
+    // returns them one at a time, however.
+    let mut notations: Vec<(bool, NotationData)> = Vec::new();
+
+    let mut n = n.iter();
+    while let Some(name) = n.next() {
+        let value = n.next().unwrap();
+
+        let (critical, name) =
+            if let Some(name) = name.strip_prefix('!') {
+                (true, name)
+            } else {
+                (false, name.as_str())
+            };
+
+        notations.push((
+            critical,
+            NotationData::new(
+                name,
+                value,
+                NotationDataFlags::empty().set_human_readable(),
+            ),
+        ));
+    }
+    Ok(notations)
+}
+
 // TODO: Replace all uses with CliTime argument type
 /// Parses the given string depicting a ISO 8601 timestamp.
 fn parse_iso8601(s: &str, pad_date_with: chrono::NaiveTime)