From a30799e0934c18dd53b77903219cc9beb867f71e Mon Sep 17 00:00:00 2001
From: Justus Winter <justus@sequoia-pgp.org>
Date: Thu, 24 Oct 2024 12:00:21 +0200
Subject: [PATCH] Use cert designators for `sq key subkey bind`.

  - See #207.
---
 NEWS                        |  3 +++
 src/cli/key/subkey.rs       | 40 ++++++++++++++++++++++++-------------
 src/commands/key/bind.rs    | 22 +++-----------------
 tests/integration/common.rs |  2 +-
 4 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/NEWS b/NEWS
index daa6a176..ef5bfda4 100644
--- a/NEWS
+++ b/NEWS
@@ -147,6 +147,9 @@
      to `--file`.
    - The argument `sq key subkey revoke --file` now requires
      `--output`.
+   - The argument `sq key subkey bind --cert-file` has been renamed to
+     `--file`.
+   - The argument `sq key subkey bind --file` now requires `--output`.
 
 * Changes in 0.38.0
 ** Notable changes
diff --git a/src/cli/key/subkey.rs b/src/cli/key/subkey.rs
index 2ba47592..5233cc54 100644
--- a/src/cli/key/subkey.rs
+++ b/src/cli/key/subkey.rs
@@ -7,7 +7,6 @@ use openpgp::KeyHandle;
 
 use crate::cli::types::ClapData;
 use crate::cli::types::FileOrCertStore;
-use crate::cli::types::FileOrStdin;
 use crate::cli::types::FileOrStdout;
 use crate::cli::types::Time;
 
@@ -829,8 +828,13 @@ respectively.
 #[clap(group(ArgGroup::new("cap-sign").args(&["can_sign", "cannot_sign"])))]
 #[clap(group(ArgGroup::new("cap-authenticate").args(&["can_authenticate", "cannot_authenticate"])))]
 #[clap(group(ArgGroup::new("cap-encrypt").args(&["can_encrypt", "cannot_encrypt"])))]
-#[clap(group(ArgGroup::new("cert_input").args(&["cert_file", "cert"]).required(true)))]
 pub struct BindCommand {
+    #[command(flatten)]
+    pub cert: CertDesignators<CertUserIDEmailFileArgs,
+                              NoPrefix,
+                              OneValueAndFileRequiresOutput,
+                              SubkeyBindDoc>,
+
     #[clap(
         long,
         value_name = "KEY",
@@ -867,18 +871,6 @@ the purpose of signature verification, for example.",
             using broken cryptography",
     )]
     pub allow_broken_crypto: bool,
-    #[clap(
-        long,
-        help = "Add keys to the specified certificate",
-        value_name = "CERT_FILE",
-    )]
-    pub cert: Option<KeyHandle>,
-    #[clap(
-        long,
-        value_name = "CERT_FILE",
-        help = "Add keys to the specified certificate",
-    )]
-    pub cert_file: Option<FileOrStdin>,
 
     #[clap(
         long = "can-sign",
@@ -970,3 +962,23 @@ keys, e.g., keys generated on an OpenPGP card, a TPM device, etc.",
     ]
 };
 test_examples!(sq_key_bind, BIND_EXAMPLES);
+
+/// Documentation for the cert argument of the key subkey bind
+/// command.
+pub struct SubkeyBindDoc {}
+
+impl AdditionalDocs for SubkeyBindDoc {
+    fn help(arg: &'static str, help: &'static str) -> clap::builder::StyledStr {
+        match arg {
+            "file" =>
+                "Add the specified subkeys to the key read from PATH"
+                .into(),
+            _ => {
+                debug_assert!(help.starts_with("Use certificates"));
+                help.replace("Use certificates",
+                             "Add the specified subkeys on the key")
+                    .into()
+            },
+        }
+    }
+}
diff --git a/src/commands/key/bind.rs b/src/commands/key/bind.rs
index 9616d0ee..d7d878fc 100644
--- a/src/commands/key/bind.rs
+++ b/src/commands/key/bind.rs
@@ -18,28 +18,12 @@ use sequoia_openpgp as openpgp;
 use crate::Sq;
 use crate::cli;
 use cli::types::EncryptPurpose;
-use crate::cli::types::FileOrStdout;
-use crate::cli::types::FileStdinOrKeyHandle;
 use crate::common::password;
 
-pub fn bind(sq: Sq, mut command: cli::key::subkey::BindCommand) -> Result<()>
+pub fn bind(sq: Sq, command: cli::key::subkey::BindCommand) -> Result<()>
 {
-    let handle: FileStdinOrKeyHandle = if let Some(file) = command.cert_file {
-        assert!(command.cert.is_none());
-        file.into()
-    } else if let Some(kh) = command.cert {
-        kh.into()
-    } else {
-        panic!("clap enforces --cert or --cert-file is set");
-    };
-
-    if handle.is_file() {
-        if command.output.is_none() {
-            // None means to write to the cert store.  When reading
-            // from a file, we want to write to stdout by default.
-            command.output = Some(FileOrStdout::new(None));
-        }
-    }
+    let handle =
+        sq.resolve_cert(&command.cert, sequoia_wot::FULLY_TRUSTED)?.1;
 
     let cert = sq.lookup_one(handle, None, true)?;
 
diff --git a/tests/integration/common.rs b/tests/integration/common.rs
index fe12641e..6bf17a9d 100644
--- a/tests/integration/common.rs
+++ b/tests/integration/common.rs
@@ -774,7 +774,7 @@ impl Sq {
         }
 
         if target.is_file() {
-            cmd.arg("--cert-file").arg(target);
+            cmd.arg("--file").arg(target);
         } else {
             cmd.arg("--cert").arg(target);
         };