Add mapbox config; first setting persists the update-check opt-out - #35
Merged
Merged
Conversation
`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>
This was referenced Sep 23, 2026
zmofei
approved these changes
Sep 23, 2026
zmofei
left a comment
Member
There was a problem hiding this comment.
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
added this pull request to stack #41
September 23, 2026 11:49
zmofei
removed this pull request from stack #41
September 23, 2026 11:58
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MAPBOX_NO_UPDATE_CHECK=1only 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.mapbox config get|set update-check on|off, persisted to~/.mapbox/config.json(or$MAPBOX_CONFIG_DIR), written through the samewrite_privatecredentials use (0600).update_check::enabledgains 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/settake akeyrestricted 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 testall cleansrc/config.rs(default value, round-trip)tests/config.rs: get/set persistence across separate invocations, unknown key/value is a usage error, file permissionstests/update_check.rs: the persisted opt-out stops the refresh child and silences the terminal notice, exercised through the realmapbox config setrather than a hand-seeded filedocs/commands.mdand README updated; CHANGELOG entry added