Repository navigation
fix(mctx): keep inline-table settings when adding deps - #2244
norrietaylor merged 3 commits into
Conversation
add_deps looked up [stack], tasks.<name> and [session] with
as_table_mut, which returns None for TOML inline tables. An
inline session = { ... } was therefore replaced wholesale by a
fresh [session] holding only packages, silently dropping vars,
patches and lifecycle hooks, while inline [stack] and task
tables made the command error out with "could not find".
Use as_table_like_mut so both explicit and inline table forms
are found, and only insert a fresh [session] when the key is
absent. The upsert helper takes ?Sized TableLike for the dyn
returns. Pinned by a test that adds deps through all inline
forms and asserts sibling keys survive the write-back.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Warning Review limit reached
This review includes 1 billable file and costs up to $0.25. Or wait 2 minutes for your next included review. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title is concise, follows the required Conventional Commit format, and clearly describes preserving inline-table settings when adding dependencies. |
| Description check | |
The description explains the problem, implementation, affected modes, and verification results. It provides the required summary and testing information, although it does not include the template's Ch… |
Full details: Description check
-
Explanation
The description explains the problem, implementation, affected modes, and verification results. It provides the required summary and testing information, although it does not include the template's Checklist section.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks each table row
Inline settings join the flow
Sibling values stay in place
Session settings load with grace
Packages hop to their new home
Comment @coderabbitai help to get the list of available commands.
The task and session checks matched the earlier build_packages substring, so they passed without the fix. Parse the written TOML and check each table's own key, that each table stays inline, the RuntimePackages arm, and that a missing task still errors.
|
/review |
|
Approved: no findings at Nothing to do. Clobber: none: |
|
/review |
There was a problem hiding this comment.
Approved on ARCH PASS at 51ae830.
There was a problem hiding this comment.
Approved on ARCH PASS at 51ae830.
There was a problem hiding this comment.
Approved on ARCH PASS at 51ae830.
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABX3vQAA
Problem
min add --session(and the stack/task variants) resolved its targettable with
as_table_mut(), which returnsNonefor TOML inlinetables. Consequences: an inline
session = { ... }was replacedwholesale by a fresh
[session]containing onlypackages, silentlydropping
vars,patchesandlifecycle_hooks; inline[stack]andtasks.<name>tables made the command fail with "could not find".Change
Context::add_depsnow usesas_table_like_mut(), which matches bothexplicit and inline table forms, so a fresh
[session]is onlyinserted when the key is absent. The upsert helper's bound becomes
TableLike + ?Sizedto accept the trait-object returns. All fourAddDepModearms are covered.Verification
cargo test --locked -p mctx add_depsfailed on the unmodified treewith the inline-stack symptom (
could not find [stack]), passesafter the fix (behavioral red/green pin in
add_deps_inline_tables,which asserts sibling keys —
use,exec,vars.ANSWER— survivethe write-back and re-parse into the config).
cargo fmt --all --check— clean.cargo clippy --workspace --all-targets --locked -- -D warnings— clean.cargo test --locked -p mctx— 47 passed, 0 failed.Summary by CodeRabbit