fix(ui): reserve pad height per message to prevent multi-line clip and scroll desync #136

Manually merged
jabber.developer2 merged 1 commits from fix/multiline-pad-clip into master 2026-06-20 09:41:17 +00:00
Collaborator

Problem. Scrolling up through chat history jumps by ~one page when passing a tall multi-line message. Long single-line (soft-wrapped) messages don't trigger it — only messages with embedded newlines.

Root cause. _win_print_internal grew the ncurses pad from the current cursor only (_win_ensure_pad_capacity(getcury)), not the height of the message about to be printed. A tall multi-line message printed near the pad bottom got clipped, so its captured height was wrong (e.g. 1 row for a 49-row message). win_redraw later re-rendered it in full, and the resulting buffer->lines / y_start_pos mismatch desynced the page-up scroll anchor → the jump. Regression from the upstream sync 72f4f186d, which replaced the fixed-size pad with the dynamic _win_ensure_pad_capacity model.

Fix. Estimate the message's rendered height (hard newlines + soft-wrap over the usable width) and reserve that many rows before printing, so it's never clipped and its captured height matches the redraw. One spot in _win_print_internal → covers all print paths (incoming/outgoing/history).

Verification. Reproduced on a real flatfile history (terminal 212×47 and 74×219). Instrumented logs showed the clipped message captured as 1 row vs 49 on redraw (Δ+61 total → ~page jump); with the fix, per-message captured height == redraw height for all entries, buffer->lines stays consistent, and the jump is gone. Builds clean (-Werror, full features); clang-format clean.

Problem. Scrolling up through chat history jumps by ~one page when passing a tall multi-line message. Long single-line (soft-wrapped) messages don't trigger it — only messages with embedded newlines. Root cause. _win_print_internal grew the ncurses pad from the current cursor only (_win_ensure_pad_capacity(getcury)), not the height of the message about to be printed. A tall multi-line message printed near the pad bottom got clipped, so its captured height was wrong (e.g. 1 row for a 49-row message). win_redraw later re-rendered it in full, and the resulting buffer->lines / y_start_pos mismatch desynced the page-up scroll anchor → the jump. Regression from the upstream sync 72f4f186d, which replaced the fixed-size pad with the dynamic _win_ensure_pad_capacity model. Fix. Estimate the message's rendered height (hard newlines + soft-wrap over the usable width) and reserve that many rows before printing, so it's never clipped and its captured height matches the redraw. One spot in _win_print_internal → covers all print paths (incoming/outgoing/history). Verification. Reproduced on a real flatfile history (terminal 212×47 and 74×219). Instrumented logs showed the clipped message captured as 1 row vs 49 on redraw (Δ+61 total → ~page jump); with the fix, per-message captured height == redraw height for all entries, buffer->lines stays consistent, and the jump is gone. Builds clean (-Werror, full features); clang-format clean.
jabber.developer2 added 1 commit 2026-06-20 08:46:17 +00:00
fix(ui): reserve pad height per message to prevent multi-line clip and scroll desync
All checks were successful
CI Code / Check spelling (pull_request) Successful in 16s
CI Code / Check coding style (pull_request) Successful in 31s
CI Code / Code Coverage (pull_request) Successful in 3m28s
CI Code / Linux (debian) (pull_request) Successful in 4m44s
CI Code / Linux (ubuntu) (pull_request) Successful in 4m57s
CI Code / Linux (arch) (pull_request) Successful in 6m49s
de172f3533
A tall multi-line message printed near the bottom of the ncurses pad was clipped because _win_print_internal grew the pad from the current cursor only (_win_ensure_pad_capacity(getcury)), not the height of the message about to be printed. The clipped message's captured height (e.g. 1 row for a 49-row message) was wrong while win_redraw later rendered it in full; the resulting buffer->lines / y_start_pos mismatch desynced the page-up scroll anchor, producing a ~one-page jump when scrolling past such messages from history.

Estimate the rendered height (hard newlines + soft-wrap over the usable width) and reserve that many rows before printing, so the message is never clipped and its captured height matches the redraw. All print paths go through _win_print_internal, so incoming/outgoing/history are covered.

Regression from the upstream sync (72f4f186d), which replaced the fixed-size pad with the dynamic _win_ensure_pad_capacity model.
jabber.developer2 force-pushed fix/multiline-pad-clip from de172f3533 to adb078a3e2 2026-06-20 09:00:42 +00:00 Compare
jabber.developer2 changed title from WIP: fix(ui): reserve pad height per message to prevent multi-line clip and scroll desync to fix(ui): reserve pad height per message to prevent multi-line clip and scroll desync 2026-06-20 09:02:11 +00:00
jabber.developer approved these changes 2026-06-20 09:20:02 +00:00
Dismissed
jabber.developer left a comment
Owner

LGTM

LGTM
src/ui/window.c Outdated
@@ -70,0 +73,4 @@
_win_estimated_lines(WINDOW* win, const char* const message, int indent, int pad_indent)
{
int usable = getmaxx(win) - (indent + pad_indent);
if (usable < 1) {

We might also use MAX(1, ...) instead, but this variant is also fine.

We might also use `MAX(1, ...)` instead, but this variant is also fine.
Author
Collaborator

Agreed, used MAX instead

Agreed, used MAX instead
jabber.developer marked this conversation as resolved
src/ui/window.c Outdated
@@ -70,0 +77,4 @@
usable = 1;
}
int newlines = 0;
for (const char* p = message; *p; p++) {

There are guarantees that messages are null-terminated, right?

There are guarantees that messages are null-terminated, right?
Author
Collaborator

Yes — message is a NUL-terminated C string. The buffer stores the body via strdup (STRDUP_OR_NULL, buffer.c), and _win_print_internal already relies on NUL-termination above this point: strncmp(message, "/me ", 4), wprintw("%s", message + offset), and _win_print_wrapped calls strlen(message). The estimate loop uses the same invariant — nothing new. (message + offset is also safe: offset is 4 only when strncmp(message, "/me ", 4) matched, i.e. the string is ≥ 4 chars.)

Yes — message is a NUL-terminated C string. The buffer stores the body via strdup (STRDUP_OR_NULL, buffer.c), and _win_print_internal already relies on NUL-termination above this point: strncmp(message, "/me ", 4), wprintw("%s", message + offset), and _win_print_wrapped calls strlen(message). The estimate loop uses the same invariant — nothing new. (message + offset is also safe: offset is 4 only when strncmp(message, "/me ", 4) matched, i.e. the string is ≥ 4 chars.)
jabber.developer marked this conversation as resolved
jabber.developer2 force-pushed fix/multiline-pad-clip from adb078a3e2 to 13fa1bf108 2026-06-20 09:25:20 +00:00 Compare
jabber.developer approved these changes 2026-06-20 09:31:44 +00:00
jabber.developer left a comment
Owner

LGTM

LGTM
jabber.developer force-pushed fix/multiline-pad-clip from 13fa1bf108 to b0e486367e 2026-06-20 09:38:18 +00:00 Compare
jabber.developer force-pushed fix/multiline-pad-clip from b0e486367e to 2d3d1ced71 2026-06-20 09:40:34 +00:00 Compare
jabber.developer2 manually merged commit 2d3d1ced71 into master 2026-06-20 09:41:17 +00:00
Sign in to join this conversation.
No description provided.