feat: IRCv3 draft/named-modes — PROP command, RPL_CHMODELIST, outbound helper#194
Merged
Conversation
Adds the protocol-layer scaffolding for IRCv3 draft/named-modes:
- draft/named-modes added to ourCaps so the client requests it on
connect.
- NamedModeSpec / NamedModes types on Server (in types/index.ts)
capture the long-form mode-name registry the server advertises.
- src/lib/irc/handlers/named-modes.ts parses the six new numerics
(960-965) plus the PROP command and emits per-event payloads
through the existing ircClient event bus.
- src/store/handlers/named-modes.ts stores the channel/user mode
registry under server.namedModes as RPL_CHMODELIST /
RPL_UMODELIST stream in.
- Dispatch wiring (IRC_DISPATCH + registerAllHandlers).
Phase 2 will use the registry to translate inbound PROP into the
existing chanmode/usermode store updates and emit MODE-equivalent
events for the rest of the UI; phase 3 adds outbound PROP for
cap-negotiated servers.
The named-modes registry from phase 1 now does real work:
- Inbound PROP commands: each item's long-form name is resolved to
its legacy MODE letter via server.namedModes; the result is fired
as a synthesised MODE event so every existing chanmode/usermode
handler (channel state, member-prefix updates for op/voice,
deafened/away tracking, etc.) keeps working unchanged. Items
that map to name-only modes (no letter) are dropped from the
MODE event and surface only via NAMED_MODES_PROP for any future
UI that wants the rich form.
- PROP-listing replies (961/960): buffered per server x channel and
flushed into the same RPL_CHANNELMODEIS path the legacy MODE
listing uses, so the chat-header / channel-settings modal pick
up channel mode state from PROP without further changes.
- Parser cleanup: the IRC client already strips the leading ':'
from trailing args, so the named-modes parser just flattens
every parv element on whitespace. Same path now handles both
"964 me 5:op=o 5:voice=v" (inline) and "964 me :5:op=o 5:voice=v"
(trailing) shapes uniformly.
7 new vitest cases in tests/protocol/named-modes.test.ts cover
single-line bursts, multi-line continuation markers, malformed and
unknown-type entries, name-only entries, and PROP item parsing
(including the default-+ branch).
Adds the outbound counterpart to phases 1+2: a single helper on
IRCClient that takes long-form mode-name items and chooses between
PROP (cap-required, can carry name-only modes) and MODE (universal,
letter-required) based on the negotiated registry the caller passes
in.
Decision matrix:
- all items have letters -> MODE (works anywhere)
- cap on + any item is name-only -> PROP (only wire form for these)
- cap off + name-only item -> drop unreachable item,
MODE-fall-through for the rest
The registry is taken as an explicit parameter rather than reached
into via the store, keeping the IRC layer free of a store dependency.
Callers (channel-settings modal, member context menu, etc.) already
have the per-server `namedModes` from the store and pass it through.
6 vitest cases pin the decision matrix: all-letters MODE, name-only
triggering PROP, cap-off dropping name-only, no-resolvable-items
no-send, user-target uses userModes registry, sign-collapsing.
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds draft/named-modes support: types and registry, client API (sendNamedMode + events), protocol handlers for PROP and numerics 960–965, store wiring to accumulate registries and bridge to legacy MODE, UI for channel advanced named-mode editing, utilities, and tests. ChangesNamed-Modes Feature Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Automated deployment preview for the PR in the Cloudflare Pages. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/protocol/named-modes.test.ts (1)
22-145: ⚡ Quick winAdd a regression test for registry replacement across bursts.
Current tests validate parsing and command selection well, but they don’t cover “old entry removed in later CHMODELIST/UMODELIST burst” behavior; adding this would prevent stale-mapping regressions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/protocol/named-modes.test.ts` around lines 22 - 145, Add a regression test that verifies registry replacement across multi-line bursts: use makeCtx() and call handleRplChmodelist (and/or handleRplUmodelist) in a sequence that simulates an initial burst containing an entry, then a subsequent burst that omits that entry (with continuation marker handling), and finally the terminating burst; after the final burst assert that the events/registry no longer contains the removed entry and that the entries reflect the latest burst only. Locate existing tests using makeCtx, handleRplChmodelist, handleRplUmodelist and events to mirror their structure and assertions (check ev.entries or mapped names) to confirm the old mapping was replaced rather than retained.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/irc/handlers/named-modes.ts`:
- Around line 209-223: Validate that parv[0] is present and a string before
using it as target and before any startsWith slicing/processing: in the block
around getNickFromNuh(source) and variable target, check typeof parv[0] ===
"string" (or truthiness) and if not, bail out (return) or skip emitting the PROP
event and optionally log a warning; apply the same guard to the similar code
block referenced at lines 235-242 so neither the mode parsing (tail assembly)
nor downstream store code will encounter undefined and call startsWith on it.
In `@src/store/handlers/named-modes.ts`:
- Around line 63-85: mergeEntries currently only upserts by name and never
removes stale entries; update mergeEntries to accept/handle a "first-of-burst"
signal (add a boolean parameter like isFirst or use an existing burst indicator)
and when that signal is true, clear or filter the previous registry before
merging: compute a Set of incoming names, start merged from an empty array (or
filter prev to only names present in incoming) and then upsert incoming entries
as before so entries missing from the new advertisement are removed; apply the
same change to the analogous routine referenced around lines 92-101 (the other
named-mode merge/registry function) so re-advertise reconnects won't retain
obsolete mappings.
---
Nitpick comments:
In `@tests/protocol/named-modes.test.ts`:
- Around line 22-145: Add a regression test that verifies registry replacement
across multi-line bursts: use makeCtx() and call handleRplChmodelist (and/or
handleRplUmodelist) in a sequence that simulates an initial burst containing an
entry, then a subsequent burst that omits that entry (with continuation marker
handling), and finally the terminating burst; after the final burst assert that
the events/registry no longer contains the removed entry and that the entries
reflect the latest burst only. Locate existing tests using makeCtx,
handleRplChmodelist, handleRplUmodelist and events to mirror their structure and
assertions (check ev.entries or mapped names) to confirm the old mapping was
replaced rather than retained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: feaf07f1-e9a8-48a4-ba47-6d27f1544ef3
📒 Files selected for processing (7)
src/lib/irc/IRCClient.tssrc/lib/irc/handlers/index.tssrc/lib/irc/handlers/named-modes.tssrc/store/handlers/index.tssrc/store/handlers/named-modes.tssrc/types/index.tstests/protocol/named-modes.test.ts
Comment on lines
+209
to
+223
| const sender = getNickFromNuh(source); | ||
| const target = parv[0]; | ||
| // Remaining args are the mode change items; the trailing one may | ||
| // start with `:` (IRC trailing-arg form) and may pack multiple | ||
| // space-separated entries. | ||
| const tail: string[] = []; | ||
| for (let i = 1; i < parv.length; i++) { | ||
| const piece = | ||
| i === parv.length - 1 && parv[i].startsWith(":") | ||
| ? parv[i].slice(1) | ||
| : parv[i]; | ||
| for (const t of piece.split(" ")) { | ||
| if (t.length) tail.push(t); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Guard malformed PROP payloads before emitting events.
On Line 210, target is read from parv[0] without validation. If missing, downstream store code calls startsWith on undefined and can crash event processing.
Suggested fix
export function handleProp(
ctx: IRCClientContext,
serverId: string,
source: string,
parv: string[],
mtags: Record<string, string> | undefined,
): void {
+ if (!parv[0]) return;
const sender = getNickFromNuh(source);
const target = parv[0];Also applies to: 235-242
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/irc/handlers/named-modes.ts` around lines 209 - 223, Validate that
parv[0] is present and a string before using it as target and before any
startsWith slicing/processing: in the block around getNickFromNuh(source) and
variable target, check typeof parv[0] === "string" (or truthiness) and if not,
bail out (return) or skip emitting the PROP event and optionally log a warning;
apply the same guard to the similar code block referenced at lines 235-242 so
neither the mode parsing (tail assembly) nor downstream store code will
encounter undefined and call startsWith on it.
Comment on lines
+63
to
+85
| /** Append the current line's entries to the running list. The first | ||
| * line clears any stale registry; the final line caps the burst. */ | ||
| function mergeEntries( | ||
| prev: NamedModeSpec[], | ||
| incoming: NamedModeSpec[], | ||
| isFinal: boolean, | ||
| ): NamedModeSpec[] { | ||
| // The protocol burst comes as `[*] ... [*] ... :final`. Each line is | ||
| // independent; we just concatenate. Callers can rely on isFinal to | ||
| // know when to read the registry. | ||
| // Dedup by name in case the server (or a future re-advertise) sends | ||
| // overlapping entries. | ||
| const merged: NamedModeSpec[] = [...prev]; | ||
| for (const entry of incoming) { | ||
| const idx = merged.findIndex((e) => e.name === entry.name); | ||
| if (idx === -1) merged.push(entry); | ||
| else merged[idx] = entry; | ||
| } | ||
| // isFinal could trigger downstream effects (e.g. "registry ready") | ||
| // -- left as a boolean for now since callers can derive from the | ||
| // store directly. | ||
| void isFinal; | ||
| return merged; |
Contributor
There was a problem hiding this comment.
Stale named-mode entries are never removed from registry.
On Line 75, merge logic only upserts by name and never clears entries missing from a new advertisement burst, so reconnect/re-advertise can keep obsolete mappings and generate incorrect MODE translations later.
Proposed direction
- function mergeEntries(prev, incoming, isFinal): NamedModeSpec[] { ...upsert... }
+ // Keep a per-server in-progress buffer and replace the registry on final burst.
+ // Pseudocode:
+ // - on first chunk: start empty buffer
+ // - on each chunk: append/upsert into buffer
+ // - on isFinal: assign `namedModes.channelModes = buffer` (or userModes), then clear bufferAlso applies to: 92-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/store/handlers/named-modes.ts` around lines 63 - 85, mergeEntries
currently only upserts by name and never removes stale entries; update
mergeEntries to accept/handle a "first-of-burst" signal (add a boolean parameter
like isFirst or use an existing burst indicator) and when that signal is true,
clear or filter the previous registry before merging: compute a Set of incoming
names, start merged from an empty array (or filter prev to only names present in
incoming) and then upsert incoming entries as before so entries missing from the
new advertisement are removed; apply the same change to the analogous routine
referenced around lines 92-101 (the other named-mode merge/registry function) so
re-advertise reconnects won't retain obsolete mappings.
… check
The "Advanced" tab in ChannelSettingsModal is gated on
`server.isUnrealIRCd`, which was set by exact-matching "UnrealIRCd" in
the RPL_YOURHOST (002) version string. ObbyIRCd is a downstream
UnrealIRCd fork that advertises its own name there ("ObbyIRCd-..."),
so the gate stayed false and channel ops on ObbyIRCd networks lost
the Advanced tab even though the underlying chanmode surface is the
same.
Match either name. Comment also clarifies that ObbyIRCd-only features
(e.g. named-modes) are detected via their own caps and should NOT be
conflated with the UnrealIRCd-parity flag.
3 tasks
When the server negotiates draft/named-modes, the channel-settings
"Advanced" tab now drops the hardcoded UnrealIRCd-specific layout
and renders one row per advertised channel mode straight from
`server.namedModes.channelModes`. Each row carries:
- a humanised label (humanizeNamedMode strips the vendor prefix
and title-cases the rest, so "obsidianirc/this-mode-lol"
becomes "This Mode Lol")
- a lower-cased vendor badge to the right (when the name is
vendored, e.g. "obsidianirc")
- the legacy `+x` letter hint (when one exists)
- a control derived from the spec's mode type:
4 (flag) -> checkbox
2/3 (param) -> text input ("" means unset)
1 (list) -> skipped (Bans/Exceptions/Invitations tabs cover it)
5 (prefix) -> skipped (member-prefix is set via member menu)
Apply path collects every staged change into a single sendNamedMode
call, which decides MODE vs PROP based on the registry + cap state
(see phase-3 helper). Pending diff is reset on modal close so a
stale draft doesn't leak across opens.
Tab visibility flag was also widened: Advanced now shows whenever
the server supports named-modes OR is UnrealIRCd-family. Plain
UnrealIRCd servers without the cap keep getting the legacy
hardcoded Advanced UI as a fallback.
The explainer text inside the Advanced tab is noise -- the user is already in a Channel Settings modal under an Advanced tab labelled 'Advanced', the visible content speaks for itself.
The vendor string is already lowercased in humanizeNamedMode but the badge had a Tailwind 'uppercase' class on it, undoing that. Drop the class and add an explicit 'lowercase' so any future caller who passes a mixed-case vendor in stays uniform.
The whole point of named-modes is to surface human-readable mode names. Carrying the legacy +letter alongside undermines that and adds visual noise without giving the user anything actionable.
Replace the auto-derived humanizeNamedMode labels in the Advanced tab with a curated registry (NAMED_MODE_META) that maps each known mode to a proper label, description, parameter placeholder, and section group. Auto-derivation produced rows like 'Floodprot' / 'Regonlyspeak' that weren't useful UX. The registry covers the obsidianirc/* modes plus the spec-table modes worth surfacing, hides modes already covered by other tabs (op/voice/ban/topiclock/...) and server-managed flags (issecure/isregistered/delayjoin-rejoinhide), and renders sectioned groups (Flood / Filtering / Behavior / Access / Properties). Unknown modes fall back to humanizeNamedMode for the label so newly added vendor modes still render.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ui/ChannelSettingsModal.tsx (1)
234-240:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
pendingNamedModesis not cleared when the channel changes.The reset only fires on
!isOpen(line 238). The effect at 1015 already supportschannelNamechanging while the modal stays open (it resetsmodesand re-fetches). Staged named-mode edits from the previous channel persist across that switch and would be applied to the new channel viaircClient.sendNamedMode(serverId, channelName, items, ...)at line 1002 — i.e. edits intended for#aget sent to#b.♻️ Suggested fix — also reset on channel change
// biome-ignore lint/correctness/useExhaustiveDependencies: Using channelName instead of channel to avoid infinite loop from object reference changes useEffect(() => { if (isOpen && channel) { // Clear current modes and fetch new ones when channel changes setModes([]); + setPendingNamedModes({}); hasFetchedRef.current = false; fetchChannelModes(); } }, [isOpen, channelName, fetchChannelModes]);Also applies to: 1014-1022
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/ChannelSettingsModal.tsx` around lines 234 - 240, The useEffect that currently resets hasFetchedRef, clears timeouts and calls setPendingNamedModes({}) only when !isOpen fails to clear staged edits when the modal stays open but channelName changes; update that effect (the one referencing hasFetchedRef.current, clearPendingTimeouts, setPendingNamedModes) to also run/reset when channelName changes (add channelName to the dependency list) so pendingNamedModes is cleared on channel switches, and make the same change to the similar effect around the modes re-fetch (the block interacting with ircClient.sendNamedMode) so staged named-mode edits cannot be sent to the wrong channel.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/ui/ChannelSettingsModal.tsx`:
- Around line 1050-1060: liveStateForNamedMode is leaking the "__HIDDEN__"
sentinel into the UI by returning stored verbatim; change it to treat the
sentinel as "not exposable" and return param: null (or "" per callers) when
stored === "__HIDDEN__" so the input and equality checks (which compare
stagedParam to live.param ?? "") behave correctly; update references that use
liveStateForNamedMode (originalModesRef, stagedParam handling, and the equality
check around mode staging) to expect null/empty for hidden values rather than
the literal "__HIDDEN__".
- Around line 1170-1190: The empty-input handling should branch on live.set
instead of comparing to (live.param ?? ""): in the input onChange for
stagedParam, change the logic so that if v === "" you call
stageNamedMode(spec.name, { sign: "-" }) when live.set is true (to stage an
unset) and stageNamedMode(spec.name, null) when live.set is false (to clear
staging); otherwise, if v is non-empty use stageNamedMode(spec.name, { sign:
"+", param: v }), and if v matches the current live.param exactly clear staging
with stageNamedMode(spec.name, null). Update the onChange in the component
(referencing stagedParam, live, spec.name, and stageNamedMode) accordingly.
---
Outside diff comments:
In `@src/components/ui/ChannelSettingsModal.tsx`:
- Around line 234-240: The useEffect that currently resets hasFetchedRef, clears
timeouts and calls setPendingNamedModes({}) only when !isOpen fails to clear
staged edits when the modal stays open but channelName changes; update that
effect (the one referencing hasFetchedRef.current, clearPendingTimeouts,
setPendingNamedModes) to also run/reset when channelName changes (add
channelName to the dependency list) so pendingNamedModes is cleared on channel
switches, and make the same change to the similar effect around the modes
re-fetch (the block interacting with ircClient.sendNamedMode) so staged
named-mode edits cannot be sent to the wrong channel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 974d274d-4184-4b17-a041-315e0b845349
📒 Files selected for processing (5)
src/components/ui/ChannelSettingsModal.tsxsrc/lib/ircUtils.tsxsrc/lib/namedModeRegistry.tssrc/store/handlers/channels.tssrc/store/handlers/named-modes.ts
✅ Files skipped from review due to trivial changes (3)
- src/lib/ircUtils.tsx
- src/store/handlers/channels.ts
- src/lib/namedModeRegistry.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/store/handlers/named-modes.ts
Comment on lines
+1050
to
+1060
| const liveStateForNamedMode = ( | ||
| spec: NamedModeSpec, | ||
| ): { set: boolean; param: string | null } => { | ||
| if (!spec.letter) return { set: false, param: null }; | ||
| const stored = originalModesRef.current[spec.letter]; | ||
| if (stored === undefined) return { set: false, param: null }; | ||
| // null = present, no param. string = present with param. "__HIDDEN__" | ||
| // (used for +k/+L masking) counts as present without an exposable | ||
| // value. | ||
| return { set: true, param: stored }; | ||
| }; |
Contributor
There was a problem hiding this comment.
__HIDDEN__ sentinel leaks into the named-modes input and breaks unset behavior.
liveStateForNamedMode returns stored verbatim, so for hidden-value modes (e.g. key/H/L) the literal string "__HIDDEN__" flows through stagedParam (line 1110) into <input value={stagedParam}> and is rendered to the user. The comment at lines 1056–1058 explicitly says hidden values are "without an exposable value", but the implementation exposes them. The legacy path already handles this correctly (lines 395–417: parsedModes.k !== "__HIDDEN__" ? parsedModes.k || "" : "").
A second consequence is in the equality check at line 1175: when the live param is "__HIDDEN__", clearing the input produces v === (live.param ?? "") ⇒ "" === "__HIDDEN__" ⇒ false, and the code stages { sign: "-" }, silently unsetting a hidden mode the user never intended to remove.
🛡️ Suggested fix — treat the sentinel as not exposable
const liveStateForNamedMode = (
spec: NamedModeSpec,
): { set: boolean; param: string | null } => {
if (!spec.letter) return { set: false, param: null };
const stored = originalModesRef.current[spec.letter];
if (stored === undefined) return { set: false, param: null };
- // null = present, no param. string = present with param. "__HIDDEN__"
- // (used for +k/+L masking) counts as present without an exposable
- // value.
- return { set: true, param: stored };
+ // null = present, no param. string = present with param.
+ // "__HIDDEN__" (used for +k/+L masking) is set but not exposable —
+ // surface it as an opaque "set" so we don't render the sentinel
+ // and don't accidentally unset it on empty input.
+ if (stored === "__HIDDEN__") return { set: true, param: null };
+ return { set: true, param: stored };
};Also applies to: 1108-1110, 1170-1190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/ChannelSettingsModal.tsx` around lines 1050 - 1060,
liveStateForNamedMode is leaking the "__HIDDEN__" sentinel into the UI by
returning stored verbatim; change it to treat the sentinel as "not exposable"
and return param: null (or "" per callers) when stored === "__HIDDEN__" so the
input and equality checks (which compare stagedParam to live.param ?? "") behave
correctly; update references that use liveStateForNamedMode (originalModesRef,
stagedParam handling, and the equality check around mode staging) to expect
null/empty for hidden values rather than the literal "__HIDDEN__".
Comment on lines
+1170
to
+1190
| <input | ||
| type="text" | ||
| value={stagedParam} | ||
| onChange={(e) => { | ||
| const v = e.target.value; | ||
| if (v === (live.param ?? "")) { | ||
| stageNamedMode(spec.name, null); | ||
| } else if (!v) { | ||
| stageNamedMode(spec.name, { sign: "-" }); | ||
| } else { | ||
| stageNamedMode(spec.name, { sign: "+", param: v }); | ||
| } | ||
| }} | ||
| placeholder={ | ||
| live.set | ||
| ? "(set, edit to change)" | ||
| : (placeholder ?? "(not set)") | ||
| } | ||
| className="flex-1 p-2 bg-discord-dark-400 text-white rounded text-sm" | ||
| /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Empty-input edge case can leave a parameterized mode set when the user wants it unset.
At line 1175, when live.set === true and live.param === null (a parameterized mode currently set without an exposed param), an empty input yields v === (null ?? "") ⇒ true, so staging is cleared instead of staging { sign: "-" }. The user clears the field expecting an unset, but the apply path emits no diff for this mode.
♻️ Suggested fix — branch on live.set rather than live.param equality
onChange={(e) => {
const v = e.target.value;
- if (v === (live.param ?? "")) {
- stageNamedMode(spec.name, null);
- } else if (!v) {
- stageNamedMode(spec.name, { sign: "-" });
- } else {
- stageNamedMode(spec.name, { sign: "+", param: v });
- }
+ if (!v) {
+ // Empty: revert to live state if not set, else stage unset.
+ stageNamedMode(spec.name, live.set ? { sign: "-" } : null);
+ } else if (live.set && v === (live.param ?? "")) {
+ stageNamedMode(spec.name, null);
+ } else {
+ stageNamedMode(spec.name, { sign: "+", param: v });
+ }
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <input | |
| type="text" | |
| value={stagedParam} | |
| onChange={(e) => { | |
| const v = e.target.value; | |
| if (v === (live.param ?? "")) { | |
| stageNamedMode(spec.name, null); | |
| } else if (!v) { | |
| stageNamedMode(spec.name, { sign: "-" }); | |
| } else { | |
| stageNamedMode(spec.name, { sign: "+", param: v }); | |
| } | |
| }} | |
| placeholder={ | |
| live.set | |
| ? "(set, edit to change)" | |
| : (placeholder ?? "(not set)") | |
| } | |
| className="flex-1 p-2 bg-discord-dark-400 text-white rounded text-sm" | |
| /> | |
| </div> | |
| <input | |
| type="text" | |
| value={stagedParam} | |
| onChange={(e) => { | |
| const v = e.target.value; | |
| if (!v) { | |
| // Empty: revert to live state if not set, else stage unset. | |
| stageNamedMode(spec.name, live.set ? { sign: "-" } : null); | |
| } else if (live.set && v === (live.param ?? "")) { | |
| stageNamedMode(spec.name, null); | |
| } else { | |
| stageNamedMode(spec.name, { sign: "+", param: v }); | |
| } | |
| }} | |
| placeholder={ | |
| live.set | |
| ? "(set, edit to change)" | |
| : (placeholder ?? "(not set)") | |
| } | |
| className="flex-1 p-2 bg-discord-dark-400 text-white rounded text-sm" | |
| /> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ui/ChannelSettingsModal.tsx` around lines 1170 - 1190, The
empty-input handling should branch on live.set instead of comparing to
(live.param ?? ""): in the input onChange for stagedParam, change the logic so
that if v === "" you call stageNamedMode(spec.name, { sign: "-" }) when live.set
is true (to stage an unset) and stageNamedMode(spec.name, null) when live.set is
false (to clear staging); otherwise, if v is non-empty use
stageNamedMode(spec.name, { sign: "+", param: v }), and if v matches the current
live.param exactly clear staging with stageNamedMode(spec.name, null). Update
the onChange in the component (referencing stagedParam, live, spec.name, and
stageNamedMode) accordingly.
# Conflicts: # src/lib/irc/IRCClient.ts # src/types/index.ts
# Conflicts: # src/types/index.ts
# Conflicts: # src/lib/irc/IRCClient.ts # src/store/handlers/index.ts # src/types/index.ts
# Conflicts: # src/lib/irc/IRCClient.ts # src/types/index.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the IRCv3 draft/named-modes spec on the client end-to-end.
Server-side support for this lives in ObbyIRCd PR #8; both can ship together.
What lands
Phase 1 — protocol scaffolding (
bd6c9c2..1aaf884)draft/named-modestoourCapsso we negotiate it on connect.NamedModeSpec/NamedModesonServer.src/lib/irc/handlers/named-modes.tsparses six new numerics:RPL_CHMODELIST(964) /RPL_UMODELIST(965) — long-form mode registryRPL_PROPLIST(961) /RPL_ENDOFPROPLIST(960) — channel mode-state listingRPL_LISTPROPLIST(963) /RPL_ENDOFLISTPROPLIST(962) — list-mode entriesPROPcommand itself.server.namedModes.Phase 2 — inbound translation (
1aaf884..6b9e9df)PROPresolves long-form names to legacy MODE letters via the registry and re-fires a syntheticMODEevent so every existing chanmode/usermode handler (channel state, prefix updates, etc.) keeps working unchanged.PROP <chan>listing replies are buffered per(server, channel)and flushed into the sameRPL_CHANNELMODEISpath the legacyMODElisting uses.NAMED_MODES_PROPevent — they have no MODE-letter equivalent to translate to.964 me 5:op=o 5:voice=v) and trailing (964 me :5:op=o 5:voice=v) wire shapes uniformly.Phase 3 — outbound helper (
6b9e9df..90ab7b8)IRCClient.sendNamedMode(serverId, target, items, registry?)chooses between PROP and MODE based on cap state + whether any item is name-only.MODE(works anywhere)PROP(only wire form that can carry these)MODEfallback for the restTests
13 new vitest cases in
tests/protocol/named-modes.test.ts:+branch and space-packed trailing itemsPipeline locally: 720 / 721 pass (was 707), build clean.
What's not in this PR
MODE-via-sendRawcall sites (ban list ops, channel-settings modal mode toggles) tosendNamedMode. Those keep working unchanged — they just won't get name-only mode reach until they switch over. Done as follow-ups so this PR stays focused on the spec implementation.Test plan
draft/named-modes-aware server (obby.t3ks.com), confirm CAP REQ goes out and:server 964 me ...shows in raw log/mode #chan +o mefrom a non-cap client; we receive:them PROP #chan +op=usand the chat header / member list update/modefrom this client still works on the wireSummary by CodeRabbit
New Features
Tests
Chores