Warnings as errors + built-in analyzers (#55), user-facing changelog (#54) - #57
Merged
Merged
Conversation
) Why: Issue #55 asks for warnings-as-errors at the maximum warning level and zero-dependency static analyzers, with a sweep of the analyzer findings. What changed: - Per-assembly csc.rsp (-warnaserror, -warn:4) beside the Runtime, Editor, and Tests.Runtime asmdefs, shipped with the package. - Generator~/Directory.Build.props: built-in Roslyn analyzers at AnalysisMode=All with analyzer diagnostics as errors; the generator-tests project keeps a documented NoWarn contract list for public-API-shape rules. - CommandArg: every culture-sensitive TryParse pins NumberStyles and InvariantCulture; same-type Convert.ChangeType calls became direct casts; Contains(char) became an ordinal IndexOf. - RegisterCommandAttribute: NormalizeName validates its argument, space stripping is ordinal, redundant initializer removed. - New PlayMode culture-invariance cases (tr-TR, de-DE, ru-RU, fr-FR).
Why: Issue #55 wants zero user-facing error/warning messages during normal operation and minimal, non-spammy logs. What changed: - TerminalUI.InitializeFont no longer logs when the font pack yields its default font; one warning only when the pack has no fonts (the previous error misclassified a recovering fallback, and the previous warning fired on every default enable). - TerminalUI.InitializeTheme no longer logs when nothing is persisted; the warning now names the stale persisted theme only when one exists and the pack does not contain it. - TerminalThemePersister drops the per-start preamble log and the full-JSON dump; remaining progress logs are gated to Editor and development builds.
Why: Issue #54 asks that the changelog record only what a package consumer can observe, with the rule enforced through instructions. What changed: - CHANGELOG.md states the user-facing-only policy in its header; internal style-sweep and generator-allocation entries and internal timings removed; entries added for culture-invariant parsing, warnings-as-errors, and console silence. - .llm/context.md gains a CHANGELOG section defining the policy and where internal numbers belong instead.
wallstop
commented
Sep 11, 2026
wallstop
commented
Sep 11, 2026
wallstop
commented
Sep 11, 2026
wallstop
commented
Sep 11, 2026
…57 review) Why: PR review asked to relocate the invariant parser locals out of CommandArg.TryGet and make them accessible for testing and reuse. What changed: - New public static CommandArgParsers (20 parsers, one per built-in type); CommandArg dispatches through it. - Generator tests list the new file in their explicit source sets. - CHANGELOG gains an Added entry for the new public type. How we know: - Generator tests 30/30; PlayMode 192/192 on Unity 6000.4.6f1 including the culture-invariance cases through the new call path.
This was referenced Sep 11, 2026
…eview) Why: PR review asked for a repo rule: multi-line comments are block comments, never stacked '//' lines. What changed: - New lint-multiline-comments.mjs (Node ESM): a state machine over the same literal scanner the comparison-direction linter uses, so comment-looking lines inside strings, verbatim fixture sources, interpolated holes, and block comments stay data. '--fix' converts runs to the block shape with a three-space content indent; a run whose content contains the block-comment close is refused, and an empty walk fails instead of reading green. - 12 contract tests; wired into pre-commit, the cross-OS csharp-style CI job, and context.md rule 21. - Swept 68 stacked '//' runs across Runtime, Editor, Tests, and Generator~ into block comments; verbatim fixture strings untouched. How we know: - Node tooling 121 pass / 0 fail; generator tests 30/30; CSharpier, all linters, meta and package validators green.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 92d414e. Configure here.
Why: The fixer replaces whole lines, so counting trailing comments (a '//' after code on its line) as run members would delete the code they trail. What changed: - commentRuns records only comment-only lines; trailing comments and runs around them stay outside the rule. - Contract tests pin the destructive-rewrite shapes.
wallstop
added a commit
that referenced
this pull request
Sep 12, 2026
…e hardening (#59 #58 #56) (#60) 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 #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 #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 (#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. <!-- CURSOR_SUMMARY --> --- > [!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. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 42f9b37. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This was referenced Sep 13, 2026
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.
DISCLOSURE: LLM-GENERATED TEXT
Why:
Fresh maintainer asks #55 (warnings as errors, maximum warning level, zero-dependency
static analyzers, silent console in normal operation) and #54 (user-facing-only
changelog), plus reviewer feedback during this PR. Master CI verified green before
starting; draft PRs #27/#29 stay superseded per #40.
What changed:
csc.rsp(-warnaserror,-warn:4) beside the Runtime, Editor, andTests.Runtime asmdefs, shipped with the package; a per-assembly response file replaces
any project-level
csc.rspfor these assemblies only.Generator~/Directory.Build.props: built-in Roslyn analyzers atAnalysisMode=Allwith analyzer diagnostics as errors; the generator-tests project keeps a documented
NoWarn contract list for public-API-shape rules the additive-only plan forbids changing.
CommandArgparsing is culture-invariant (every culture-sensitiveTryParsepinsNumberStyles+InvariantCulture); same-typeConvert.ChangeTypebecame directcasts. The parsers now live in the new public
CommandArgParsersclass (review ask).only for real misconfiguration; theme persistence progress logs are
Editor/development-build only.
lint-multiline-comments.mjslinter (pre-commit + CI + context.md rule 21,review ask): stacked
//comment runs become block comments; 68 runs swept.How we know:
CS0414(rsp honored), thenclean; PlayMode 192/192 after every change batch; console shows no package-owned
warnings in normal paths.
byte-compare OK. Node tooling 121 pass; meta/llm/csharp-style linters and the
package-content validator pass.
Follow-ups opened: #56 (caret-test frame timing), #58 (more built-in argument
types), #59 (table-driven TryGet dispatch refactor).