Use bitflags crate for ergonomics#77
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplace per-field boolean table options with typed bitflags; update Cargo features/dependencies; migrate serialize/deserialize, FFI structs/converters/defaults, and tests to use .from_bits_retain(), .contains(), and .bits() across character, color, font, and pixmap tables. ChangesDependency & Core Structure Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/core/tables/character/mod.rs (1)
180-214:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against panics when modifier flags and record payload diverge.
On Line 184, Line 199, and Line 214,
unwrap()can panic if a flag is set but aCharacterrecord lacks the corresponding value. Prefer returning a serialization error after validating each flagged field is present.Proposed fix
+ for character in &self.characters { + if self.modifier_flags.contains(CharacterTableModifierFlags::UseAdvanceX) + && character.advance_x.is_none() + { + return Err(SerializeError::InvalidTableState); + } + if self.modifier_flags.contains(CharacterTableModifierFlags::UsePixmapIndex) + && character.pixmap_index.is_none() + { + return Err(SerializeError::InvalidTableState); + } + if self + .modifier_flags + .contains(CharacterTableModifierFlags::UsePixmapTableIndex) + && character.pixmap_table_index.is_none() + { + return Err(SerializeError::InvalidTableState); + } + } + for (index, character) in self.characters.iter().enumerate() {🤖 Prompt for 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. In `@src/core/tables/character/mod.rs` around lines 180 - 214, The code currently calls unwrap() on character.advance_x, character.pixmap_index, and character.pixmap_table_index when the corresponding modifier_flags (CharacterTableModifierFlags::UseAdvanceX, ::UsePixmapIndex, ::UsePixmapTableIndex) are set, which can panic if the record lacks the value; change each unwrap to validate the Option is Some and, on None, return a serialization error (or map to the existing error type) instead of panicking—locate the blocks that call engine.bytes.push(character.advance_x.unwrap()) / character.pixmap_index.unwrap() / character.pixmap_table_index.unwrap() and replace with safe checks that produce a clear error including which field/table/record failed.src/core/tables/font/serialize.rs (1)
69-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
link_flagsagainstcharacter_table_indexesbefore serialization.If
LinkCharacterTablesis set butcharacter_table_indexesisNone(or vice versa), the encoded link byte and payload structure disagree.Proposed fix
+ if self + .link_flags + .contains(FontTableLinkFlags::LinkCharacterTables) + != self.character_table_indexes.is_some() + { + return Err(SerializeError::InvalidTableState); + } + engine.bytes.push(self.link_flags.bits());🤖 Prompt for 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. In `@src/core/tables/font/serialize.rs` around lines 69 - 84, The serializer writes link_flags.bits() and then unconditionally serializes character_table_indexes, which can produce inconsistent wire format when FontTableLinkFlags::LinkCharacterTables is set but character_table_indexes is None (or vice versa); update the serialization in serialize.rs (the function that pushes engine.bytes and handles character_table_indexes) to validate that self.link_flags.contains(FontTableLinkFlags::LinkCharacterTables) iff self.character_table_indexes.is_some(), return or propagate an error (or panic with a clear message) when the two are inconsistent, and only emit the character_table_indexes payload when the flag is present so the encoded link byte and payload structure always agree (refer to symbols: link_flags, character_table_indexes, FontTableLinkFlags::LinkCharacterTables, engine.bytes.push).src/core/tables/character/serialize.rs (1)
77-96:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent invalid character-table encoding when flags and optional fields are out of sync.
On Line 77 and Line 131, flags are written from bitfields, but value/link payloads are still emitted from
Optionpresence. Also on Line 85, tagging usesis_some()instead of the emitted flag. These can diverge and produce inconsistent output/tagging.Proposed fix
+ if self + .configuration_flags + .contains(crate::core::CharacterTableConfigurationFlags::ConstantClusterCodePoints) + != self.constant_cluster_codepoints.is_some() + { + return Err(SerializeError::InvalidTableState); + } + if self + .link_flags + .contains(CharacterTableLinkFlags::LinkPixmapTables) + != self.pixmap_table_indexes.is_some() + { + return Err(SerializeError::InvalidTableState); + } + engine.bytes.push(self.configuration_flags.bits()); // Configuration flags byte @@ - value: self.constant_cluster_codepoints.is_some(), + value: self + .configuration_flags + .contains(crate::core::CharacterTableConfigurationFlags::ConstantClusterCodePoints),Also applies to: 131-146
🤖 Prompt for 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. In `@src/core/tables/character/serialize.rs` around lines 77 - 96, The encoding can become inconsistent because flags are written from self.configuration_flags.bits() while payloads and tagging are gated by Option presence; update serialize logic so payloads (e.g., writing constant_cluster_codepoints) are only emitted when the corresponding flag bit in self.configuration_flags is set (not just Option::is_some()), and update tagging (TagKind::CharacterTableUseConstantClusterCodepoints) and any checks that currently use self.constant_cluster_codepoints.is_some() to instead derive presence from the same flag bit you wrote via self.configuration_flags.bits(); ensure you also use that flag when computing configuration_values_start and when emitting any other optional fields referenced later (the code paths around engine.bytes.push(self.configuration_flags.bits()), the constant_cluster_codepoints emission, and the tagging calls).src/core/tables/pixmap/serialize.rs (1)
54-119:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce flag-to-payload consistency before writing pixmap table bytes.
configuration_flags.bits()andlink_flags.bits()are now authoritative, but payload emission still depends onOptionfields. If these diverge, the output stream becomes structurally invalid for deserialization.Proposed fix
+ if self + .configuration_flags + .contains(PixmapTableConfigurationFlags::ConstantWidth) + != self.constant_width.is_some() + { + return Err(SerializeError::InvalidTableState); + } + if self + .configuration_flags + .contains(PixmapTableConfigurationFlags::ConstantHeight) + != self.constant_height.is_some() + { + return Err(SerializeError::InvalidTableState); + } + if self + .configuration_flags + .contains(PixmapTableConfigurationFlags::ConstantBitsPerPixel) + != self.constant_bits_per_pixel.is_some() + { + return Err(SerializeError::InvalidTableState); + } + engine.bytes.push(self.configuration_flags.bits()); @@ + if self + .link_flags + .contains(PixmapTableLinkFlags::LinkColorTables) + != self.color_table_indexes.is_some() + { + return Err(SerializeError::InvalidTableState); + } + engine.bytes.push(self.link_flags.bits());Also applies to: 145-210
🤖 Prompt for 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. In `@src/core/tables/pixmap/serialize.rs` around lines 54 - 119, Before emitting payload bytes, validate that PixmapTableConfigurationFlags (configuration_flags) match the presence of the corresponding Option fields and fail early if they diverge: ensure that if PixmapTableConfigurationFlags::ConstantWidth is set then self.constant_width is Some (and vice versa), same for ConstantHeight and ConstantBitsPerPixel; if a mismatch occurs return an error (or propagate a Serialize error) instead of writing bytes. Update the emit logic around engine.bytes.push (and engine.tags.tag_byte) to only write bytes when the flag is set (not merely when the Option is Some), and apply the same validation for link_flags/payload pairs in the other region (lines ~145-210) so flags are authoritative and serialization cannot produce structurally invalid output.src/core/tables/color/mod.rs (1)
286-303:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce flag/value invariants before serializing to avoid malformed bytes and panics.
Line 399 writes config flags from
self.configuration_flags, but Line 407/416 behavior is driven byself.constant_alpha. If those diverge, the output layout shifts. Also, Line 290 and Line 303 can panic viaunwrap()when required per-record fields are missing.💡 Suggested fix
pub(crate) fn push_configurations<T: TagWriter>(&self, engine: &mut SerializeEngine<T>) { @@ - engine.bytes.push(self.configuration_flags.bits()); // configuration flags + let use_constant_alpha = self + .configuration_flags + .contains(ColorTableConfigurationFlags::ConstantAlpha); + engine.bytes.push(self.configuration_flags.bits()); @@ - vec![TagKind::ColorTableUseConstantAlpha { + vec![TagKind::ColorTableUseConstantAlpha { table_index: engine.tagging_data.current_table_index, - value: self.constant_alpha.is_some(), + value: use_constant_alpha, }], engine.bytes.byte_index(), ); @@ - if let Some(constant_alpha) = self.constant_alpha { - engine.bytes.push(constant_alpha); + if use_constant_alpha { + let constant_alpha = self + .constant_alpha + .ok_or(SerializeError::InvalidColorTableState)?; + engine.bytes.push(constant_alpha); #[cfg(feature = "tagging")] engine.tags.tag_byte( TagKind::ColorTableConstantAlpha { table_index: engine.tagging_data.current_table_index, value: constant_alpha, }, engine.bytes.byte_index(), ); + } else if self.constant_alpha.is_some() { + return Err(SerializeError::InvalidColorTableState); } }for (index, color) in self.colors.iter().enumerate() { @@ - if self - .modifier_flags - .contains(ColorTableModifierFlags::UseColorType) - { - engine.bytes.push(color.color_type.unwrap() as u8); + if self + .modifier_flags + .contains(ColorTableModifierFlags::UseColorType) + { + let color_type = color + .color_type + .ok_or(SerializeError::InvalidColorTableState)?; + engine.bytes.push(color_type as u8); @@ - if self.constant_alpha.is_none() { - engine.bytes.push(color.custom_alpha.unwrap()); + if !self + .configuration_flags + .contains(ColorTableConfigurationFlags::ConstantAlpha) + { + let custom_alpha = color + .custom_alpha + .ok_or(SerializeError::InvalidColorTableState)?; + engine.bytes.push(custom_alpha);Also applies to: 399-417
🤖 Prompt for 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. In `@src/core/tables/color/mod.rs` around lines 286 - 303, The serializer currently unwraps per-record fields and assumes flag/value invariants (e.g., using ColorTableModifierFlags::UseColorType then color.color_type.unwrap(), and checking self.constant_alpha vs. color.custom_alpha.unwrap()), which can panic or produce malformed layouts; update the serialization in ColorTable (mod.rs) to first validate invariants and handle missing/invalid cases: when self.modifier_flags.contains(ColorTableModifierFlags::UseColorType) only read and push color.color_type after checking color.color_type.is_some() (otherwise return an error or skip with a clear log), and similarly ensure self.constant_alpha.is_none() implies color.custom_alpha.is_some() before unwrapping (otherwise return an explicit error), so no unwrap() is used and the output layout remains consistent when writing bytes via engine.bytes.push and tagging via engine.tags.tag_byte.
🤖 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.
Outside diff comments:
In `@src/core/tables/character/mod.rs`:
- Around line 180-214: The code currently calls unwrap() on character.advance_x,
character.pixmap_index, and character.pixmap_table_index when the corresponding
modifier_flags (CharacterTableModifierFlags::UseAdvanceX, ::UsePixmapIndex,
::UsePixmapTableIndex) are set, which can panic if the record lacks the value;
change each unwrap to validate the Option is Some and, on None, return a
serialization error (or map to the existing error type) instead of
panicking—locate the blocks that call
engine.bytes.push(character.advance_x.unwrap()) /
character.pixmap_index.unwrap() / character.pixmap_table_index.unwrap() and
replace with safe checks that produce a clear error including which
field/table/record failed.
In `@src/core/tables/character/serialize.rs`:
- Around line 77-96: The encoding can become inconsistent because flags are
written from self.configuration_flags.bits() while payloads and tagging are
gated by Option presence; update serialize logic so payloads (e.g., writing
constant_cluster_codepoints) are only emitted when the corresponding flag bit in
self.configuration_flags is set (not just Option::is_some()), and update tagging
(TagKind::CharacterTableUseConstantClusterCodepoints) and any checks that
currently use self.constant_cluster_codepoints.is_some() to instead derive
presence from the same flag bit you wrote via self.configuration_flags.bits();
ensure you also use that flag when computing configuration_values_start and when
emitting any other optional fields referenced later (the code paths around
engine.bytes.push(self.configuration_flags.bits()), the
constant_cluster_codepoints emission, and the tagging calls).
In `@src/core/tables/color/mod.rs`:
- Around line 286-303: The serializer currently unwraps per-record fields and
assumes flag/value invariants (e.g., using ColorTableModifierFlags::UseColorType
then color.color_type.unwrap(), and checking self.constant_alpha vs.
color.custom_alpha.unwrap()), which can panic or produce malformed layouts;
update the serialization in ColorTable (mod.rs) to first validate invariants and
handle missing/invalid cases: when
self.modifier_flags.contains(ColorTableModifierFlags::UseColorType) only read
and push color.color_type after checking color.color_type.is_some() (otherwise
return an error or skip with a clear log), and similarly ensure
self.constant_alpha.is_none() implies color.custom_alpha.is_some() before
unwrapping (otherwise return an explicit error), so no unwrap() is used and the
output layout remains consistent when writing bytes via engine.bytes.push and
tagging via engine.tags.tag_byte.
In `@src/core/tables/font/serialize.rs`:
- Around line 69-84: The serializer writes link_flags.bits() and then
unconditionally serializes character_table_indexes, which can produce
inconsistent wire format when FontTableLinkFlags::LinkCharacterTables is set but
character_table_indexes is None (or vice versa); update the serialization in
serialize.rs (the function that pushes engine.bytes and handles
character_table_indexes) to validate that
self.link_flags.contains(FontTableLinkFlags::LinkCharacterTables) iff
self.character_table_indexes.is_some(), return or propagate an error (or panic
with a clear message) when the two are inconsistent, and only emit the
character_table_indexes payload when the flag is present so the encoded link
byte and payload structure always agree (refer to symbols: link_flags,
character_table_indexes, FontTableLinkFlags::LinkCharacterTables,
engine.bytes.push).
In `@src/core/tables/pixmap/serialize.rs`:
- Around line 54-119: Before emitting payload bytes, validate that
PixmapTableConfigurationFlags (configuration_flags) match the presence of the
corresponding Option fields and fail early if they diverge: ensure that if
PixmapTableConfigurationFlags::ConstantWidth is set then self.constant_width is
Some (and vice versa), same for ConstantHeight and ConstantBitsPerPixel; if a
mismatch occurs return an error (or propagate a Serialize error) instead of
writing bytes. Update the emit logic around engine.bytes.push (and
engine.tags.tag_byte) to only write bytes when the flag is set (not merely when
the Option is Some), and apply the same validation for link_flags/payload pairs
in the other region (lines ~145-210) so flags are authoritative and
serialization cannot produce structurally invalid output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b376ff1a-09a4-437b-a701-b42726aed5c8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomlsrc/core/mod.rssrc/core/tables/character/deserialize.rssrc/core/tables/character/mod.rssrc/core/tables/character/serialize.rssrc/core/tables/color/mod.rssrc/core/tables/font/deserialize.rssrc/core/tables/font/serialize.rssrc/core/tables/pixmap/deserialize.rssrc/core/tables/pixmap/serialize.rs
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 @.devcontainer/devcontainer.json:
- Line 6: The devcontainer image tag
"mcr.microsoft.com/devcontainers/rust:1-1-bullseye" is invalid; update the
"image" value in .devcontainer/devcontainer.json (the "image" key) to a
published tag such as "mcr.microsoft.com/devcontainers/rust:1-bullseye" or
"mcr.microsoft.com/devcontainers/rust:2-1-bullseye" (choose the one matching the
intended Rust version) so the manifest resolves successfully.
🪄 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 Plus
Run ID: 37ab04d7-cbf0-4a08-b37a-a8e8af4a9b84
📒 Files selected for processing (1)
.devcontainer/devcontainer.json
| { | ||
| "name": "Rust", | ||
| // Or use a Dockerfile or Docker Compose file. More info: https://containers.dev/guide/dockerfile | ||
| "image": "mcr.microsoft.com/devcontainers/rust:1-1-bullseye", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the available tags for mcr.microsoft.com/devcontainers/rust Docker images?
💡 Result:
Available tags for mcr.microsoft.com/devcontainers/rust (current variants and common tag patterns): 1) Latest - mcr.microsoft.com/devcontainers/rust:latest [1][2] - mcr.microsoft.com/devcontainers/rust:trixie [1][2] - mcr.microsoft.com/devcontainers/rust:bookworm [1][2] - mcr.microsoft.com/devcontainers/rust:bullseye [1][2] 2) Major version tags - mcr.microsoft.com/devcontainers/rust:1 (and OS-pinned variants mcr.microsoft.com/devcontainers/rust:1-trixie,:1-bookworm,:1-bullseye) [2] 3) “2” major and minor/patch semantic version tags (pattern includes OS-pinned variants) - Examples of tags shown in the upstream Rust devcontainer image docs: - mcr.microsoft.com/devcontainers/rust:2-1 (and:2-1-trixie,:2-1-bookworm,:2-1-bullseye) [2] - mcr.microsoft.com/devcontainers/rust:2.0-1 (and:2.0-1-trixie,:2.0-1-bookworm,:2.0-1-bullseye) [2] - mcr.microsoft.com/devcontainers/rust:2.0.9-1 (and:2.0.9-1-trixie,:2.0.9-1-bookworm,:2.0.9-1-bullseye) [2] Authoritative place to view the full, complete list - https://mcr.microsoft.com/en-us/artifact/mar/devcontainers/rust/tags [1] Note: The complete list of every published semantic-version tag is dynamic; the MCR “tags” page above is the authoritative source for all currently available tags. The variants listed (trixie/bookworm/bullseye) and tag naming patterns (latest/major/semantic-version-with-OS-suffix) are explicitly documented in the devcontainers image sources. [1][2]
Citations:
- 1: https://mcr.microsoft.com/en-us/artifact/mar/devcontainers/rust/tags
- 2: https://github.com/devcontainers/images/tree/main/src/rust
🏁 Script executed:
#!/bin/bash
# Check if the specified image tag exists in Microsoft Container Registry
IMAGE="mcr.microsoft.com/devcontainers/rust"
TAG="1-1-bullseye"
echo "Checking if ${IMAGE}:${TAG} exists..."
curl -s -o /dev/null -w "%{http_code}" "https://mcr.microsoft.com/v2/devcontainers/rust/manifests/${TAG}"
echo ""
echo "Response code 200 indicates the tag exists, 404 indicates it does not."Repository: SimplePixelFont/spf.rs
Length of output: 213
Fix invalid devcontainer Rust image tag (1-1-bullseye)
.devcontainer/devcontainer.json(line 6) usesmcr.microsoft.com/devcontainers/rust:1-1-bullseye- MCR manifest lookup for
1-1-bullseyereturns 404 (tag not available); published patterns use tags like1-bullseyeand2-1-bullseye - Change the image tag to an existing variant (e.g.,
mcr.microsoft.com/devcontainers/rust:1-bullseye, or2-1-bullseyeif that’s the intended Rust version)
🤖 Prompt for 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.
In @.devcontainer/devcontainer.json at line 6, The devcontainer image tag
"mcr.microsoft.com/devcontainers/rust:1-1-bullseye" is invalid; update the
"image" value in .devcontainer/devcontainer.json (the "image" key) to a
published tag such as "mcr.microsoft.com/devcontainers/rust:1-bullseye" or
"mcr.microsoft.com/devcontainers/rust:2-1-bullseye" (choose the one matching the
intended Rust version) so the manifest resolves successfully.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/ffi/mod.rs (1)
98-116: ⚡ Quick winBreaking ABI change - document for FFI consumers.
The struct layouts for
SPFPixmapTable,SPFCharacterTable,SPFColorTable, andSPFFontTablehave changed (new flag fields replacing/adding to previous boolean fields). This is a breaking change for existing C consumers. Ensure this is documented in release notes or migration guides.Also applies to: 131-147, 162-173, 189-199
🤖 Prompt for 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. In `@src/ffi/mod.rs` around lines 98 - 116, The change replaces/expands boolean fields with flag bytes in the FFI structs (SPFPixmapTable, SPFCharacterTable, SPFColorTable, SPFFontTable) which alters their memory layout and is a breaking ABI change for C consumers; update the release notes and migration guide to explicitly state the ABI break, list the affected structs by name, describe the old vs new field layouts (e.g., booleans -> c_uchar flag fields, added fields like configuration_flags/link_flags), recommend bumping the crate's major version, provide example C struct definitions and a short migration snippet showing how to read the new flag bytes, and optionally document a compatibility plan (e.g., a #[cfg(feature = "legacy_abi")] path or a conversion function) so consumers can adapt.
🤖 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 `@tests/integration_tests.rs`:
- Around line 28-31: The test sets PixmapTableConfigurationFlags::ConstantWidth
but leaves pixmap_table.constant_width as None, causing a flag/value mismatch:
update the test so that when PixmapTableConfigurationFlags::ConstantWidth is
present you set pixmap_table.constant_width = Some(<u8>) with a valid u8 value,
or alternatively remove ConstantWidth from pixmap_table.configuration_flags (and
add a comment only if you intend to test malformed input); ensure the change
keeps the flag/value pair consistent with the serializer/deserializer
expectations referenced by pixmap_table.constant_width and
PixmapTableConfigurationFlags::ConstantWidth.
---
Nitpick comments:
In `@src/ffi/mod.rs`:
- Around line 98-116: The change replaces/expands boolean fields with flag bytes
in the FFI structs (SPFPixmapTable, SPFCharacterTable, SPFColorTable,
SPFFontTable) which alters their memory layout and is a breaking ABI change for
C consumers; update the release notes and migration guide to explicitly state
the ABI break, list the affected structs by name, describe the old vs new field
layouts (e.g., booleans -> c_uchar flag fields, added fields like
configuration_flags/link_flags), recommend bumping the crate's major version,
provide example C struct definitions and a short migration snippet showing how
to read the new flag bytes, and optionally document a compatibility plan (e.g.,
a #[cfg(feature = "legacy_abi")] path or a conversion function) so consumers can
adapt.
🪄 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 Plus
Run ID: 861e288c-0fbe-4f0b-a382-3234370eec4b
📒 Files selected for processing (8)
.vscode/settings.jsonsrc/ffi/converters/character_table.rssrc/ffi/converters/color_table.rssrc/ffi/converters/font_table.rssrc/ffi/converters/pixmap_table.rssrc/ffi/defaults.rssrc/ffi/mod.rstests/integration_tests.rs
✅ Files skipped from review due to trivial changes (1)
- .vscode/settings.json
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
1 participant
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 by CodeRabbit
New Features
Breaking Changes
Chores
Tests