aboutsummaryrefslogtreecommitdiffstats
path: root/crates
diff options
context:
space:
mode:
authorKarl Ding <karlding@users.noreply.github.com>2025-10-20 13:07:41 -0700
committerGitHub <noreply@github.com>2025-10-20 13:07:41 -0700
commitaa9e5ad1d55cf6b82127f080be0f30a0c2c9f7e7 (patch)
tree90937fc94994d595c2991f84e1e4cc5213da69ae /crates
parentfeat: Interactive Inspector (#2319) (diff)
downloadatuin-aa9e5ad1d55cf6b82127f080be0f30a0c2c9f7e7.zip
fix: stats ngram window size cli parsing (#2946)
Fix panic when running "atuin stats" with an empty window size: $ RUST_BACKTRACE=1 atuin stats year -n0 thread 'main' panicked at crates/atuin-history/src/stats.rs:278:14: window size must be non-zero stack backtrace: 0: __rustc::rust_begin_unwind 1: core::panicking::panic_fmt 2: core::option::expect_failed 3: atuin_history::stats::compute 4: atuin::command::client::Cmd::run_inner::{{closure}} 5: tokio::runtime::context::scoped::Scoped<T>::set 6: tokio::runtime::scheduler::current_thread::CoreGuard::block_on 7: tokio::runtime::context::runtime::enter_runtime 8: atuin::command::AtuinCmd::run 9: atuin::main note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. The ngram window size had a default value of 1 (indicating that perhaps this case was considered), but did not validate the parsed value when a non-default value was given. Such validation can either occur: 1. during clap argument parsing 2. after clap parses arguments, but before computing statistics If we were to do this after clap finishes parsing arguments, this would be a runtime check in run(), but it seems better to return an error as soon as possible. It's possible to do this without a custom parsing function via clap::value_parser! and specifying a range: clap::value_parser!(u64).range(1..) However, this does not appear to support custom error messages so we would be left with the default message specifying an invalid range. Therefore, add a custom parser function and return an error when the value cannot be parsed as a valid ngram value. <!-- Thank you for making a PR! Bug fixes are always welcome, but if you're adding a new feature or changing an existing one, we'd really appreciate if you open an issue, post on the forum, or drop in on Discord --> ## Checks - [x] I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle - [x] I have checked that there are no existing pull requests for the same thing
Diffstat (limited to 'crates')
-rw-r--r--crates/atuin/src/command/client/stats.rs14
1 files changed, 13 insertions, 1 deletions
diff --git a/crates/atuin/src/command/client/stats.rs b/crates/atuin/src/command/client/stats.rs
index daffdd84..3ee60633 100644
--- a/crates/atuin/src/command/client/stats.rs
+++ b/crates/atuin/src/command/client/stats.rs
@@ -11,6 +11,18 @@ use atuin_client::{
use atuin_history::stats::{compute, pretty_print};
+fn parse_ngram_size(s: &str) -> Result<usize, String> {
+ let value = s
+ .parse::<usize>()
+ .map_err(|_| format!("'{s}' is not a valid window size"))?;
+
+ if value == 0 {
+ return Err("ngram window size must be at least 1".to_string());
+ }
+
+ Ok(value)
+}
+
#[derive(Parser, Debug)]
#[command(infer_subcommands = true)]
pub struct Cmd {
@@ -22,7 +34,7 @@ pub struct Cmd {
count: usize,
/// The number of consecutive commands to consider
- #[arg(long, short, default_value = "1")]
+ #[arg(long, short, default_value = "1", value_parser = parse_ngram_size)]
ngram_size: usize,
}