Skip to content

fix(settings): distinguish unset known settings from unknown ones #9818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jdx merged 3 commits into from
May 12, 2026
+15 −1
Merged

fix(settings): distinguish unset known settings from unknown ones #9818

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36128fe
fix(settings): distinguish unset known settings from unknown ones
jdx May 12, 2026
c393894
fix(settings): also treat namespace prefixes as known settings
jdx May 12, 2026
5186233
test(settings): update test_settings_unset for new error message
jdx May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions e2e/cli/test_settings_set
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ assert_contains "mise settings -T" "all_compile = true"
mise settings unset all_compile
assert "mise settings get all_compile" "false"
assert_fail "mise settings get abcdefg" "mise ERROR Unknown setting: abcdefg"

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If the error message for unknown settings is updated to include brackets for consistency, this test case should be updated to match the new output.

assert_fail "mise settings get abcdefg" "mise ERROR Unknown setting: [abcdefg]"

assert_fail "mise settings get python.compile" "mise ERROR Setting [python.compile] is not set"
assert_fail "mise settings get cargo.registry_name" "mise ERROR Setting [cargo.registry_name] is not set"
Comment on lines 15 to +18

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To fully verify that the logic correctly distinguishes between unset known settings and actual typos (unknown settings), it is recommended to add a test case where a key has a valid prefix but an invalid leaf name (e.g., python.compyle). This ensures that the else block in src/cli/settings/get.rs is correctly reached when the full key is not present in SETTINGS_META.

assert_fail "mise settings get abcdefg" "mise ERROR Unknown setting: abcdefg"
assert_fail "mise settings get python.compyle" "mise ERROR Unknown setting: python.compyle"
assert_fail "mise settings get python.compile" "mise ERROR Setting [python.compile] is not set"
assert_fail "mise settings get cargo.registry_name" "mise ERROR Setting [cargo.registry_name] is not set"

assert "mise settings all_compile" "false"
assert "mise settings all_compile=1"
assert "mise settings all_compile" "true"
Expand Down
3 changes: 3 additions & 0 deletions src/cli/settings/get.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::config;
use crate::config::Settings;
use crate::config::settings::SETTINGS_META;
use eyre::bail;

/// Show a current setting
Expand Down Expand Up @@ -37,6 +38,8 @@ impl SettingsGet {
if let Some(v) = value.as_table().and_then(|t| t.get(k.0)) {
key = k.1;
value = v.clone()
} else if SETTINGS_META.contains_key(self.setting.as_str()) {
bail!("Setting [{}] is not set", self.setting);
} else {
bail!("Unknown setting: {}", self.setting);

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The current check only identifies leaf settings as 'known'. Organizational groups (e.g., cargo, python) will still report as Unknown setting if they contain no set values, which is inconsistent with the goal of distinguishing known settings from typos.

Additionally, the error messages use different formatting styles (brackets vs. no brackets, and different phrasing). It is better to use a consistent style for both types of errors. Using brackets for the key is consistent with other error messages in the codebase.

Suggested change
} else if SETTINGS_META.contains_key(self.setting.as_str()) {
bail!("Setting [{}] is not set", self.setting);
} else {
bail!("Unknown setting: {}", self.setting);
} else if SETTINGS_META.keys().any(|k| k == &self.setting || k.starts_with(&format!("{}.", self.setting))) {
bail!("Setting [{}] is not set", self.setting);
} else {
bail!("Unknown setting: [{}]", self.setting);

}
Expand Down
Loading