fix: Add per-field serde defaults to Colors for partial config support#6
Merged
Merged
Conversation
Add #[serde(default)] attributes to each field in the Colors struct to
support partial color configuration in TOML/JSON files.
Problem:
When users specify [logging.colors] with only some fields (e.g., just
timestamp), serde would deserialize ALL fields, setting unspecified
fields to None instead of using the struct's Default implementation.
This caused twyg to lose its colored output for unspecified fields.
Example of broken behavior:
```toml
[logging.colors]
timestamp = { fg = "HiBlack", bg = "Reset" }
```
Before this fix:
- timestamp: Some(HiBlack) ✓
- level_info: None ✗ (should be HiGreen)
- message: None ✗ (should be Green)
- All other fields: None ✗
Solution:
Add #[serde(default = "default_fn")] to each field with corresponding
default functions. Now each field independently falls back to its
default value when not specified in the config.
After this fix:
- timestamp: Some(HiBlack) ✓ (from config)
- level_info: Some(HiGreen) ✓ (from default)
- message: Some(Green) ✓ (from default)
- All other fields: Some(...) ✓ (from defaults)
Changes:
- Added #[serde(default = "...")] to all 13 fields in Colors struct
- Added 13 default_* functions matching Colors::default() values
- Added test to verify partial deserialization preserves defaults
Testing:
- All existing tests pass
- New test confirms partial config works correctly
- Verified with burrow project using twyg
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add toml = "0.9" to dev-dependencies - Change test from JSON to TOML deserialization - Matches how users actually configure colors in TOML files
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.
Problem
When users specify
[logging.colors]in their TOML config with only some fields (e.g., justtimestamp), serde would deserialize ALL fields of theColorsstruct. Unspecified fields would be set toNoneinstead of using the struct'sDefaultimplementation values.This caused twyg to lose colored output for all unspecified color fields.
Example of Broken Behavior
Config file:
Before this fix:
timestamp:Some(HiBlack)✅ (from config)level_info:None❌ (should beHiGreen)message:None❌ (should beGreen)None❌Result: Only timestamps were colored, all other log output was plain white text.
Solution
Add
#[serde(default = "default_fn")]attribute to each field in theColorsstruct, with corresponding default functions. This makes serde apply per-field defaults when fields are missing from the config.After this fix:
timestamp:Some(HiBlack)✅ (from config)level_info:Some(HiGreen)✅ (from default)message:Some(Green)✅ (from default)Some(...)✅ (from defaults)Result: Users can now customize individual colors while keeping defaults for everything else.
Changes
#[serde(default = "default_*")]to all 13 fields inColorsstructdefault_timestamp,default_level_info, etc.) matching values fromColors::default()test_colors_partial_deserialization_preserves_defaultsto verify the fixTesting
Example Usage
Users can now do this in their config:
Or this:
Breaking Changes
None. This is a pure bug fix that makes partial configuration work as users would naturally expect.