Skip to content

Add mapbox config; first setting persists the update-check opt-out - #35

Merged
mattpodwysocki merged 1 commit into
mainfrom
feat/106-persisted-config
Sep 23, 2026
Merged

mattpodwysocki merged 1 commit into
mainfrom
feat/106-persisted-config

Conversation

@mattpodwysocki

Copy link
Copy Markdown
Contributor

Summary

  • MAPBOX_NO_UPDATE_CHECK=1 only lasts for the shell session it's set in — there was no way to turn the update check off once and have it stay off.
  • Adds mapbox config get|set update-check on|off, persisted to ~/.mapbox/config.json (or $MAPBOX_CONFIG_DIR), written through the same write_private credentials use (0600).
  • update_check::enabled gains a fifth gate alongside the existing four, checked at both call sites (notify() and the detached refresh child) — either the env var or the persisted setting is enough to stop it.
  • get/set take a key restricted to a small table, so a second setting is a new key and match arm rather than a new pair of subcommands.

Test plan

  • cargo build, cargo fmt, cargo clippy --all-targets -- -D warnings, cargo test all clean
  • New unit tests in src/config.rs (default value, round-trip)
  • New tests/config.rs: get/set persistence across separate invocations, unknown key/value is a usage error, file permissions
  • New e2e tests in tests/update_check.rs: the persisted opt-out stops the refresh child and silences the terminal notice, exercised through the real mapbox config set rather than a hand-seeded file
  • docs/commands.md and README updated; CHANGELOG entry added

`MAPBOX_NO_UPDATE_CHECK=1` only lasts for the shell session it's set in —
there was no way to turn the check off once and have it stay off.

`mapbox config get|set update-check on|off` persists to
`~/.mapbox/config.json` (or `$MAPBOX_CONFIG_DIR`), written through the same
`write_private` credentials use, so it gets the same 0600 treatment. The
setting is a fifth gate alongside the existing four in
`update_check::enabled`, checked at both call sites — `notify()` and the
detached refresh child — so either the env var or the persisted setting is
enough to stop it.

`get`/`set` take a `key` restricted to a small table, so a second setting is
a new key and match arm rather than a new pair of subcommands.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@zmofei zmofei left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (non-blocking): in notify(), crate::config::update_check_enabled() is passed as an argument to enabled(). Rust computes all function arguments before the call, so this reads config.json on every run — even when stderr isn't a terminal, or MAPBOX_NO_UPDATE_CHECK is set. Those cases used to cost nothing. Suggestion: make the last parameter of enabled() a closure instead of a plain bool, so the && chain only reads the file when the earlier, cheaper checks already passed.

@zmofei
zmofei added this pull request to stack #41 September 23, 2026 11:49
@zmofei
zmofei removed this pull request from stack #41 September 23, 2026 11:58
@mattpodwysocki
mattpodwysocki merged commit 24473c3 into main Sep 23, 2026
8 checks passed
@mattpodwysocki
mattpodwysocki deleted the feat/106-persisted-config branch September 23, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants