Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 52 additions & 6 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,29 @@ fallbacks) for inline images/maps.
- **A highlight rule needs no route to reach the pane a capture rule sent the line to.** There is one
line and one set of destinations, and every matched rule's highlight is on it. Do not "fix" that into
a per-rule delivery.
- **A prompt reaches the pane again, and everywhere a line reaches** (`WorldSession.OnOutputReceived`,
Core; `fix(prompt)`, `cd06e8b`). `22bd97f`'s header/status-bar rewrite deleted the one render that ever
read `CurrentPrompt`, so a prompt was parsed and stored but shown nowhere. **A prompt is now an ordinary
line through `ProcessOutputLine`**, the same function every other line of output runs through, so it
reaches scrollback and the spill, `PlainTextLogSink`/`HtmlLogSink`, `RestoreLog`, `OutputSearch`, the
trigger/alias/macro engines, and the unread/activity/away bars, exactly as a line the server terminated
with its own newline would. **On a MUD that prompts after every command, this is one extra line per
command** — the cost of the fix, not a defect in it; Mudlet does the same, and it is what
"searchable and trigger-visible" requires. Two decisions not to relitigate.
**It lands exactly once.** `ProcessOutputLine` is handed the pre-emoji `raw` line, not the
already-substituted one built for `CurrentPrompt`/`PromptChanged` — handing it the substituted line
would run emoji substitution twice. **A prompt boundary with no printable run prints no line.** A burst
that ends in bare SGR (`ESC[0m`) or an erase sequence and then goes quiet for the hold buffers nothing in
`AnsiParser`, so `Flush()` returns null; `CurrentPrompt`/`PromptChanged` still update, but
`ProcessOutputLine` is not called — that is a boundary, not a blank line the server sent, and is the
opposite case from `fix(line)`'s genuinely blank line, which does print. Getting this backwards means a
blank line lands in every buffer once per prompt, forever.
- **The line carries `StyledLine.IsPrompt`**, set once at the end of `ProcessOutputLine` on the line
actually delivered — not threaded through `StripColour`/`ExpandTabs`/the trigger engine/emoji
substitution. It travels through `LinePrinted` and `SpawnLineEventArgs` for free, since both already
carry a `StyledLine`, and round-trips through `StyledLineCodec` (a previously-unused flag bit, so no
`RestoreLog` format-version bump). **Wired, not yet acted on**: nothing today gags, logs, restores,
searches or badges a prompt any differently from an ordinary line because of this bit.

## Building and testing

Expand Down Expand Up @@ -1278,7 +1301,27 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/]
permitted transitions from `StartNegotiation`; before it each one hit `OnUnhandledTriggerAsync`,
logging Critical and recovering through `Trigger.Error`, which on some interleavings ate the
sequence behind it. **2.10.0** carries the raw bytes of each MSSP value beside the decoded
string. **2.11.0** is the prompt-marker fix below.
string. **2.11.0** is the prompt-marker fix below. **2.12.0** is the one that actually motivated
a further jump: it ships **`PacketPatchProtocol`**, included in `AddDefaultMUDProtocols`
automatically (an appended, defaulted `packetPatchHoldTime` parameter this codebase doesn't set)
and infers a prompt boundary from 500ms of silence for servers that mark none at all.
`IProtocolContext` is unchanged in 2.12.0: the prompt-boundary members
(`TakePartialLineAsPrompt`, `HasPartialLine`, `HasSeenMarkedPrompt`) are public on
`TelnetInterpreter`, reachable through the interface's existing `Interpreter` property, so there
is no public break to migrate for. The
`CallbackOnByteAsync` property this file reaches by reflection is unchanged across the whole jump:
still `public Func<byte, Encoding, ValueTask>? CallbackOnByteAsync { get; init; }` on
`TelnetInterpreter`.
- **`cd06e8b` (the prompt render) and `04e0e26` (this 2.12.0 bump) must not be separated.** Reverting
the render alone while 2.12.0 stays pinned sends every packet-patch-inferred prompt fragment to
`CurrentPrompt`, which nothing renders — the text is simply gone, silently. Before 2.12.0 that same
fragment was merely glued to the head of the next line, which reads as a cosmetic wrap issue rather
than lost text. Revert both together or neither.
- **Never pin a version that is not on nuget.org, and never work around it with a local feed.** A
pin that only resolves from a local `nuget.config` builds here and fails `NU1101` everywhere else;
if a locally-built nupkg of that version is already in `~/.nuget/packages/`, the real package
publishing later fails `NU1403` (content mismatch) until that cache directory is deleted. Delete
the stale cache; do not commit a `nuget.config` pointing at `/tmp`.
- **A prompt ends with `IAC EOR` or `IAC GA`, and until 2.11.0 this client could only see the first
of them.** `TelnetSession.OnPromptAsync` is the only thing that flushes `_pending` — the
unterminated line `CallbackOnByteAsync` accumulates — so a server whose prompt boundary the library
Expand Down Expand Up @@ -1446,11 +1489,14 @@ markup (`[bold #rrggbb on #rrggbb]…[/]`, `[[`/`]]` escaping, `[link=url]…[/]
2066 settles on — and naming one is an *override*: still offered at the head of the order so a
cooperative server agrees, but used regardless of what it says. Four things about this library will
bite you, and all four already have:
- **`TelnetInterpreter.CurrentEncoding` defaults to `Encoding.ASCII`**, and that default is not inert:
it is handed to `CallbackOnByteAsync`/`CallbackOnSubmitAsync` for every byte and used for GMCP, MSDP
and everything we send. On a server that never negotiates CHARSET — most MU\* servers — every
byte above 0x7F became `?`. `TelnetSession` seeds that property (reflectively, `internal set`, the
same way `CharsetProtocol` itself writes it) with the head of the stated order.
- **`TelnetInterpreter.CurrentEncoding` defaults to `Encoding.UTF8`** (`TelnetStandardInterpreter.cs:46`),
and that default is not something to rely on unseeded: it is handed to
`CallbackOnByteAsync`/`CallbackOnSubmitAsync` for every byte and used for GMCP, MSDP and everything
we send, before this session's own encoding decision (an override, or the head of `CharsetOrder`)
has had any say. `TelnetSession` seeds that property (reflectively, `internal set`, the same way
`CharsetProtocol` itself writes it) with the head of the stated order — otherwise a world pinned to
a non-UTF-8 override, or a server that never negotiates CHARSET at all, would decode against
whichever encoding the library happens to default to rather than what this session decided.
**MSSP fields were decoded as `Encoding.ASCII` through 2.6.x** (fixed in 2.7.0, pinned by
`MsspParsingTests`); treat non-ASCII in an MSSP field from an older library as unrecoverable.
Two consequences worth knowing from the pre-fix era: the plaintext `MSSP-REQUEST` fallback went
Expand Down
2 changes: 1 addition & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<PackageVersion Include="SharpConsoleUI" Version="2.5.14" />

<!-- Telnet negotiation: TELOPT/NAWS/MTTS/EOR/CHARSET/MSSP/GMCP/MSDP/MCCP/MXP negotiation. -->
<PackageVersion Include="TelnetNegotiationCore" Version="2.11.0" />
<PackageVersion Include="TelnetNegotiationCore" Version="2.12.0" />

<!-- Sandboxed Lua scripting (pure-managed, no native deps). -->
<PackageVersion Include="MoonSharp" Version="2.0.0" />
Expand Down
27 changes: 23 additions & 4 deletions src/SharpMUTerm.Core/Session/WorldSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -406,10 +406,25 @@ private void OnOutputReceived(object? sender, TelnetOutputEventArgs e)
if (e.IsPrompt)
{
_parser.Feed(e.Text);
var raw = _parser.Flush() ?? StyledLine.Empty;
var prompt = ApplyEmoji(_text?.StripIncomingColour == true ? StyledText.StripColour(raw) : raw);
var flushed = _parser.Flush();
var raw = flushed ?? StyledLine.Empty;
var prompt = ApplyEmoji(_text?.StripIncomingColour == true ? StyledText.StripColour(raw) : raw)
.WithPrompt(true);
CurrentPrompt = prompt;
PromptChanged?.Invoke(this, prompt);

// Uses raw, not prompt: ProcessOutputLine runs StripIncomingColour/ExpandTabs/triggers/
// ApplyEmoji itself, so the already-substituted line would double-substitute.
//
// Only when flushed is not null. A prompt boundary with no printable run (a bare SGR
// reset, an erase sequence) makes Flush() return null — that is a boundary, not a blank
// line the server sent, unlike the deliberate ProcessOutputLine(StyledLine.Empty) call
// below. Do not merge the two.
if (flushed is not null)
{
ProcessOutputLine(raw, isPrompt: true);
}

return;
}

Expand Down Expand Up @@ -442,7 +457,11 @@ private void OnOutputReceived(object? sender, TelnetOutputEventArgs e)
0,
TextSettings.MaxTabWidth);

private void ProcessOutputLine(StyledLine line)
/// <summary>
/// <paramref name="isPrompt"/> is not threaded through the pipeline below (colour-strip, tab
/// expand, triggers, emoji) — it is applied once, at the end, to the line actually delivered.
/// </summary>
private void ProcessOutputLine(StyledLine line, bool isPrompt = false)
{
// Colour is stripped from what the *server* sent, before the triggers run: a highlight rule
// and this client's own system/echo lines are not "incoming ANSI colour" and keep theirs.
Expand Down Expand Up @@ -470,7 +489,7 @@ private void ProcessOutputLine(StyledLine line)
// rather than before it, so a link's target is exactly the text under it — a span whose visible
// text and destination disagree is the shape of a phishing link, and this client should not be
// in the business of manufacturing one.
var shown = ApplyLinks(ApplyEmoji(result.Line));
var shown = ApplyLinks(ApplyEmoji(result.Line)).WithPrompt(isPrompt);

foreach (var target in result.SpawnTargets)
{
Expand Down
36 changes: 23 additions & 13 deletions src/SharpMUTerm.Core/Telnet/TelnetSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,14 @@ public sealed class TelnetSession : ITelnetSession

// TelnetInterpreter.CurrentEncoding has an internal setter, which CharsetProtocol itself writes
// through reflection once negotiation settles. We seed it the same way at connect time, for two
// reasons. It *defaults to Encoding.ASCII*, and that default is not inert: it is handed to
// CallbackOnByteAsync/CallbackOnSubmitAsync for every byte, and used to decode GMCP, MSDP and MSSP
// payloads and to encode everything we send — so before negotiation, or on the many MU* servers
// that never implement RFC 2066 at all, every byte above 0x7F became '?'. And because the seed is
// an instance nothing else can produce, "has CHARSET settled?" becomes an exact reference
// comparison rather than a guess about a value that might legitimately be ASCII.
// reasons. It *defaults to Encoding.UTF8* (TelnetStandardInterpreter.cs:46), and that default is
// not inert: it is handed to CallbackOnByteAsync once per byte and to CallbackOnSubmitAsync once
// per submitted line, and used to decode GMCP, MSDP and MSSP payloads and to encode everything we
// send — so before negotiation, a
// world pinned to a non-UTF-8 override, or a server that never implements RFC 2066 at all, would
// decode against UTF-8 regardless of what this session decided. And because the seed is an
// instance nothing else can produce, "has CHARSET settled?" becomes an exact reference comparison
// rather than a guess about a value that might legitimately be UTF-8.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
private static readonly PropertyInfo? InterpreterEncodingProperty =
typeof(TelnetInterpreter).GetProperty(nameof(TelnetInterpreter.CurrentEncoding)) is { CanWrite: true } p
? p
Expand Down Expand Up @@ -291,8 +293,10 @@ public TelnetSession(ITransport transport, ILogger? logger = null, TelnetSession
/// <para>
/// Precedence is override, then negotiation, then the head of the stated preference order. The
/// last arm is what a server that never speaks CHARSET lands on: the app's first preference,
/// normally UTF-8. It is emphatically <em>not</em> the interpreter's own <c>Encoding.ASCII</c>
/// default, which is what used to reach the decode path and mangle every non-ASCII byte.
/// normally UTF-8. It is emphatically <em>not</em> the interpreter's own unseeded
/// <c>Encoding.UTF8</c> default (<c>TelnetStandardInterpreter.cs:46</c>) reached before this
/// session's own decision — an override to a non-UTF-8 charset, or the head of a non-default
/// <see cref="TelnetSessionOptions.CharsetOrder"/> — has had any say.
/// </para>
/// </summary>
public SessionEncoding CurrentEncoding
Expand Down Expand Up @@ -423,17 +427,23 @@ private Task<TelnetInterpreter> BuildInterpreterAsync()
}

/// <summary>
/// Replaces the interpreter's <see cref="Encoding.ASCII"/> default with what this session would
/// otherwise assume, so nothing decodes as ASCII merely because negotiation hasn't happened yet —
/// including the GMCP/MSDP/MSSP payloads and outbound bytes the library encodes for itself. The
/// library overwrites this the moment CHARSET settles.
/// Replaces the interpreter's unseeded <see cref="Encoding.UTF8"/> default
/// (<c>TelnetStandardInterpreter.cs:46</c>) with what this session would otherwise assume, so
/// nothing decodes against that default merely because negotiation hasn't happened yet —
/// including the GMCP/MSDP/MSSP payloads and outbound bytes the library encodes for itself. A
/// world pinned to a non-UTF-8 override is exactly the case this protects: without seeding, its
/// bytes would decode as UTF-8 until CHARSET settles or forever on a server that never negotiates
/// it. The library overwrites this the moment CHARSET settles.
/// </summary>
private void SeedInterpreterEncoding(TelnetInterpreter interpreter)
{
if (InterpreterEncodingProperty is null)
{
_logger.LogWarning(
"TelnetInterpreter.CurrentEncoding is not writable; pre-negotiation payloads will decode as ASCII.");
"TelnetInterpreter.CurrentEncoding is not writable; pre-negotiation GMCP/MSDP/MSSP payloads and "
+ "outbound bytes the library encodes for itself will decode/encode against its unseeded UTF-8 "
+ "default regardless of this session's own encoding decision. Ordinary output lines are "
+ "unaffected — OnSubmitAsync decodes those with this session's own CurrentEncoding.Encoding.");
return;
}

Expand Down
23 changes: 21 additions & 2 deletions src/SharpMUTerm.Core/Text/StyledLine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ public sealed class StyledLine
private readonly StyledSpan[] _spans;
private string? _text;

public StyledLine(IEnumerable<StyledSpan> spans, TerminalColor? ruleColor = null)
public StyledLine(IEnumerable<StyledSpan> spans, TerminalColor? ruleColor = null, bool isPrompt = false)
{
ArgumentNullException.ThrowIfNull(spans);
_spans = spans.Where(s => s.Length > 0).ToArray();
RuleColor = ruleColor;
IsPrompt = isPrompt;
}

/// <summary>
Expand All @@ -24,8 +25,26 @@ public StyledLine(IEnumerable<StyledSpan> spans, TerminalColor? ruleColor = null
/// </summary>
public TerminalColor? RuleColor { get; }

/// <summary>
/// True when this line <em>is</em> a prompt — a telnet <c>IAC GA</c>/<c>EOR</c> boundary, or
/// TelnetNegotiationCore's packet-patch silence heuristic, rather than a line the server
/// terminated with its own newline (<see cref="WorldSession.OnOutputReceived"/>).
/// <para>
/// Set once, at the end of <c>WorldSession.ProcessOutputLine</c>, on the line actually delivered
/// to <c>LinePrinted</c>/<c>SpawnLine</c> — not threaded through <c>StripColour</c>,
/// <c>ExpandTabs</c>, the trigger engine or emoji substitution, none of which are asked to
/// preserve it. A consumer can tell a prompt from an ordinary line; as of this flag's
/// introduction nothing yet treats one differently (what gags, logs, restores, searches or
/// badges is unchanged).
/// </para>
/// </summary>
public bool IsPrompt { get; }

/// <summary>Returns a copy of this line carrying the given trigger-highlight rule colour.</summary>
public StyledLine WithRule(TerminalColor color) => new(_spans, color);
public StyledLine WithRule(TerminalColor color) => new(_spans, color, IsPrompt);

/// <summary>Returns a copy of this line with <see cref="IsPrompt"/> set to <paramref name="isPrompt"/>.</summary>
public StyledLine WithPrompt(bool isPrompt) => isPrompt == IsPrompt ? this : new(_spans, RuleColor, isPrompt);

/// <summary>An empty line (a blank row of output).</summary>
public static StyledLine Empty { get; } = new(Array.Empty<StyledSpan>());
Expand Down
18 changes: 14 additions & 4 deletions src/SharpMUTerm.Core/Text/StyledLineCodec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace SharpMUTerm.Core.Text;
/// 7-bit-length-prefixed UTF-8, so text needs no escaping and may contain any character including
/// control bytes and newlines):
/// <code>
/// byte flags bit 0: a RuleColor follows
/// byte flags bit 0: a RuleColor follows; bit 1: line.IsPrompt
/// colour ruleColor (present only when bit 0 is set)
/// 7bit spanCount
/// spanCount times:
Expand All @@ -44,6 +44,14 @@ public static class StyledLineCodec
{
private const byte FlagHasRuleColor = 1 << 0;

/// <summary>
/// A previously-unused bit added for <see cref="StyledLine.IsPrompt"/>. Backward-compatible by
/// construction, so this did not need a <c>RestoreLog</c> format-version bump: a record written
/// before this bit existed always has it clear, and reading that as "not a prompt" is simply
/// correct — that data genuinely predates the concept, not a value being reinterpreted.
/// </summary>
private const byte FlagIsPrompt = 1 << 1;

/// <summary>
/// The encoding used for every string field. Replacement (not exception) fallback, so an
/// unpaired surrogate — which nothing in the inbound pipeline can produce, since text is decoded
Expand All @@ -57,7 +65,7 @@ public static void Write(BinaryWriter writer, StyledLine line)
ArgumentNullException.ThrowIfNull(writer);
ArgumentNullException.ThrowIfNull(line);

var flags = (byte)(line.RuleColor is null ? 0 : FlagHasRuleColor);
var flags = (byte)((line.RuleColor is null ? 0 : FlagHasRuleColor) | (line.IsPrompt ? FlagIsPrompt : 0));
writer.Write(flags);
if (line.RuleColor is { } rule)
{
Expand Down Expand Up @@ -102,6 +110,7 @@ public static StyledLine Read(BinaryReader reader)

var flags = reader.ReadByte();
TerminalColor? rule = (flags & FlagHasRuleColor) != 0 ? ReadColor(reader) : null;
var isPrompt = (flags & FlagIsPrompt) != 0;

var spanCount = reader.Read7BitEncodedInt();
if (spanCount < 0)
Expand All @@ -111,7 +120,8 @@ public static StyledLine Read(BinaryReader reader)

if (spanCount == 0)
{
return rule is null ? StyledLine.Empty : StyledLine.Empty.WithRule(rule.Value);
var empty = rule is null ? StyledLine.Empty : StyledLine.Empty.WithRule(rule.Value);
return empty.WithPrompt(isPrompt);
}

var spans = new StyledSpan[spanCount];
Expand All @@ -136,7 +146,7 @@ public static StyledLine Read(BinaryReader reader)
spans[i] = new StyledSpan(text, style, interaction);
}

return new StyledLine(spans, rule);
return new StyledLine(spans, rule, isPrompt);
}

/// <summary>Encodes one line's payload to a fresh array (convenience for tests and callers off the hot path).</summary>
Expand Down
Loading
Loading