Table-driven argument parsing, six new built-in types, caret lifecycle hardening (#59 #58 #56) by wallstop · Pull Request #60 · wallstop/DxCommandTerminal

wallstop · 2026-09-11T19:36:13Z

DISCLOSURE: LLM-GENERATED TEXT

Why:

Closes #59 (table-driven TryGet dispatch), closes #58 (new built-in
argument types), addresses #56 (frame-timing-sensitive token-completion
caret tests). Master CI was green at 8917558; draft PRs #27/#29 stay
superseded per #40.

What changed:

CommandArg.TryGet<T> dispatches through a static
Type -> strongly typed parser-delegate table (36 built-ins). Precedence
unchanged (per-call override -> registered -> string -> named constants ->
table -> enum); culture invariance from Warnings as errors + built-in analyzers (#55), user-facing changelog (#54) #57 preserved; no public API change.
All composite parsers are now public per-type CommandArgParsers methods
(Vector2..RectInt, Color, Quaternion) sharing one split helper.
New built-in types: Bounds, BoundsInt, RectOffset
(RectOffset(left, right, top, bottom) order), Plane, Ray,
System.Numerics.Complex.
TerminalUI.ApplyPendingCaret consumes the queued caret only once the
write sticks on a focused pass: the panel can re-clamp a programmatic
caret write, so consume-on-first-write lost that race (fixes a real caret
regression found while hardening Token-completion caret tests are frame-timing sensitive under no-domain-reload session sequences #56's tests).
Token-completion UI tests: every post-CompleteCommand value/caret read
now polls with a bounded helper instead of one yield return null;
unfocused marker behavior is pinned synchronously.
Generator-test Unity shim gains Bounds/BoundsInt/RectOffset/Plane/Ray
stubs so the Unity-free lane keeps compiling the real Runtime sources.
run-terminal-tests skill gains UI-test polling rules and the
domain-reload-clears environment note.

How we know:

PlayMode 199/199 on Unity 6000.4.6f1 (192 prior + 7 new) on a fresh
domain; filtered caret suites green on consecutive re-runs. The known
editor poisoned-session mode (Token-completion caret tests are frame-timing sensitive under no-domain-reload session sequences #56) still requires a domain reload to
clear; the runbook is in the skill.
Generator 30/30; shipped payload byte-compare OK. CSharpier, all three
C# linters, meta lint, LLM-instructions lint, and node tooling (121)
pass. CHANGELOG carries one user-facing Added entry.

Note

Medium Risk
Broad changes to core CommandArg parsing and shell/UI hot paths; behavior is intended to stay equivalent but regressions would affect every command argument and completion/caret timing.

Overview
This PR tightens runtime/editor performance and parsing in one pass: it bans LINQ under Runtime/ and Editor/ (new lint-linq-production in CI and pre-commit) and rewrites call sites to explicit loops, CopyTo, and caller-owned buffers so hot paths do not pay for enumerators or throwaway collections.

CommandArg now dispatches built-ins through static parser tables (generic and untyped), removing reflection from TryGet(Type, out object) and folding composite parsing into public CommandArgParsers methods. Six new built-ins are added: Bounds, BoundsInt, RectOffset, Plane, Ray, and Complex, with Unity ToString()-style labels accepted where documented.

Supporting allocation work includes CachedStringBuilder for repeated string assembly, CommandShell.ClearVariables() with a cached key snapshot, CommandHistory.CopyHistory for completion, and TerminalUIEditor caching of inspector popup/font-key arrays across OnGUI redraws. TerminalUI.ApplyPendingCaret only clears the pending caret once the write sticks while focused, addressing frame-coupled completion/caret tests; the run-terminal-tests skill documents bounded polling for UI reads.

Docs and CHANGELOG record the LINQ policy and parser additions; the generator Unity shim grows stubs for the new geometry types.

^{Reviewed by Cursor Bugbot for commit 42f9b37. Bugbot is set up for automated code reviews on this repo. Configure here.}

…/Complex fallbacks - CommandArgParsers: per-type parse functions (bounds, ray, complex) with all-delimiter coverage; matches the existing split-based contract. - Fixed stale CA rule references after the analyzer swap; kept the documented format notes. - Full pass under the CA rule set; no behavior changes. Refs #104

- ApplyPendingCaret consumes the queued caret only once the write sticks on a focused pass; the panel can re-clamp a programmatic caret write, so consume-on-first-write lost that race. - Token-completion UI tests poll value and caret through bounded helpers instead of single-frame reads; a queued-position fallback covers panels that defer applying caret state. - Pin unfocused marker behavior synchronously in PendingCaretWritesAndKeepsMarkerWhileUnfocused. - run-terminal-tests skill gains the polling rules and the domain-reload-clears environment note (issue #56). - CHANGELOG: single console delimiter wording. PlayMode 199/199 on Unity 6000.4.6f1 (fresh domain).

cursor

Stale Bugbot comment from a previous run.

Bugbot: the new types rejected the types' own ToString output while Color and Rect strip theirs, so pasted log lines failed to parse. - Strip the Center:/Extents:, Position:/Size:, normal:/distance:, and Origin:/Dir: labels per component; bare positional input keeps its reading. - Bounds reads its ToString trailing triple as extents (half the size) and doubles it; label presence decides, not the value. - RectOffset has no structured ToString (plain class); the parser doc and a negative test pin that. - Data-driven ToString round-trip tests for all four types. PlayMode 199/199 on Unity 6000.4.6f1.

Every LINQ operator allocates enumerators and closures, and several copy the whole sequence; in a zero-allocation-first runtime library that can only be a silent regression. Removes all LINQ from shipped code: - Hot paths: CleanedContents and TrySplitComponents Aggregate chains become plain foreach loops; completion history walks use a new CommandHistory.CopyHistory fill method; TryGet's enum cache drops an OfType().ToArray() copy for a direct array cast. - Cold paths rewritten as loops or StringBuilder joins (shell discovery logs, built-in theme/font listings, editor pack pickers and command caches); Enumerable.Empty fallbacks become Array.Empty or cached empty lists; GetHistory keeps its IEnumerable contract via yield. - New lint-linq-production.mjs (pre-commit + csharp-style CI + context.md rule 22) fails any using System.Linq, System.Linq. call, or static Enumerable. call under Runtime/ and Editor/, with 7 contract tests. No --fix by design: removing LINQ is a per-site code decision. PlayMode 199/199 on Unity 6000.4.6f1; generator 30/30; payload byte-compare OK; tooling 127 pass; all linters green.

cursor

Stale Bugbot comment from a previous run.

Reviewer follow-up on 102bd87: removing LINQ must not re-introduce the allocations through loops. - New CachedStringBuilder (ThreadStatic rent/return, capacity retained); theme/font listings and the shell rejected-signature message rent instead of allocating per call. - TerminalUIEditor popup arrays and font-key arrays are cached against their sources (reference + count stamps) and rebuilt only on change; no per-frame ToArray() in OnGUI. - Bulk variable clear moved into CommandShell.ClearVariables with a cached snapshot buffer. - foreach over List<T> with first/last separators instead of counting loops (context.md rule 11); Counting loops kept only where the index is genuinely used. - context.md rule 23 codifies the allocation discipline. PlayMode 199/199 on Unity 6000.4.6f1; tooling 127 pass; linters green.

cursor

Stale Bugbot comment from a previous run.

…ling Review round on 499a917, fixes for 14 points: - CommandArg.TryGet(Type, out object): untyped parser adapters replace MakeGenericMethod+Invoke - the non-generic path is reflection-free at dispatch (IL2CPP/WebGL safe); registered parsers mirror into the same delegate shape at registration time. - TerminalKeyboardController.ControlTypes: written out explicitly, no Enum.GetValues at runtime; the all-members test guards drift. - CommandShell: rejected signatures store their formatted text at rejection time, so the error pass never reflects; ClearVariables is capture-count-and-clear. - TerminalUIEditor: one RefreshCache<T> builder for PackNames/FriendlyThemeNames; one RefreshKeyCache builder using ICollection<string>.CopyTo (bulk); stale flags set where _fontsByPrefix is rebuilt (Bugbot: same reference plus same count after an in-place refill kept stale keys); FriendlyThemeName only allocates when a marker is present; RegisteredCommands iterated via var so System.Reflection leaves the file. - CachedStringBuilder.Scope: amortized zero-allocation struct for using statements; replaces try/finally at the rental sites. - lint-linq-production.mjs masks comments and string literals with the comparison-direction scanner, so banned vocabulary in block comments or strings cannot trip it (Bugbot); 5 new contract cases. PlayMode 199/199 on Unity 6000.4.6f1; generator 30/30; payload byte-compare OK; tooling 132 pass.

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8f66994. Configure here.}

@event

Bugbot round on 8f66994: - FriendlyThemeNames passes FriendlyThemeName as its transform; the identity delegate dropped the -theme/theme- strip from the popup. - stripComments null-checks consumeLiteral (verbatim identifiers like @event, dangling quotes copy verbatim instead of throwing); 2 new contract cases pin both shapes. PlayMode 199/199 on Unity 6000.4.6f1; tooling 132 pass.

Review follow-up on 0315323: 'ensure we have tests to verify that all built-in types are registered.' - CommandArg exposes BuiltInParserTypes (internal static property over the table's keys; callers cannot mutate it). - New BuiltInParserTableCoversEveryBuiltInType PlayMode test: the table must match the expected type list exactly (a removed or stale entry fails the count/membership), and each row drives the untyped TryGet(Type, out object) path with a representative input plus a junk-rejection check, so a table entry that stops parsing is caught here even without a dedicated per-type test. Adding a built-in type without a table entry and a row here fails this test. PlayMode 200/200 on Unity 6000.4.6f1.

wallstop added 2 commits September 11, 2026 18:58

cursor Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread Runtime/CommandTerminal/Backend/CommandArgParsers.cs