Conversation
There was a problem hiding this comment.
Pull request overview
Bumps the crate to v0.2.0 and adds new Serde helpers to support TOML (de)serialization of optional terminal style configuration, plus a dedicated attribute Serde helper for crossterm.
Changes:
- Add
option_content_style_serdemodules for bothtermionandcrosstermto serialize/deserializeOption<ContentStyle>as style strings. - Add
crossterm_config::attribute_serdefor serializing/deserializingcrossterm::style::Attributetokens. - Update crate version references to
0.2.0inCargo.toml,Cargo.lock, and README.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/termion_config/option_content_style_serde.rs | New Serde adapter + tests for Option<termion_config::ContentStyle> via style-string encoding. |
| src/termion_config.rs | Exposes the new option_content_style_serde module. |
| src/crossterm_config/option_content_style_serde.rs | New Serde adapter + tests for Option<crossterm::style::ContentStyle> via style-string encoding. |
| src/crossterm_config/convert/content_style.rs | Makes attribute token helpers visible for reuse by new attribute Serde module. |
| src/crossterm_config/convert.rs | Re-exports attribute token helpers for internal crate use. |
| src/crossterm_config/attribute_serde.rs | New Serde adapter + tests for crossterm::style::Attribute token encoding. |
| src/crossterm_config.rs | Exposes attribute_serde and option_content_style_serde. |
| README.md | Updates dependency examples to termcfg = "0.2.0". |
| Cargo.toml | Bumps package version to 0.2.0. |
| Cargo.lock | Updates locked version to 0.2.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| fn parse_attribute_token(token: &str) -> Result<Attribute, StyleError> { | ||
| pub fn parse_attribute_token(token: &str) -> Result<Attribute, StyleError> { |
There was a problem hiding this comment.
parse_attribute_token is re-exported as pub(crate) from crossterm_config::convert. Consider making this function pub(crate) as well (instead of pub) to keep the visibility as narrow as possible and avoid accidentally expanding the API surface later.
| pub fn parse_attribute_token(token: &str) -> Result<Attribute, StyleError> { | |
| pub(crate) fn parse_attribute_token(token: &str) -> Result<Attribute, StyleError> { |
| } | ||
|
|
||
| fn attribute_to_token(attribute: Attribute) -> Option<&'static str> { | ||
| pub fn attribute_to_token(attribute: Attribute) -> Option<&'static str> { |
There was a problem hiding this comment.
attribute_to_token is only used internally and re-exported as pub(crate) from crossterm_config::convert. Consider changing this function’s visibility from pub to pub(crate) (or similarly minimal) to avoid unnecessarily widening visibility.
| pub fn attribute_to_token(attribute: Attribute) -> Option<&'static str> { | |
| pub(crate) fn attribute_to_token(attribute: Attribute) -> Option<&'static str> { |
|
|
||
| assert!(serialized.contains("fg=lightblue")); | ||
| assert!(serialized.contains("attr=bold")); | ||
| } |
There was a problem hiding this comment.
The option wrapper’s serialization behavior for None isn’t covered here. Adding a test that Config { style: None } serializes without emitting a style key (matching the crossterm variant) would help prevent regressions in how None is represented in TOML.
| } | |
| } | |
| #[test] | |
| fn serializes_none_without_style_key() { | |
| let config = Config { style: None }; | |
| let serialized = toml::to_string(&config).unwrap(); | |
| // Ensure that when style is None, the field is omitted entirely | |
| assert!(!serialized.contains("style")); | |
| } |
No description provided.