Skip to content

fix(standard-reply): show only the human-readable description#197

Merged
ValwareIRC merged 1 commit into
mainfrom
fix/standard-reply-ui
May 11, 2026
Merged

fix(standard-reply): show only the human-readable description#197
ValwareIRC merged 1 commit into
mainfrom
fix/standard-reply-ui

Conversation

@ValwareIRC
Copy link
Copy Markdown
Contributor

@ValwareIRC ValwareIRC commented May 10, 2026

The IRCv3 standard-replies spec is explicit that <description> is the end-user-facing text and <command>/<code> are computer-readable. The old UI inverted that — bolded FAIL AUTHENTICATE SASL_FAIL at the top and demoted the actual message — and the parser was also corrupting the description in two ways:

  1. parv.slice(N).join(" ").substring(1) chopped the first character off every description (the parser already strips the leading :).
  2. parv[2] was unconditionally treated as the context, so FAIL CMD CODE :Description (no context) was rendered as a chip with the description text and an empty body.

This refactors the FAIL/WARN/NOTE/SUCCESS/REGISTER handlers to use the trailing argument as the description and to slice context from parv[2..n-1]. The UI now renders only the description plus optional context chips for channels/nicks/accounts the description references. The technical command/code is still on the row's title for hover.

Summary by CodeRabbit

  • New Features

    • Standard reply notifications now display context as chips alongside messages
    • Enhanced hover tooltips showing comprehensive message metadata
  • Bug Fixes

    • Improved handling of empty descriptions with fallback text display
  • Tests

    • Added test suite for standard reply message parsing and rendering

Review Change Stack

The IRCv3 standard-replies spec is explicit that `<description>` is the
end-user-facing text and `<command>`/`<code>` are computer-readable. The
old UI inverted that — bolded `FAIL AUTHENTICATE SASL_FAIL` at the top
and demoted the actual message — and the parser was also corrupting the
description in two ways:

1. `parv.slice(N).join(" ").substring(1)` chopped the first character
   off every description (the parser already strips the leading `:`).
2. `parv[2]` was unconditionally treated as the context, so `FAIL CMD
   CODE :Description` (no context) was rendered as a chip with the
   description text and an empty body.

This refactors the FAIL/WARN/NOTE/SUCCESS/REGISTER handlers to use the
`trailing` argument as the description and to slice context from
`parv[2..n-1]`. The UI now renders only the description plus optional
context chips for channels/nicks/accounts the description references.
The technical command/code is still on the row's `title` for hover.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aca8d9bd-19ca-4974-9d58-98ebf5888c1b

📥 Commits

Reviewing files that changed from the base of the PR and between 6843862 and 1ea223a.

📒 Files selected for processing (9)
  • src/components/message/MessageItem.tsx
  • src/components/ui/StandardReplyNotification.tsx
  • src/lib/irc/IRCClient.ts
  • src/lib/irc/handlers/auth.ts
  • src/lib/irc/handlers/index.ts
  • src/store/handlers/users.ts
  • src/types/index.ts
  • tests/components/StandardReplyNotification.test.tsx
  • tests/lib/standardReplyHandlers.test.ts

📝 Walkthrough

Walkthrough

This PR adds machine-readable context array support to IRCv3 standard replies. It updates IRC protocol parsing to extract context tokens, propagates context through event routing and store handlers, refactors the StandardReplyNotification component to render context as chips with metadata tooltips, and adds comprehensive test coverage for both parsing logic and UI rendering.

Changes

Standard Reply Context Feature

Layer / File(s) Summary
Data Types & Contracts
src/types/index.ts, src/lib/irc/IRCClient.ts
Message interface adds standardReplyContext?: string[] field; EventMap extends WARN, NOTE, and SUCCESS events with context: string[] array.
IRC Wire Protocol Parsing
src/lib/irc/handlers/auth.ts
New splitStandardReply(parv, trailing) helper parses IRCv3 standard replies into command, code, context array, and message. All auth handlers (FAIL, WARN, NOTE, SUCCESS, REGISTER) refactored to accept trailing parameter and use the helper for structured parsing.
Event Router Wiring
src/lib/irc/handlers/index.ts
IRC_DISPATCH routes for FAIL, WARN, NOTE, SUCCESS, and REGISTER updated to capture and forward trailing parameter to handler functions.
Store Event Handlers
src/store/handlers/users.ts
WARN and NOTE handlers refactored to accept context field, select target channel from current selection (or first available), emit structured standard-reply messages with context, and fall back to global notifications when no channel exists.
UI Component Implementation
src/components/ui/StandardReplyNotification.tsx
Component now accepts optional context?: string[] prop; computes icon and colors inline instead of via helper functions; renders context (or fallback target) as chips; builds hover title with type, command, code, and context; trims message with "(no description)" fallback; removes header line with command/code.
Message Component Wiring
src/components/message/MessageItem.tsx
MessageItem passes message.standardReplyContext to StandardReplyNotification via new context prop when rendering standard-reply messages.
Tests & Verification
tests/components/StandardReplyNotification.test.tsx, tests/lib/standardReplyHandlers.test.ts
Component tests refactored to verify human-readable description visibility, context chip rendering, metadata in title attribute, and empty-message fallback behavior; new handler test suite validates IRCv3 wire parsing and event payload structure with regression coverage for description truncation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ObsidianIRC/ObsidianIRC#77: Updates StandardReplyNotification styling; related at code level to this PR's component refactoring.

Suggested reviewers

  • matheusfillipe

Poem

🐰 Context hops through the IRC wire,
Standard replies climb ever higher,
Chips align with metadata bright,
Parsing complete, descriptions just right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: refactoring standard-reply UI to prioritize human-readable description over technical command/code details.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/standard-reply-ui

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

Pages Preview
Preview URL: https://fix-standard-reply-ui.obsidianirc.pages.dev

Automated deployment preview for the PR in the Cloudflare Pages.

@ValwareIRC ValwareIRC merged commit e8c2335 into main May 11, 2026
4 checks passed
@ValwareIRC ValwareIRC deleted the fix/standard-reply-ui branch May 11, 2026 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants