Repository navigation
Fix GNU flat-button box colors and cover tab-line startup - #584
Conversation
Keep plain boxes and flat-button distinct until face realization so omitted colors use foreground and background respectively. Preserve the distinction in pdump and retain preceding styles when a repeated :style is nil. Add startup tab-line regressions and verify rendered box colors through unit, live oracle, TUI, and X11/Wayland GPU tests.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 58.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 12 files. (1 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the two main changes: correcting GNU flat-button box colors and adding tab-line startup coverage. Description check The description directly explains the box-color fix, tab-line regressions, schema changes, and validation results. It is fully related to the changeset. Linked Issues check Issue #583has two coding objectives. The PR preservesBoxStyle::SimpleandBoxStyle::Flatthrough parsing and pdump storage, realizes default colors after face colors, and adds coverage for anony…Out of Scope Changes check The changes stay within issue #583. Face parsing, box realization, pdump schema and round-trip updates support theflat-buttonfix. GUI, TUI, oracle, and layout regressions support tab-line behavior…Full details: Linked Issues check
-
Explanation
Issue
#583has two coding objectives. The PR preservesBoxStyle::SimpleandBoxStyle::Flatthrough parsing and pdump storage, realizes default colors after face colors, and adds coverage for anonymous, named, and default faces. This fixes plain-box versusflat-buttoncolor behavior. The PR also adds startup regressions forglobal-tab-line-mode, new files, split windows, and tab placement. The reviewed summary states that the existing tab-line implementation already renders correctly on the current base, and the new tests cover the reported startup case.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#583. Face parsing, box realization, pdump schema and round-trip updates support theflat-buttonfix. GUI, TUI, oracle, and layout regressions support tab-line behavior and box rendering. The added fixtures and pixel checks validate the reported symptoms. No unrelated product behavior is identified.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 58.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 12 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
There was a problem hiding this comment.
🔵 Needs a closer look
It changes the pdump face-section format (bumping to v4 to reject existing dumps) and alters box-color realization semantics across multiple crates, with verification resting on gated GUI/oracle tests that cannot be exercised here.
No open issues 🎉
What changed in this PR
This PR fixes GitHub issue #583 by correcting the realized color of colorless :box '(:style flat-button) borders and adding regression coverage for global-tab-line-mode activated from an init file. GNU draws plain boxes and flat-button boxes with the same drawing kind (FACE_SIMPLE_BOX) but defaults their colors differently — plain boxes to the face foreground, flat buttons to the face background. The previous code collapsed every parsed box style into BoxStyle::Flat and defaulted all box colors to the foreground, so flat buttons were painted with the wrong color.
The fix introduces a distinct BoxStyle::Simple variant that is kept separate from Flat through Lisp parsing and pdump storage, and chooses the default color after face colors are realized. The pdump face-section schema is bumped to version 4 so older, ambiguous dumps are rejected rather than silently mis-decoded.
Changes:
- Add
BoxStyle::Simple, makegnu_code/from_gnu_codeexplicit (drawing code1decodes toSimple), and preserve a preceding:stylewhen a later:styleis nil (in bothface.rsparsing andxfacesresolution). - Centralize box realization in a new
apply_box_borderhelper that defaults the color to foreground forSimpleand background forFlat/Raised/Pressed. - Add
DumpBoxStyle::Simple, bumpFACE_FORMAT_VERSIONto 4, and extend pdump conversions; add core, bridge, oracle, TUI, and GUI tests plus a GUI fixture.
| File | Description |
|---|---|
crates/neovm-core/src/face.rs |
Adds Simple variant, explicit gnu_code/from_gnu_code, and nil-preserving :style parsing. |
crates/neovm-core/src/emacs_core/display/xfaces/mod.rs |
Defaults boxes to Simple and preserves a prior :style when a later one is nil. |
crates/neovm-core/src/emacs_core/runtime/pdump/types.rs |
Adds DumpBoxStyle::Simple = 3. |
crates/neovm-core/src/emacs_core/runtime/pdump/face_image.rs |
Bumps face-section format version to 4 to reject ambiguous old dumps. |
crates/neovm-core/src/emacs_core/runtime/pdump/convert.rs |
Maps Simple between runtime and dump box styles. |
crates/neomacs-layout-engine/src/neovm_bridge.rs |
Adds apply_box_border, choosing default box color by style. |
crates/neomacs-gui-tests/fixtures/issue-583-tab-line/init.el |
GUI fixture exercising startup tab lines, splits, and flat-button box. |
crates/neomacs-gui-tests/tests/window_chrome.rs |
GUI regression asserting tab placement, resolved colors, and GPU pixels. |
crates/neomacs-tui-tests/tests/windows_tabs.rs |
TUI regression for global tab line from init and visiting a new file. |
crates/neovm-oracle-tests/tests/oracle_tab_line.rs |
Live-GNU parity for tab-line activation and flat-button style ordering. |
crates/neovm-core/src/face/tests/face_test.rs, .../face_image/tests/face_image_test.rs, .../neovm_bridge/tests/neovm_bridge_test.rs |
Unit tests covering the new style distinction and dump round-trips. |
I verified that BoxStyle::from_gnu_code and the removed IntoPrimitive/TryFromPrimitive derives have no remaining production callers, that box_style_to_u8 is fully replaced, that apply_box_border reads fg/bg only after they are installed in all three realization paths, that the dump conversions map by name (not discriminant) so the differing enum orderings are safe, and that the new test files follow existing registration and harness conventions. No objective defects were found in the changed hunks.
Review effort: Balanced
Colorless
:box '(:style flat-button)boxes used the foreground instead of the face background. GNU's face attribute contract gives plain boxes and flat buttons different default colors, even though both use the same drawing kind.Keep
BoxStyle::SimpleandBoxStyle::Flatdistinct through parsing and dump storage, and choose their default colors after face colors are realized. Use exhaustive enum matches, share box realization across default/named/anonymous faces, and preserve a preceding style when a repeated:styleis nil. Bump the face-section schema to 4 so old ambiguous dumps are rejected.The missing-tab symptom already renders correctly on the current base. Add regressions for enabling global tab lines through an actual startup init, visiting a new file, and split windows. GUI checks assert tab placement below frame chrome, resolved border colors, and actual GPU pixels.
Validation on the final development build, with default features plus video and regenerated Lisp bytecode/pdump:
Both the original border-color regression and trailing-nil regression failed before their fixes. The bootstrap unit's stale-bytecode guard passed after a full bytecode rebuild, with the guard retained.
Fixes #583.