config: layered XDG-style config discovery (engine + distributors API) - #14
Merged
Merged
Conversation
Introduce a private module-level helper that merges one toml::Value tree into another. Tables recurse so sibling keys present only in the base are preserved, while every non-table slot (scalars, arrays, options) is replaced wholesale by the overlay. This is the foundation for the upcoming 3-layer XDG config merge (builtin -> /etc -> user); future tasks will wire it into the loader. Add two unit tests (test_merge_recursive_table, test_merge_array_replaces) covering the recursion arm and the array-replace-not-append guarantee.
Cycle 3 of the 3-layer config merge refactor. Implements the test-friendly pure form that takes explicit system + optional user paths and returns a Config built by overlaying both layers on top of Config::default() via merge_value. parse_layer reads + parses a single TOML layer into a generic toml::Value so we can stage merges before the final coerce step. This commit covers only the happy path (Ok arms + try_into coercion). Warn-on-missing-system, warn-on-malformed, and the warn-fallback for a failed final coerce are intentionally left as TODOs for cycle 4 so each diagnostic gets its own RED test. Config::load() is unchanged in this cycle; the BCON_CONFIG dispatcher that wires this entry point into production discovery is scheduled for cycle 5. - Add parse_layer free fn next to merge_value. - Add Config::load_layered_from_paths as pub(crate) so the unit test can call it but external API consumers cannot. - Add test_load_three_layer_merge covering user-wins-on-overlap and builtin-transparency for fields absent from both files.
Fill in the TODO(cycle 4) markers left by the happy-path commit: - System layer missing: warn with reinstall hint, fall back to builtin. - System layer malformed: warn with file path and parser error, skip the layer. - User layer malformed: warn likewise, skip the user layer. - Merged Config coerce failure: warn and fall back to Config::default (the previous .expect lost the toml::de::Error message). Add a comment near the Config::default()->toml::Value try_from .expect clarifying why that one stays as a panic: it guards a type invariant (any Default impl must be TOML-serializable), not user input. Tests: - test_load_no_system_layer: system path missing + user None must return Config::default() values on representative leaf fields. - test_load_malformed_user_layer: user file with invalid TOML is skipped, system layer wins on the overlapping key, no panic. cargo test --bin bcon config::tests: 7/7 passing.
Config::load() now follows the new XDG-style discovery model: 1. If BCON_CONFIG env var is set and the file exists, load that single file as an exclusive bypass (skip the merge entirely). Intended for debug and test scenarios where a single-file override is desired. 2. Otherwise call Config::load_layered_from_paths with /etc/bcon/config.toml as the system layer and ~/.config/bcon/config.toml as the user layer. The old first-found-exclusive body is removed in favor of delegating to load_layered_from_paths. Config::config_path() stays untouched because ConfigWatcher uses it to identify the primary path to watch. Tests: - test_load_bcon_config_env_bypasses_layers: writes a tempdir override file, sets BCON_CONFIG, calls Config::load(), restores prior env, and asserts the override's font.size won (proving merge was bypassed). A static Mutex serializes future env-touching tests. Plan deviation: The Cycle 5 plan called for tests/config_layer_test.rs as a separate integration test. bcon's src/lib.rs only re-exports the ipc module, not config; exposing Config as part of the lib API would broaden the upstream PR surface unnecessarily for what is fundamentally a single test. The unit module already supports the same setup via tempfile, so the env-bypass test lives in src/config/mod.rs::tests instead. If a future task needs cross-crate integration tests of Config, lib re-export can be reconsidered then. cargo test --bin bcon config::tests: 8/8 passing. No new clippy lints introduced (config/mod.rs:427 warning is pre-existing).
Exposes the canonical TOML serialization of Config::default() so that package distributors (Arch AUR, Debian apt, future RPM, etc.) can ship the rendered string as their /etc/bcon/config.toml template without re-running the bcon binary in the build chroot. Calling the binary at package time would side-effect through dirs::config_dir() and fontconfig::FontFinder::new(), which are fragile under makepkg's clean chroot — committing a static template generated from this API keeps packaging reproducible. The new function uses toml::to_string_pretty(&Self::default()) and .expect()s on serialization failure: that path is unreachable for any correct Default impl (it's a type invariant, not a runtime input problem), matching the same .expect() rationale used for toml::Value::try_from(&default_cfg) inside load_layered_from_paths. Test: test_default_template_round_trips confirms the rendered string parses cleanly back into a Config equivalent to Config::default() on representative leaf fields (font.size, terminal.scrollback_lines, keybinds.copy). This is the upstream-side drift detector. The distribution-side drift guard (comparing this output against the checked-in packaging/arch/bcon-default.toml) belongs to the AUR packaging commit and lands separately on the local-build branch once PR1 has merged upstream. cargo test --bin bcon config::tests: 11/11 passing.
The new layered config tests use tempfile::tempdir() to set up isolated XDG directories so that `cargo test` neither depends on nor pollutes the developer's ~/.config/bcon/. cargo test --bin bcon config::tests: 9/9 passing.
Owner
|
Clean and well-tested — thanks! Looking forward to the CLI half. |
sanohiro
pushed a commit
that referenced
this pull request
May 7, 2026
…et token Updates user-facing documentation to describe the layered config discovery model from #14 and the new --init-config target/preset split. docs/configuration.md and configuration.ja.md: - "Config File Locations" rewritten as a layered XDG merge (built-in defaults <- /etc/bcon/config.toml <- ~/.config/bcon/config.toml) with helix/mpv-style semantics (table-recursive, array-replace). - New "Generating a Config File: --init-config" section with two subsections: Destinations - 4-row table for system / user / absolute path / tilde-prefixed path (path rows note BCON_CONFIG for loading the resulting file). Examples - typical invocations whose generated file is picked up by the standard layered load. - New "Single-file bypass: BCON_CONFIG" section, placed after --init-config, with two use cases: loading an ad-hoc config generated by a path target, and BCON_CONFIG=/dev/null bcon for clean-defaults A/B-comparison. - "system" removed from the preset list since it is now a target, not a preset. - "Example Config" renamed to "Configuration File Example" so the upstream-content section name no longer collides with the new --init-config "Examples" subsection. CLAUDE.md: - Configuration section expanded to mention the layered merge, BCON_CONFIG, and the explicit target form of --init-config. docs/installation.md and installation.ja.md are intentionally unchanged: their existing --init-config=system,vim,jp invocations remain valid under the new parser (the comma-list first-token scheme).
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
Implements the layered XDG-style config discovery proposed in #13. Per the
maintainer's two-PR split request, this PR contains the engine + tests +
distributors API half. The
--init-configCLI redesign and thedocs/configuration.mdrewrite ship in a follow-up PR.Behavior
Config::default()(built-in)/etc/bcon/config.toml$XDG_CONFIG_HOME/bcon/config.tomlBCON_CONFIG=<path>Merge rules: tables merge recursively, scalars / arrays / options replace
wholesale — same family as
helix,mpv,neovim.BCON_CONFIGis anexclusive override (not merged), so
BCON_CONFIG=/dev/nullis a cleanway to bisect against built-in defaults during development. Reading
/dev/nullyields an empty TOML document, which serde populates with alldefaults thanks to
#[serde(default)]on every section.Backward compatibility
Pre-existing
~/.config/bcon/config.toml-only deployments continue workingunchanged. No file paths, env variables, or
--init-configsemanticschanged name in this PR (CLI is revisited in PR2). The runtime warns once
when
/etc/bcon/config.tomlis missing so package installs areself-diagnosing, but it does not error.
What's in this PR
merge_value— table-recursive TOML merge primitiveload_layered_from_paths— layered merge dispatcher with gracefuldegradation on missing/malformed layers (warn + skip, never panic)
Config::loaddispatchesBCON_CONFIGbypass vs. layered mergeConfig::default_template() -> String— pub API so distributors(Arch AUR, Debian apt, etc.) can ship the rendered defaults as
/etc/bcon/config.tomlwithout invoking the bcon binary at packagetime (which would side-effect via
dirs::config_dir()andfontconfig::FontFinder::new(), fragile undermakepkg's clean chroot)tempfiledev-dependency for isolated XDG dirs in testsTest plan
cargo test --bin bcon config::tests: 9/9 passing.test_merge_recursive_tabletest_merge_array_replacestest_load_three_layer_mergetest_load_no_system_layer/etc/bcon/is absenttest_load_malformed_user_layertest_load_bcon_config_env_bypasses_layersBCON_CONFIGexclusive overridetest_default_template_round_tripsConfig::default_template()round-tripstest_parse_color,test_parse_keybindManual verification
End-to-end verified on a separate Arch VM (
bcon@tty2.serviceas root,with an existing
/etc/bcon/config.toml): system + user merge order,missing-system warning, malformed-user graceful skip,
BCON_CONFIGexclusive bypass, and stateless cleanup all behave as designed.
Refs
Refs #13 (engine half). Follow-up PR will cover
--init-configCLIredesign +
docs/configuration.mdrewrite.