設定ファイルの項目を CLI から打ち消せるようにする (設定の可逆化)#48
Merged
Merged
Conversation
…celled `embed` was OR-merged with the config, so `embed = true` in .risundlerc.toml was irreversible from the CLI (#24). Treat the pair as an Option<bool> resolved last-wins, preferring an explicit CLI flag over the config over the builtin default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A single -k used to replace the default keep = ["std"] wholesale, so forgetting -k std silently expanded the entire standard library (#24). The effective set is now (config keep ∪ --keep) − --no-keep; --no-keep wins over --keep regardless of order, erring toward the self-contained (expanded) output. --no-keep std restores the ability to empty the set from the CLI, which previously required the accidental -k '' trick. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Forgetting "std" in the config's keep (or --no-keep'ing it by accident elsewhere) silently emits a huge bundle that only surfaces at submission size limits. Warn on absence, but stay quiet when the CLI passed --no-keep std explicitly: absence is guarded, explicit intent is respected (#24). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eplacing Passing any option after -- used to discard the whole configured list, so `-- -O0` also dropped -std=gnu++17 (#24). Append the CLI options and delegate override semantics to the compiler's own last-wins rules (-std=... beats an earlier one, -U kills -D); risundle does not interpret flags itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe bundling CLI now supports reversible keep, embed, and configuration overrides. Settings resolution merges CLI and config values by field type, warns when ChangesConfiguration resolution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Bundle
participant Settings
participant Config
CLI->>Bundle: Parse override flags
Bundle->>Settings: Resolve CLI and config inputs
Settings->>Config: Load configuration unless --no-config
Config-->>Settings: Return config values
Settings-->>Bundle: Return effective settings
Bundle->>Bundle: Warn if std is not kept
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Appending cannot cancel every kind of configured flag (a config -I, for example), so provide one cross-cutting escape hatch defined as: behave exactly as if no config file were found. This keeps the effective settings reachable from builtin defaults via the CLI alone, for any future config key (#24). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document the #24 semantics across README, cheatsheet, and spec (en/ja): --keep is now additive with --no-keep as its inverse, CLI compiler options append to the configured ones, --no-embed cancels a configured embed, and --no-config behaves exactly as if no config file were found. The spec also records the std-missing warning and its suppression, and the cheatsheet FAQ about -k dropping std is reworked into the config keep footgun it has become. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
"pre-tree-shaking code" could be read as the preprocessed-but-unshaken output; the attachment is the untouched text of the input file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TwoSquirrels
changed the title
There was a problem hiding this comment.
Pull request overview
This PR addresses non-reversible configuration layering by ensuring every config-file setting can be explicitly overridden (or negated) from the CLI, with merge semantics defined per setting type (scalar/bool/set/list). It introduces new CLI flags and updates resolution logic, tests, and documentation accordingly.
Changes:
- Add reversible CLI overrides:
--no-embed, additive--keepplus subtractive--no-keep, and global escape hatch--no-config. - Change compiler options layering so CLI options after
--are appended to configured options (instead of replacing). - Add warnings when
stdis missing from effectivekeep(suppressed when explicitly removed via--no-keep std) and update specs/cheatsheets.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/commands/bundle.rs | Updates settings resolution to support --no-config, additive/subtractive keep, appended options, and std-missing warning. |
| src/cli.rs | Adds new CLI flags (--no-keep, --no-embed, --no-config) and an embed_override() helper with tests. |
| README.md | Documents new CLI flags and updated layering semantics. |
| README.ja.md | Japanese documentation updates for new flags and layering semantics. |
| docs/spec.md | Formalizes type-based layering rules and --no-config semantics in the spec. |
| docs/spec.ja.md | Japanese spec update mirroring the new layering rules. |
| docs/cheatsheet.md | Updates recipes/FAQ to match additive keep, --no-keep, and --no-embed behavior. |
| docs/cheatsheet.ja.md | Japanese cheatsheet update mirroring the new behaviors and recipes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli.rs`:
- Around line 33-39: Add the `overrides_with = "embed"` argument attribute to
the `no_embed` field in the CLI arguments definition, matching the existing
override configuration on `embed`, so specifying both flags makes the last one
take precedence and keeps `embed_override()` consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9838fa1f-c33d-4e99-9b7e-5cc1dd997314
📒 Files selected for processing (8)
README.ja.mdREADME.mddocs/cheatsheet.ja.mddocs/cheatsheet.mddocs/spec.ja.mddocs/spec.mdsrc/cli.rssrc/commands/bundle.rs
One-sided overrides_with already resolves last-wins in both directions (verified in review), so this is intent documentation, not a bug fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
"cancels the config file" read as if the whole file were ignored, which is --no-config's job. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Closes #24
概要
設定ファイルに書いた項目を CLI から打ち消せない非可逆性 (#24) を解消する。「config に置ける項目は CLI で必ず打ち消せる」という不変条件を全項目で成立させ、重ね方は項目の型ごとに定める: スカラー = 上書き、bool =
--x/--no-xペア (後勝ち)、集合 = 加算 + 打ち消し、順序付きリスト = 追記。変更内容
--no-embedを追加 —--embedと後勝ちのペアにし、解決順を「CLI で明示 > config > 組み込み既定」に (従来は OR で config のembed = trueを戻せなかった)--keepを加算化し--no-keepを追加--keep) −--no-keep。同じ ID は順序に依らず--no-keepが勝つ。-k stdの書き忘れで std が全展開される footgun が消え、--no-keep stdで CLI から keep を空にもできる--no-keep stdと明示された場合は意図とみなし抑制 (書き忘れは防護、明示は尊重)--のコンパイラオプションを config への追記に変更-std等) や-Uに委譲。従来の置き換え方式にあった「-- -O0で-std=gnu++17まで消える」隠れバグも解消--no-configを追加 — 「設定ファイルが 1 つも見つからない環境と完全に同一の挙動」と定義する横断的な脱出ハッチ。追記方式で打ち消せない config のフラグ (-I等) もこれで外せる-kで std が展開される」FAQ は config のkeep書き忘れの FAQ に改稿、「keep を今回だけ展開したい →--no-keep」レシピを追加config ファイル側の意味論は従来通り (各項目は宣言的な全量指定でマージしない)。バージョン番号はこの PR では触らない (breaking 変更を含むため、次リリースは v2.0 を想定)。
検証
-k mylibで std も残る (加算)--no-keep stdで std が展開され、警告も抑制されるkeepに std 抜け → 警告が出るembed = trueを--no-embedで打ち消せる--no-configで設定ファイルのある環境が無い環境と同一挙動になる-- -DEXTRA指定後も既定の-DONLINE_JUDGEが生きる (追記)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--no-keepfor one-run unkeeping, with defined precedence over keep settings.--embed/--no-embed, where--no-embedcancels configured embedding.--no-configto ignore.risundlerc.tomlentirely.Documentation