fix: accept key=value format in set/add subcommands#8053
Conversation
`mise settings key=value` worked but `mise settings set key=value` did not because the `=` splitting logic only existed in the parent command. Make `value` optional in `SettingsSet` and split on `=` when no separate value argument is provided. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @jdx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Updates mise settings set to accept key=value input format, aligning behavior with the parent settings command and removing the need for a separate <VALUE> argument in that case.
Changes:
- Makes
SettingsSet.valueoptional to allowsettings set key=value. - Adds runtime parsing of
settingfor=whenvalueis not provided. - Updates parent
settingscommand to passvalueasSome(...)intoSettingsSet.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/cli/settings/set.rs | Makes value optional and adds key=value parsing logic in SettingsSet::run. |
| src/cli/settings/mod.rs | Adjusts construction of SettingsSet to use Option<String> for value. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/cli/settings/set.rs
Outdated
| let (key, value) = match self.value { | ||
| Some(v) => (self.setting, v), | ||
| None => { | ||
| let (k, v) = self.setting.split_once('=').ok_or_else(|| { | ||
| eyre!( | ||
| "Usage: mise settings set <KEY>=<VALUE> or mise settings set <KEY> <VALUE>" | ||
| ) | ||
| })?; | ||
| (k.to_string(), v.to_string()) | ||
| } | ||
| }; | ||
| set(&key, &value, false, self.local) |
There was a problem hiding this comment.
When value is provided, setting is accepted verbatim even if it contains =. This allows inputs like mise settings set ruby.compile=false true to proceed and then fail later with Unknown setting: ruby.compile=false (or set an unintended value), rather than producing a clear “unexpected extra argument / ambiguous input” error. Consider explicitly rejecting the combination of setting containing = and value being Some(_) (or alternatively, split setting on = and error if it conflicts with the separate value).
src/cli/settings/set.rs
Outdated
| @@ -17,15 +17,26 @@ pub struct SettingsSet { | |||
| #[clap()] | |||
| pub setting: String, | |||
| /// The value to set | |||
There was a problem hiding this comment.
The field doc comment says “The value to set” but value is now optional and the value may come from setting via key=value. Consider updating the doc/help text to reflect both supported forms (e.g., “The value to set (optional if provided as KEY=VALUE)”) so CLI help output stays accurate.
| /// The value to set | |
| /// The value to set (optional if provided as KEY=VALUE) |
There was a problem hiding this comment.
Code Review
This pull request correctly implements support for the key=value format in the settings set subcommand, enhancing its flexibility. The changes across src/cli/settings/mod.rs and src/cli/settings/set.rs effectively handle both key value and key=value syntaxes. My review includes one suggestion to refactor the argument parsing logic in src/cli/settings/set.rs for better performance and readability, which has been kept as it does not conflict with any provided rules.
src/cli/settings/set.rs
Outdated
| let (key, value) = match self.value { | ||
| Some(v) => (self.setting, v), | ||
| None => { | ||
| let (k, v) = self.setting.split_once('=').ok_or_else(|| { | ||
| eyre!( | ||
| "Usage: mise settings set <KEY>=<VALUE> or mise settings set <KEY> <VALUE>" | ||
| ) | ||
| })?; | ||
| (k.to_string(), v.to_string()) | ||
| } | ||
| }; | ||
| set(&key, &value, false, self.local) |
There was a problem hiding this comment.
The current implementation for parsing the key and value can be made more efficient. In the case of key=value input, it allocates two new strings from slices of an existing string, which is unnecessary.
By moving the set call into the match arms, you can avoid these allocations and make the code more direct. For the key=value case, you can pass string slices directly to set, as they will live long enough.
| let (key, value) = match self.value { | |
| Some(v) => (self.setting, v), | |
| None => { | |
| let (k, v) = self.setting.split_once('=').ok_or_else(|| { | |
| eyre!( | |
| "Usage: mise settings set <KEY>=<VALUE> or mise settings set <KEY> <VALUE>" | |
| ) | |
| })?; | |
| (k.to_string(), v.to_string()) | |
| } | |
| }; | |
| set(&key, &value, false, self.local) | |
| match self.value { | |
| Some(value) => set(&self.setting, &value, false, self.local), | |
| None => { | |
| let (key, value) = self.setting.split_once('=').ok_or_else(|| { | |
| eyre!( | |
| "Usage: mise settings set <KEY>=<VALUE> or mise settings set <KEY> <VALUE>" | |
| ) | |
| })?; | |
| set(key, value, false, self.local) | |
| } | |
| } |
`mise settings key=value` worked but `mise settings set key=value` did not because the `=` splitting logic only existed in parent commands. Apply the same fix to all set/add subcommands: - `mise settings set key=value` - `mise settings add key=value` - `mise config set key=value` - `mise shell-alias set alias=command` Make `value` optional in each and split on `=` when no separate value argument is provided. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
settings set subcommandCover the new key=value parsing in: - settings set - settings add - config set - shell-alias set Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
bugbot run |
- Fix variable shadowing bug in config/set.rs where loop variable
`key` shadowed the outer `key`, breaking `starts_with("tools.")`
check (rename outer to `full_key`, loop var to `part`)
- Avoid unnecessary string allocations in settings set/add by calling
`set()` directly in each match arm with borrowed slices
- Update doc comments for optional value fields
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
bugbot run |
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.7 x -- echo |
20.8 ± 0.5 | 20.1 | 28.5 | 1.00 |
mise x -- echo |
21.0 ± 0.4 | 20.3 | 24.0 | 1.01 ± 0.03 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.7 env |
20.2 ± 0.6 | 19.6 | 25.9 | 1.00 |
mise env |
20.3 ± 0.3 | 19.7 | 21.9 | 1.00 ± 0.04 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.7 hook-env |
21.0 ± 0.3 | 20.4 | 22.5 | 1.00 |
mise hook-env |
21.3 ± 0.5 | 20.3 | 24.8 | 1.01 ± 0.03 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.7 ls |
18.9 ± 0.4 | 18.3 | 20.6 | 1.00 |
mise ls |
19.1 ± 0.4 | 18.5 | 21.2 | 1.01 ± 0.03 |
xtasks/test/perf
| Command | mise-2026.2.7 | mise | Variance |
|---|---|---|---|
| install (cached) | 119ms | 121ms | -1% |
| ls (cached) | 71ms | 72ms | -1% |
| bin-paths (cached) | 76ms | 75ms | +1% |
| task-ls (cached) | 536ms | 536ms | +0% |
### 🚀 Features - **(node)** support package.json as idiomatic version file by @jdx in [#8059](#8059) - **(ruby)** graduate precompiled ruby from experimental (gradual rollout) by @jdx in [#8052](#8052) - add --dry-run-code flag to exit non-zero when there is work to do by @jdx in [#8063](#8063) ### 🐛 Bug Fixes - **(core)** respect MISE_ARCH override in bun and erlang plugins by @jdx in [#8062](#8062) - **(hooks)** resolve 12 community-reported hooks issues by @jdx in [#8058](#8058) - accept key=value format in set/add subcommands by @jdx in [#8053](#8053) ### 📚 Documentation - bump action versions in GitHub Actions examples by @muzimuzhi in [#8065](#8065) - add opengraph meta tags by @jdx in [#8066](#8066) ### 📦️ Dependency Updates - upgrade toml to 0.9 and toml_edit to 0.24 (TOML 1.1) by @jdx in [#8057](#8057) ### 📦 Registry - add quicktype (npm:quicktype) by @zdunecki in [#8054](#8054) - use inline table for test definitions by @jdx in [#8056](#8056)
## Summary - `mise settings set key=value`, `mise settings add key=value`, `mise config set key=value`, and `mise shell-alias set alias=command` now all work - Previously only parent commands (like `mise settings`) split on `=`, so subcommands like `mise settings set ruby.compile=false` would fail with a missing `<VALUE>` argument - Makes `value` optional in each subcommand and splits the first arg on `=` when no separate value is provided ## Changed files - `src/cli/settings/set.rs` — `SettingsSet.value` now `Option<String>` - `src/cli/settings/add.rs` — `SettingsAdd.value` now `Option<String>` - `src/cli/config/set.rs` — `ConfigSet.value` now `Option<String>` - `src/cli/shell_alias/set.rs` — `ShellAliasSet.command` now `Option<String>` - `src/cli/settings/mod.rs` — wrap value in `Some(...)` at construction sites ## Test plan - [x] `mise settings set ruby.compile=true` — succeeds - [x] `mise settings set ruby.compile true` — still works - [x] `mise settings set ruby.compile` — errors with usage hint - [x] `mise settings add disable_hints=python_multi` — succeeds - [x] `mise settings add disable_hints python_multi` — still works - [x] `mise shell-alias set ll="ls -la"` — succeeds - [x] `mise shell-alias set ll` — errors with usage hint - [x] Lint passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small, localized CLI argument-parsing change with added e2e coverage and doc/spec updates; main risk is minor behavior differences around edge cases in `=` parsing. > > **Overview** > `mise config set`, `mise settings set`, `mise settings add`, and `mise shell-alias set` now accept a single `KEY=VALUE`/`ALIAS=COMMAND` argument in addition to the existing two-argument form by making the value parameter optional and splitting on `=` when omitted (with clearer usage errors when neither is provided). > > Updates the CLI spec (`mise.usage.kdl`), generated docs/manpage, Fig completion metadata, and adds e2e coverage for the new shorthand plus failure cases when no value is supplied. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5e28b08. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
### 🚀 Features - **(node)** support package.json as idiomatic version file by @jdx in [jdx#8059](jdx#8059) - **(ruby)** graduate precompiled ruby from experimental (gradual rollout) by @jdx in [jdx#8052](jdx#8052) - add --dry-run-code flag to exit non-zero when there is work to do by @jdx in [jdx#8063](jdx#8063) ### 🐛 Bug Fixes - **(core)** respect MISE_ARCH override in bun and erlang plugins by @jdx in [jdx#8062](jdx#8062) - **(hooks)** resolve 12 community-reported hooks issues by @jdx in [jdx#8058](jdx#8058) - accept key=value format in set/add subcommands by @jdx in [jdx#8053](jdx#8053) ### 📚 Documentation - bump action versions in GitHub Actions examples by @muzimuzhi in [jdx#8065](jdx#8065) - add opengraph meta tags by @jdx in [jdx#8066](jdx#8066) ### 📦️ Dependency Updates - upgrade toml to 0.9 and toml_edit to 0.24 (TOML 1.1) by @jdx in [jdx#8057](jdx#8057) ### 📦 Registry - add quicktype (npm:quicktype) by @zdunecki in [jdx#8054](jdx#8054) - use inline table for test definitions by @jdx in [jdx#8056](jdx#8056)
Summary
mise settings set key=value,mise settings add key=value,mise config set key=value, andmise shell-alias set alias=commandnow all workmise settings) split on=, so subcommands likemise settings set ruby.compile=falsewould fail with a missing<VALUE>argumentvalueoptional in each subcommand and splits the first arg on=when no separate value is providedChanged files
src/cli/settings/set.rs—SettingsSet.valuenowOption<String>src/cli/settings/add.rs—SettingsAdd.valuenowOption<String>src/cli/config/set.rs—ConfigSet.valuenowOption<String>src/cli/shell_alias/set.rs—ShellAliasSet.commandnowOption<String>src/cli/settings/mod.rs— wrap value inSome(...)at construction sitesTest plan
mise settings set ruby.compile=true— succeedsmise settings set ruby.compile true— still worksmise settings set ruby.compile— errors with usage hintmise settings add disable_hints=python_multi— succeedsmise settings add disable_hints python_multi— still worksmise shell-alias set ll="ls -la"— succeedsmise shell-alias set ll— errors with usage hint🤖 Generated with Claude Code
Note
Low Risk
Small, localized CLI argument-parsing change with added e2e coverage and doc/spec updates; main risk is minor behavior differences around edge cases in
=parsing.Overview
mise config set,mise settings set,mise settings add, andmise shell-alias setnow accept a singleKEY=VALUE/ALIAS=COMMANDargument in addition to the existing two-argument form by making the value parameter optional and splitting on=when omitted (with clearer usage errors when neither is provided).Updates the CLI spec (
mise.usage.kdl), generated docs/manpage, Fig completion metadata, and adds e2e coverage for the new shorthand plus failure cases when no value is supplied.Written by Cursor Bugbot for commit 5e28b08. This will update automatically on new commits. Configure here.