Make --ttl a required argument

This is a breaking change but the default behavior is arbitrary and it's
unlikely most users actually need or want this TTL. Furthermore, making
this change will enable introducing other expiration mechanisms in the
future. More details in #48 and #27.

src/main.rs

@@ -5,6 +5,7 @@ use std::process::{Command, exit, Stdio};
 use std::time::Duration;
 
 use anyhow::{Context, Result};
+use clap::error::{ContextKind, ContextValue, ErrorKind};
 use clap::Parser;
 
 use bkt::{CommandDesc, Bkt};
@@ -136,7 +137,7 @@ struct Cli {
     command: Vec<OsString>,
 
     /// Duration the cached result will be valid for
-    #[arg(long, value_name = "DURATION", default_value = "60s", visible_alias = "time-to-live", env = "BKT_TTL")]
+    #[arg(long, value_name = "DURATION", visible_alias = "time-to-live", env = "BKT_TTL")]
     ttl: humantime::Duration,
 
     /// Duration after which the result will be asynchronously refreshed
@@ -186,7 +187,19 @@ struct Cli {
 }
 
 fn main() {
-    let cli = Cli::parse();
+    // TODO remove this suggestion in 0.9.0
+    let mut cli = Cli::try_parse();
+    if let Err(err) = cli.as_mut() {
+        if matches!(err.kind(), ErrorKind::MissingRequiredArgument) {
+            // https://github.com/clap-rs/clap/discussions/5318
+            err.insert(ContextKind::Suggested, ContextValue::StyledStrs(vec![[
+                "Prior to 0.8.0 --ttl was optional, and defaulted to 60 seconds.",
+                "To preserve this behavior pass `--ttl=1m` or set `BKT_TTL=1m` in your environment."
+            ].join(" ").into()]));
+        }
+        err.exit();
+    }
+    let cli = cli.expect("Not Err");
 
     match run(cli) {
         Ok(code) => exit(code),

tests/cli.rs

@@ -32,6 +32,8 @@ mod cli {
         }
         assert!(path.exists(), "Could not find bkt binary in {:?}", dir);
         let mut bkt = Command::new(&path);
+        // Set a TTL here rather than in every test - tests that care about the TTL should override
+        bkt.env("BKT_TTL", "5s");
         bkt.env("BKT_TMPDIR", cache_dir.as_ref().as_os_str());
         bkt
     }
@@ -116,7 +118,7 @@ mod cli {
     fn help() {
         let dir = TestDir::temp();
         let out = succeed(bkt(dir.path("cache")).arg("--help"));
-        assert!(out.contains("bkt [OPTIONS] -- <COMMAND>..."));
+        assert!(out.contains("bkt [OPTIONS] --ttl <DURATION> -- <COMMAND>..."), "Was:\n---\n{}\n---", out);
     }
 
     #[test]
@@ -142,16 +144,16 @@ mod cli {
         let dir = TestDir::temp();
         let file = dir.path("file");
         let args = ["--", "bash", "-c", COUNT_INVOCATIONS, "arg0", file.to_str().unwrap()];
-        let first_result = succeed(bkt(dir.path("cache")).args(args));
+        let first_result = succeed(bkt(dir.path("cache")).arg("--ttl=1m").args(args));
         assert_eq!(first_result, "1");
 
         // Slightly stale is still cached
         make_dir_stale(dir.path("cache"), Duration::from_secs(10)).unwrap();
-        let subsequent_result = succeed(bkt(dir.path("cache")).args(args));
+        let subsequent_result = succeed(bkt(dir.path("cache")).arg("--ttl=1m").args(args));
         assert_eq!(first_result, subsequent_result);
 
         make_dir_stale(dir.path("cache"), Duration::from_secs(120)).unwrap();
-        let after_stale_result = succeed(bkt(dir.path("cache")).args(args));
+        let after_stale_result = succeed(bkt(dir.path("cache")).arg("--ttl=1m").args(args));
         assert_eq!(after_stale_result, "2");
 
         // Respects BKT_TTL env var (other tests cover --ttl)