apps/ircc: fix text formatting issues (#376) #392

2026-03-29T04:29:41Z

SumitKumar-17 commented

2026-03-29 04:29:41 +00:00

First-time contributor

This PR Fixes

Leading whitespace stripped from messages
skip_parameter and window_open both ate spaces after the trailing-param :. Now preserved per RFC 1459/2812.
\x0F (format reset) ignored
Previously a no-op, so the last \x03 color bled to end-of-line. Now resets to default in render and is zero-width in line-wrap.
Control bytes inflated line width
\x02 bold, \x1D italic, \x1F underline, \x16 reverse, and mIRC \x03[fg[,bg]] args were each counted as a visible column, causing premature wrapping. All now zero-width, matching the renderer.
First byte after \x03N,N was silently eaten by a .no_colors fallthrough — now jmp .line.
Bare \x03 now resets to theme default instead of picking irc_colors[0] by accident.
text_nextline is also called backwards during scroll-up — a DF check in .skip_color avoids esi corruption.
New skip_mirc_color_args consumes the exact same byte span as the renderer's dec_to_esi, replacing two duplicated blocks.

Remaining (out of scope)

Visual bold/italic/underline rendering is not implemented.
Width accounting is fixed so wrapping is correct, but draw_channel_text still skips those bytes without changing font attributes — needs an attribute-aware draw path, a separate larger change.

## This PR Fixes - **Leading whitespace stripped from messages** `skip_parameter` and `window_open` both ate spaces after the trailing-param `:`. Now preserved per RFC 1459/2812. - **`\x0F` (format reset) ignored** Previously a no-op, so the last `\x03` color bled to end-of-line. Now resets to default in render and is zero-width in line-wrap. - **Control bytes inflated line width** `\x02` bold, `\x1D` italic, `\x1F` underline, `\x16` reverse, and mIRC `\x03[fg[,bg]]` args were each counted as a visible column, causing premature wrapping. All now zero-width, matching the renderer. - First byte after `\x03N,N` was silently eaten by a `.no_colors` fallthrough — now `jmp .line`. - Bare `\x03` now resets to theme default instead of picking `irc_colors[0]` by accident. - `text_nextline` is also called backwards during scroll-up — a DF check in `.skip_color` avoids `esi` corruption. - New `skip_mirc_color_args` consumes the exact same byte span as the renderer's `dec_to_esi`, replacing two duplicated blocks. ## Remaining (out of scope) - **Visual bold/italic/underline rendering is not implemented.** Width accounting is fixed so wrapping is correct, but `draw_channel_text` still skips those bytes without changing font attributes — needs an attribute-aware draw path, a separate larger change.

SumitKumar-17 added 1 commit 2026-03-29 04:31:08 +00:00

ircc: fix text formatting issues (#376 )

Build system / Check kernel codestyle (pull_request) Successful in 52s

Details

Build system / Build (pull_request) Successful in 10m19s

Details

c449fb9d71

fix leading whitespaces being trimmed ,formatting clear chars ignored,
bold/italic/other control bytes counted toward line width in
text_insert_newlines and text_nextline, causing incorrect wrapping,
leading spaces stripped from PRIVMSG content

SumitKumar-17 force-pushed fix/376-irc-text-formatting from 85b0585685 to c449fb9d71

2026-03-29 04:31:08 +00:00

Compare

Burer requested review from Burer 2026-05-18 06:45:21 +00:00

Burer requested review from Doczom 2026-05-18 06:45:22 +00:00

Burer requested review from dunkaist 2026-05-18 06:45:22 +00:00

Burer requested review from hidnplayr 2026-05-18 06:45:22 +00:00

Burer approved these changes 2026-05-18 11:18:13 +00:00

Dismissed

Burer left a comment

Issues I see here:

Digit-parser mismatch between width calc and renderer.
The new .skip_color parser in text_insert_newlines/text_nextline consumes at most 2 fg + 2 bg digits, but the renderer's dec_to_esi consumes all consecutive digits. For non-standard color codes with 3+ digits (e.g. \x03123), line-wrap width and rendered output desync.
Make the width parser consume the same byte span as dec_to_esi.
Undocumented behavior change in draw_channel_text.
The full \x03fg,bg path now does jmp .line instead of falling through to .no_colors, removing an implicit inc edx that previously ate the first character after a background-color code. This looks like a genuine bug fix, but it is outside the PR's stated scope and not mentioned in the description.
Please confirm if it is intentional or not.
New code duplication introduced by this PR.
The ~30-line .skip_color/.skip_color_comma/.skip_color_done block is added verbatim to both text_insert_newlines and text_nextline.
Both copies are correct, but since the duplication is created here, factor it into a shared helper while it's fresh.

Issues I see here: - [x] **Digit-parser mismatch between width calc and renderer.** The new `.skip_color` parser in `text_insert_newlines`/`text_nextline` consumes at most 2 fg + 2 bg digits, but the renderer's `dec_to_esi` consumes *all* consecutive digits. For non-standard color codes with 3+ digits (e.g. `\x03123`), line-wrap width and rendered output desync. Make the width parser consume the same byte span as `dec_to_esi`. - [x] **Undocumented behavior change in `draw_channel_text`.** The full `\x03fg,bg` path now does `jmp .line` instead of falling through to `.no_colors`, removing an implicit `inc edx` that previously ate the first character after a background-color code. This looks like a genuine bug fix, but it is outside the PR's stated scope and not mentioned in the description. Please confirm if it is intentional or not. - [x] **New code duplication introduced by this PR.** The ~30-line `.skip_color`/`.skip_color_comma`/`.skip_color_done` block is added verbatim to both `text_insert_newlines` and `text_nextline`. Both copies are correct, but since the duplication is created here, factor it into a shared helper while it's fresh.

Burer requested changes 2026-05-18 11:18:53 +00:00

Burer left a comment

See comment above.

Burer added 1 commit 2026-05-20 09:29:42 +00:00

apps/ircc: refactor color-arg skipping, fix width/render desync

Build system / Check kernel codestyle (pull_request) Successful in 57s

Details

Build system / Build (pull_request) Successful in 10m27s

Details

eae9ab3518

- Factor duplicated .skip_color from text_insert_newlines and
  text_nextline into a shared skip_mirc_color_args matching the
  renderer's dec_to_esi byte span.
- Guard text_nextline backward scans (DF check) so forward-parsing
  \x03 args doesn't corrupt esi during scroll-up.
- Document the silent jmp .line that stops .no_colors from eating
  the byte after \x03N,N.
- Document the .color_reset branch for bare \x03.

dunkaist approved these changes 2026-05-20 11:47:10 +00:00

hidnplayr commented

2026-05-20 13:13:39 +00:00

The color code counting logic seems overly complicated at first sight.
Did I really wrote a routine that counts characters backwards?
From what I remember, we just might need to scan backwards to find newlines.

How did you validate its correct behavior?

The color code counting logic seems overly complicated at first sight. Did I really wrote a routine that counts characters backwards? From what I remember, we just might need to scan backwards to find newlines. How did you validate its correct behavior?

Burer changed title from ~~ircc: fix text formatting issues (#376)~~ to apps/ircc: fix text formatting issues (#376)

2026-06-07 16:57:16 +00:00

Build system / Check kernel codestyle (pull_request) Successful in 57s

Details

Build system / Build (pull_request) Successful in 10m27s

Details

Test PR / Build *

Required

Checking for merge conflicts…

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u http://git.kolibrios.org/SumitKumar-17/kolibrios fix/376-irc-text-formatting:SumitKumar-17-fix/376-irc-text-formatting

git checkout SumitKumar-17-fix/376-irc-text-formatting

Sign in to join this conversation.

No Reviewers

Doczom

hidnplayr

Burer

dunkaist

4 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KolibriOS/kolibrios#392

Allow edits from maintainers