Cargo.toml: update clap version to 4

This changes the organization of the arguments and strictly speaking is an
incompatible change. Before, the device argument could be specified before
--reset-device or --setup-device, but now it must be after the option name.
I think this is an acceptable change, because this isn't really a public
interface and this is the natural way to specify this. This way the setup
is simpler and easier to understand at a glance.

I added #[rustfmt::skip] because the formatter insists on squishing some
arg descriptions into a single line which doesn't look good and also
indents the modifiers after '--setup-device' and 'dir' options differently
for some reason.

Author: keszybz

Cargo.toml

@@ -14,7 +14,7 @@ exclude = ["tests/07a-mount-point-excl", "tests/10-example"]
 
 [dependencies]
 anyhow = "1.0.12"
-clap = { version = "3", default-features = false, features = ["std", "cargo"] }
+clap = { version = "4.5", default-features = false, features = ["std", "cargo", "help" , "usage", "error-context"] }
 liboverdrop = "0.1.0"
 rust-ini = ">=0.15, <0.18"
 log = { version = "0.4", features = ["std"] }

src/main.rs

@@ -6,7 +6,6 @@ mod kernlog;
 mod setup;
 
 use anyhow::Result;
-use clap::{crate_description, crate_name, crate_version, App, Arg};
 use log::{info, LevelFilter};
 use std::borrow::Cow;
 use std::env;
@@ -22,38 +21,36 @@ enum Opts {
     ResetDevice(String),
 }
 
-fn get_opts() -> Opts {
-    let opts = App::new(crate_name!())
-        .version(crate_version!())
-        .about(crate_description!())
+#[rustfmt::skip]
+fn command() -> clap::Command {
+    clap::command!()
+        .arg(
+            clap::arg!(--"setup-device" <device> "Set up a single device")
+                .conflicts_with("reset-device")
+        )
         .arg(
-            Arg::from_usage("--setup-device 'Set up a single device'")
-                .conflicts_with("reset-device"),
+            clap::arg!(--"reset-device" <device> "Reset (destroy) a device")
         )
-        .arg(Arg::from_usage("--reset-device 'Reset (destroy) a device'"))
-        .arg(Arg::from_usage(
-            "<directory|device> 'Target directory for generator or device to operate on'",
-        ))
         .arg(
-            Arg::from_usage(
-                "[extra-dir] 'Unused target directories to satisfy systemd.generator(5)'",
-            )
-            .number_of_values(2)
-            .conflicts_with_all(&["setup-device", "reset-device"]),
+            clap::arg!([dir] "Target directory to write output to and two optional\n\
+                              unused directories to satisfy systemd.generator(5)")
+                .num_args(1..=3)
+                .conflicts_with_all(["setup-device", "reset-device"])
+                .required_unless_present_any(["setup-device", "reset-device"])
         )
-        .after_help(&*format!("Uses {}.", setup::SYSTEMD_MAKEFS_COMMAND))
-        .get_matches();
-
-    let val = opts
-        .value_of("directory|device")
-        .expect("clap invariant")
-        .to_string();
-    if opts.is_present("setup-device") {
-        Opts::SetupDevice(val)
-    } else if opts.is_present("reset-device") {
-        Opts::ResetDevice(val)
+        .after_help(setup::AFTER_HELP)
+}
+
+fn get_opts() -> Opts {
+    let opts = command().get_matches();
+
+    if let Some(val) = opts.get_one::<String>("setup-device") {
+        Opts::SetupDevice(val.clone())
+    } else if let Some(val) = opts.get_one::<String>("reset-device") {
+        Opts::ResetDevice(val.clone())
     } else {
-        Opts::GenerateUnits(val)
+        let val = opts.get_one::<String>("dir").expect("clap invariant");
+        Opts::GenerateUnits(val.clone())
     }
 }
 
@@ -91,3 +88,41 @@ fn main() -> Result<()> {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn verify_app() {
+        command().debug_assert();
+    }
+
+    #[test]
+    fn parse_setup_device() {
+        let m = command().get_matches_from(vec!["prog", "--setup-device", "/dev/zram1"]);
+        assert_eq!(m.get_one::<String>("setup-device").unwrap(), "/dev/zram1");
+    }
+
+    #[test]
+    fn parse_reset_device() {
+        let m = command().get_matches_from(vec!["prog", "--reset-device", "/dev/zram1"]);
+        assert_eq!(m.get_one::<String>("reset-device").unwrap(), "/dev/zram1");
+    }
+
+    #[test]
+    fn parse_with_dir() {
+        let m = command().get_matches_from(vec!["prog", "/dir1"]);
+        assert!(m.get_one::<String>("setup-device").is_none());
+        assert!(m.get_one::<String>("reset-device").is_none());
+        assert_eq!(m.get_one::<String>("dir").unwrap(), "/dir1");
+    }
+
+    #[test]
+    fn parse_with_dirs() {
+        let m = command().get_matches_from(vec!["prog", "/dir1", "/dir2", "/dir3"]);
+        assert!(m.get_one::<String>("setup-device").is_none());
+        assert!(m.get_one::<String>("reset-device").is_none());
+        assert_eq!(m.get_one::<String>("dir").unwrap(), "/dir1");
+    }
+}

src/setup.rs

@@ -18,6 +18,11 @@ pub const SYSTEMD_MAKEFS_COMMAND: &str = concat!(
     ),
     "/systemd-makefs"
 );
+/// A constant string for use in clap --help output.
+#[rustfmt::skip]
+pub const AFTER_HELP: &str = concat!(
+    "Uses ", env!("SYSTEMD_UTIL_DIR"), "/systemd-makefs", "."
+);
 
 pub fn run_device_setup(device: Option<Device>, device_name: &str) -> Result<()> {
     let device = device.ok_or_else(|| anyhow!("Device {} not found", device_name))?;