Repository navigation
fix(xdisp): mode-line multibyte identity follows inputs, not content (#470) - #473
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughMode-line rendering now tracks multibyte identity from its inputs and preserves it through composition, slicing, and output encoding. Core and oracle tests cover Unicode content and unibyte bytes. A paired-session test compares GNU Emacs and neomacs after repeated mode-line updates. ChangesMode-line string identity
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to Mode-line output preserves input string identity through repeated rendering, and the inspected byte8 path retains unibyte raw bytes. The added regression coverage addresses the reported rendering behavior; no actionable issue remains before merge. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes core mode-line string-identity and unibyte byte-decoding semantics shared across all redisplay paths, so human review of the encoding behavior is warranted despite comprehensive tests.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
This PR fixes issue #470, where a mode line containing a Latin-1-supplement character (e.g. · U+00B7) initially rendered correctly but later degraded into the literal octal escape \267. The root cause was that into_display_output derived the rendered string's multibyte flag from its content (any(code > 0xFF)); since U+0080..U+00FF codes fit in one byte, the string was re-encoded as unibyte raw bytes on re-evaluation. The fix makes ModeLineRendered carry an input-driven multibyte flag (GNU's concat/substring identity rule) through every append path, and decodes unibyte high bytes as byte8 characters so byte identity survives any later multibyte promotion. This touches a core redisplay/string-encoding path shared by all mode-line formatting.
Changes:
- Add an input-driven
multibytefield toModeLineRendered, OR-ing it in across string/char/slice/truncation/append paths, and use it ininto_display_outputinstead of the content heuristic. - Decode unibyte high bytes via byte8 (
BYTE8_TO_CHAR) inmode_line_string_char_codesso raw bytes stay byte-faithful through promotion. - Add core unit, GNU-oracle, and TUI PTY regression tests covering dot identity, the full U+0080..00FF band, re-derivation stability, and a unibyte raw-byte parity control.
| File | Description |
|---|---|
crates/neovm-core/src/emacs_core/display/xdisp/mod.rs |
Adds the multibyte field and input-driven threading; updates unibyte decoding to byte8; registers the new test module. |
crates/neovm-core/src/emacs_core/display/xdisp/tests/mode_line_multibyte_identity.rs |
New core unit tests for mode-line multibyte identity (dot, band, re-derivation, unibyte control, above-band guard). |
crates/neovm-oracle-tests/tests/oracle_mode_line_flow.rs |
Adds two GNU-oracle parity cases asserting identity follows inputs and survives re-derivation. |
crates/neomacs-tui-tests/tests/issue_470.rs |
New TUI PTY regression asserting the dot stays a dot across forced mode-line updates with exact GNU parity. |
crates/neomacs-tui-tests/tests/tui.rs |
Registers the new issue_470 test module. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d3df587 to
57c5a6b
Compare
…470) `into_display_output' derived the rendered mode-line string's multibyte flag from the accumulated character codes ("any code > 0xFF"), so a mode line whose only non-ASCII characters live in U+0080..U+00FF -- e.g. the U+00B7 MIDDLE DOT from issue #470 -- was re-encoded as a UNIBYTE string of raw bytes on every later re-evaluation. A raw byte displays as GNU's octal escape (`\` + `%03o` of CHAR_TO_BYTE8, src/xdisp.c:8649-8662), which is exactly the reported "dot renders correctly, then turns into a literal `\267`". GNU never re-encodes: a string's multibyte flag follows its INPUTS (the `concat' rule -- multibyte iff any argument is multibyte), and `substring' keeps the source string's flag. `ModeLineRendered' now carries that input-driven flag through every append path (strings, chars, slices, re-derived renders, truncations), and the unibyte decode arm maps each high byte to its byte8 character (`BYTE8_TO_CHAR`) so the unibyte raw-byte identity survives promotion losslessly and re-truncates through CHAR_TO_BYTE8 exactly like GNU. Tests: 5 new unit tests cover the dot, the whole degrading band, the re-derivation transition, the unibyte raw-byte GNU-parity control, and the above-band guard.
…entity The PTY-pair regression boots a real GNU/neomacs pair, installs a mode line holding both a literal U+00B7 and `(string 183)`, and forces five full mode-line re-evaluations: neomacs must render `A·B·C` identical to GNU every time (pre-fix it degraded to `A\267B\267C`). Probed on real GNU 31.1: `(string 183)` is a MULTIBYTE string holding U+00B7 (princ emits c2 b7, the mode line shows the dot glyph), so both elements are identity cases, not legitimate raw-byte escapes. Two active-oracle parity cases in oracle_mode_line_flow.rs assert `format-mode-line` result identity through the GNU harness: multibyte iff its inputs were multibyte, surviving a re-derivation through a second `format-mode-line`. Expects blessed from Xvfb GNU 31.1 (NEOVM_ORACLE_MODE=refresh); neomacs snapshot mode matches. Verified: neovm-core mode-line families 133/133, xdisp/display 601/601, TUI pair green (red pre-fix), oracle refresh+snapshot 2/2, minibuffer_line_gui GUI smoke green.
57c5a6b to
87d4a87
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It alters core redisplay multibyte/unibyte encoding semantics for all mode-line strings, a subtle and high-impact path that warrants final human review despite the comprehensive tests.
Review effort: Balanced
Findings: None
Resolved since last review (1)
Closes #470.
Root cause
into_display_outputderived the rendered mode-line string's multibyte flag from its content (any(code > 0xFF)). A mode line whose only non-ASCII characters sit in U+0080..U+00FF — e.g. the reported·(U+00B7 MIDDLE DOT) — has every code inside one byte, so each later re-evaluation re-encoded the accumulated text as a unibyte string of raw bytes. GNU's redisplay renders a raw byte as the octal escape\+%03oofCHAR_TO_BYTE8(src/xdisp.c:8649-8662): exactly the reported "renders correctly, then turns into a literal\267".Fix
GNU never re-encodes: a string's multibyte identity follows its inputs (the
concatrule — multibyte iff any argument is multibyte;substringkeeps the source flag).ModeLineRenderednow carries that input-driven flag through every append path — strings, chars, string slices, re-derived renders, truncations — andinto_display_outputuses it instead of the content heuristic.The unibyte decode arm maps each high byte to its byte8 character (
BYTE8_TO_CHAR, src/character.h), so the char-code domain itself carries byte identity: byte8 codes make unibyte→multibyte promotion lossless (char_string's overlong raw-byte form) and the unibyte arm'sas u8is exactly GNU'sCHAR_TO_BYTE8.Empirical GNU 31.1 probes that pinned the oracle behavior:
(string 183)is multibyte holding U+00B7 (princemitsc2 b7); a PTY mode line(list "A" "·" "B" (string 183) "C")rendersA·B·C— both elements are identity cases, not legitimate raw-byte escapes. Pre-fix neomacs degraded both toA\267B\267C.Verification
mode_line_multibyte_identity, 5 tests, red pre-fix): dot, whole degrading band U+0080..00FF, re-derivation transition, unibyte raw-byte GNU-parity control, above-band guard — 5/5; mode-line families 133/133; xdisp/display 601/601.oracle_mode_line_flow.rs, 2 new cases):format-mode-lineresult identity follows inputs and survives re-derivation through a secondformat-mode-line. Expects blessed from live Xvfb GNU 31.1 (NEOVM_ORACLE_MODE=refresh); neomacs snapshot mode matches.issue_470.rs): real GNU/neomacs pair, forcedforce-mode-line-update t+ redisplay × 5, exact full-screen parity — green post-fix, red pre-fix.minibuffer_line_guigreen (identity is decided in core before any frontend).cargo fmt --all+cargo check --workspace --all-targetsclean.Full TUI sweep note
A full-suite run surfaced 8 failures outside this change: all 8 reproduce identically against a pre-fix baseline binary (base commit built in an isolated worktree +
CARGO_TARGET_DIR, rerun viaNEOMACS_TUI_NEOMACS_BIN). Decomposition: one wrap-fragile test predicate (message row wrapping depends on test PID length; GNU evaluates in 0.02 s), five vertico face-background divergences (Rgb(179,179,179)vs default), and the TTYscroll-bar-modeapplication divergence — pre-existing candidates for separate issues.