You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The record subcommand's default output path is documented inconsistently across --help, the source doc comments, and the README, and the effective path is resolved through a brittle string-sentinel mechanism. The cache base is also a hard-coded ~/.cache/... string that ignores platform conventions on Windows and macOS.
This was surfaced by #210 (@victorhooi), which correctly fixed the README table but left the deeper inconsistencies untouched. The CLA on that PR is unsigned, so the fix is re-implemented here, incorporating the README correction with credit to the original reporter.
Current behavior
Effective default (no -o, no config file): ~/.cache/all-smi/records/all-smi-record.ndjson.zst. Settings::default() always sets record.output_dir = "~/.cache/all-smi/records" (src/common/config_file.rs), and RecorderOptions::from_args_with_settings joins the default basename under it even when no config file is present (src/record/mod.rs).
all-smi record --help advertises the --output default as all-smi-record.ndjson.zst — i.e. the current working directory (src/cli.rs).
The source doc comment says the compiled default is ./all-smi-record.ndjson.zst (src/record/mod.rs).
Docs disagree — all-smi record --help, where most users look first, contradicts the README and the actual behavior.
Brittle resolution + latent wart — "operator did not pass -o" is detected by string-matching args.output against the magic basename (src/record/mod.rs); the in-code comment itself flags this as fragile ("if that ever changes, update this guard"). As a side effect, passing -o all-smi-record.ndjson.zst explicitly is silently redirected into the cache dir instead of the current directory.
Not platform-aware — ~/.cache/... is a hard-coded string expanded via dirs::home_dir(), not the platform-aware dirs::cache_dir(). So on Windows it lands at C:\Users\<you>\.cache\all-smi\records\... (a Unix-style dotdir in the profile) and on macOS at ~/.cache/... rather than ~/Library/Caches/.... By contrast the config path uses dirs::config_dir() (%APPDATA%\all-smi on Windows), so the codebase is internally inconsistent.
Proposed solution
Scope A — core fix (low-risk, no change to where files land):
Make RecordArgs::output an Option<PathBuf> (drop the clap default_value); resolve the effective default explicitly in from_args_with_settings (None → <output_dir>/<DEFAULT_BASENAME>, Some(p) → honored verbatim). This removes the string-sentinel fragility and the explicit--o redirect wart.
Introduce a single shared DEFAULT_BASENAME constant (dedupe the literal currently in src/cli.rs and src/record/mod.rs).
Scope B — platform-correct cache location (decision needed; may be a separate issue):
B1: derive the cache base from dirs::cache_dir() so Windows → %LOCALAPPDATA%\all-smi\records, macOS → ~/Library/Caches/all-smi/records, Linux → $XDG_CACHE_HOME/all-smi/records. Apply consistently to the energy WAL and users CSV as well. Note: this moves existing files on macOS/Windows (and Linux when $XDG_CACHE_HOME is set) and requires a migration/changelog note.
B2: keep ~/.cache/... deliberately for cross-platform consistency, but document the rationale and the platform expansion explicitly.
Recommendation: land Scope A first; treat Scope B (especially B1, a behavior change) as a separate decision/PR to keep changes focused. Either way, the "config is platform-aware but cache is hard-coded ~/.cache" inconsistency is worth reconciling.
Affected files
Scope A: src/cli.rs, src/record/mod.rs, README.md
Scope B: src/common/config_file.rs, src/common/paths.rs, and for consistency src/metrics/energy_wal.rs, src/view/event_handler.rs
Acceptance criteria
all-smi record --help, the source doc comment, and the README all state the same default output path.
Default-path resolution no longer relies on string-matching the basename; an explicit -o <path> is always honored verbatim, including when it equals the default basename.
DEFAULT_BASENAME is defined once and shared between the CLI arg and the resolver.
(Scope B, if chosen) the cache location is platform-correct via dirs::cache_dir(), applied consistently across record output / energy WAL / users CSV, with a migration/changelog note.
Tests cover: no -o + no config; no -o + config output_dir set; explicit -o equal to the default basename; explicit -o absolute path.
Summary
The
recordsubcommand's default output path is documented inconsistently across--help, the source doc comments, and the README, and the effective path is resolved through a brittle string-sentinel mechanism. The cache base is also a hard-coded~/.cache/...string that ignores platform conventions on Windows and macOS.This was surfaced by #210 (@victorhooi), which correctly fixed the README table but left the deeper inconsistencies untouched. The CLA on that PR is unsigned, so the fix is re-implemented here, incorporating the README correction with credit to the original reporter.
Current behavior
-o, no config file):~/.cache/all-smi/records/all-smi-record.ndjson.zst.Settings::default()always setsrecord.output_dir = "~/.cache/all-smi/records"(src/common/config_file.rs), andRecorderOptions::from_args_with_settingsjoins the default basename under it even when no config file is present (src/record/mod.rs).all-smi record --helpadvertises the--outputdefault asall-smi-record.ndjson.zst— i.e. the current working directory (src/cli.rs)../all-smi-record.ndjson.zst(src/record/mod.rs).~/.cache/....Problems
all-smi record --help, where most users look first, contradicts the README and the actual behavior.-o" is detected by string-matchingargs.outputagainst the magic basename (src/record/mod.rs); the in-code comment itself flags this as fragile ("if that ever changes, update this guard"). As a side effect, passing-o all-smi-record.ndjson.zstexplicitly is silently redirected into the cache dir instead of the current directory.~/.cache/...is a hard-coded string expanded viadirs::home_dir(), not the platform-awaredirs::cache_dir(). So on Windows it lands atC:\Users\<you>\.cache\all-smi\records\...(a Unix-style dotdir in the profile) and on macOS at~/.cache/...rather than~/Library/Caches/.... By contrast the config path usesdirs::config_dir()(%APPDATA%\all-smion Windows), so the codebase is internally inconsistent.Proposed solution
Scope A — core fix (low-risk, no change to where files land):
RecordArgs::outputanOption<PathBuf>(drop the clapdefault_value); resolve the effective default explicitly infrom_args_with_settings(None→<output_dir>/<DEFAULT_BASENAME>,Some(p)→ honored verbatim). This removes the string-sentinel fragility and the explicit--oredirect wart.DEFAULT_BASENAMEconstant (dedupe the literal currently insrc/cli.rsandsrc/record/mod.rs).long_help, themod.rsdoc comment, and the README (fold in Update default output path in README.md #210's README correction, credit @victorhooi).Scope B — platform-correct cache location (decision needed; may be a separate issue):
dirs::cache_dir()so Windows →%LOCALAPPDATA%\all-smi\records, macOS →~/Library/Caches/all-smi/records, Linux →$XDG_CACHE_HOME/all-smi/records. Apply consistently to the energy WAL and users CSV as well. Note: this moves existing files on macOS/Windows (and Linux when$XDG_CACHE_HOMEis set) and requires a migration/changelog note.~/.cache/...deliberately for cross-platform consistency, but document the rationale and the platform expansion explicitly.Recommendation: land Scope A first; treat Scope B (especially B1, a behavior change) as a separate decision/PR to keep changes focused. Either way, the "config is platform-aware but cache is hard-coded
~/.cache" inconsistency is worth reconciling.Affected files
src/cli.rs,src/record/mod.rs,README.mdsrc/common/config_file.rs,src/common/paths.rs, and for consistencysrc/metrics/energy_wal.rs,src/view/event_handler.rsAcceptance criteria
all-smi record --help, the source doc comment, and the README all state the same default output path.-o <path>is always honored verbatim, including when it equals the default basename.DEFAULT_BASENAMEis defined once and shared between the CLI arg and the resolver.dirs::cache_dir(), applied consistently across record output / energy WAL / users CSV, with a migration/changelog note.-o+ no config; no-o+ configoutput_dirset; explicit-oequal to the default basename; explicit-oabsolute path.References