diff --git a/.claude/skills/commit-messages/SKILL.md b/.claude/skills/commit-messages/SKILL.md new file mode 100644 index 0000000000..3159328907 --- /dev/null +++ b/.claude/skills/commit-messages/SKILL.md @@ -0,0 +1,72 @@ +--- +name: commit-messages +description: MUST use before writing any git commit message in this repository. Covers the gitlint rules CI enforces (title/body length, blank line, trailing punctuation) and how to write a compliant message on the first try. +--- + +# Commit Messages + +CI (`.github/workflows/CommitMessage.yml`) runs `gitlint` against every +commit on a PR and fails the build on any violation. `.gitlint` in the repo +root only exempts `Agent-Logs-Url:` and the Copilot Autofix co-author +trailer from the body-length rule -- every other line, in every commit, +is checked. There is no leniency for "just the summary" or "just this once." + +## The rules that actually fire in practice + +| Rule | Limit | Notes | +|---|---|---| +| Title length (T1) | 72 characters | Counts the whole subject line, including the `type:` prefix. | +| Body line length (B1) | 80 characters | Per line, not per paragraph. A heredoc does NOT auto-wrap -- you must break lines yourself. | +| Blank line after title (B4) | required | One empty line between the subject and the body. | +| Title trailing punctuation (T3) | none | No period, no colon, at the end of the subject line. | +| Trailing whitespace (T2, B2) | none | Watch for trailing spaces left by hand-wrapped lines. | +| Hard tabs (B3) | none | Use spaces in the body. | + +## Writing a compliant message the first time + +- Count the title before committing to it. "docs: remove doc-pointers and + provenance narration from PR #964" is 63 characters; adding a scope like + "and cleanup notes" on top of an already-full title is how T1 fails. +- Wrap body prose by hand at well under 80 characters per line -- a heredoc + passed to `git commit -F -`/`-m` reproduces exactly the line breaks you + typed, it does not reflow them. Aim for ~70 so a `Co-Authored-By:` trailer + or an indented list item added later doesn't push a line over. +- Prefer several short lines over one long one, and several short + paragraphs over one dense one -- a commit message is read in a `git log` + pane, not a text editor with wrapping. + +## Verify before considering a commit done + +Run the same check CI runs, scoped to the current branch: + +```powershell +gitlint --ignore body-is-missing --commits main..HEAD +``` + +(Substitute the actual base branch if not `main`.) A clean run prints +nothing and exits 0. `Build/Agent/commit-messages.ps1` wraps this with the +same base-ref auto-detection CI uses, if you want the base resolved for +you instead of naming it. + +## Fixing a violation after the fact + +If a commit already landed non-compliant and hasn't been pushed to a shared +branch, reword it without a full interactive rebase: + +```powershell +git rebase ^ --exec 'if [ "$(git rev-parse HEAD)" = "" ]; then git commit --amend -F ; fi' +``` + +Git's `--exec` always runs the quoted command through its own bundled `sh`, +even on Windows, so that inner string stays POSIX syntax regardless of +which shell you type this from -- only the outer PowerShell single-quotes +(passed through verbatim, unlike bash's backslash-escaped double-quotes) +change here. + +This replays history non-interactively (no editor, no `-i` prompt) and +amends only the one commit whose SHA matches, at the point in the replay +where it is HEAD. Never do this on a branch that has already been pushed +and could have a PR or other work based on it -- check +`git rev-parse --abbrev-ref --symbolic-full-name @{u}` and +`git ls-remote --heads origin ` first, and confirm with the user if +either shows the branch is shared. diff --git a/.claude/skills/fieldworks-code-commenting/SKILL.md b/.claude/skills/fieldworks-code-commenting/SKILL.md index a8bf4c7668..4e2c0bfd21 100644 --- a/.claude/skills/fieldworks-code-commenting/SKILL.md +++ b/.claude/skills/fieldworks-code-commenting/SKILL.md @@ -1,185 +1,165 @@ --- name: fieldworks-code-commenting -description: The FieldWorks code-comment standard for C#. Use whenever writing or editing code comments in this repository -- new code, refactors, or comment audits. Covers doc-comment contracts, banned content categories, legacy references, XML doc tags, and XML-doc placement. +description: The FieldWorks code-comment standard for C#, C/C++, IDL, PowerShell, and project-file/Avalonia-view XML comments. Use whenever writing or editing a comment anywhere in this repository -- new code, refactors, or comment audits, in .cs, .cpp/.h, .idl, .ps1, .csproj/.vcxproj/.props/.targets, or .axaml alike. Covers doc-comment contracts, banned content categories, the 200-character cap on implementation comments, legacy references, XML doc tags, and XML-doc placement. --- # FieldWorks Code Commenting The standard for every comment an agent writes or audits in this repository. -Apply it while authoring -- do not write loose comments and clean them later. - -The audience is the next reader of the code -- never the reviewer of the -current diff, never a coverage gate. +Apply it while authoring, not as cleanup afterward. The audience is the next +reader of the code -- never the current reviewer, never a coverage gate. + +## Scope + +Every comment, any language: `//`/`///` in C#, C/C++, and IDL; `#`/`<# #>` +in PowerShell; `` in project files (`.csproj`/`.vcxproj`/`.props`/ +`.targets`/`.proj`) and Avalonia views (`.axaml`). `Build/Agent/comment- +hygiene.ps1` mechanically enforces banned content, ASCII-punctuation-only, +and the 200-character implementation-comment budget against all of the +above (see `Get-CommentHygieneLanguage` for the exact extension list). +A C-style `/* */` block comment is not scanned -- only whole-line +`//`/`///`/`#` comments and `` XML comments are. Judgment-based +rules (accuracy, WHAT-not-HOW, standalone clarity) are not mechanically +checked and still apply while authoring. ## The standard -1. **Accuracy first, then brevity.** A comment that misstates behavior is - worse than none. Target 3-4 sentences (max ~6 lines); the ticket reference - carries the background. -2. **Doc comments state WHAT and WHY, never HOW.** The how-test: would the - sentence survive an equivalent reimplementation? If not it is a how, and a - "so that..." clause does not redeem it -- keep the purpose, drop the - mechanism. -3. **A member's summary states only its OWN contract.** Never narrate what - callers do with it. -4. **Every comment must stand alone.** No reader has this conversation, the - PR, or any design document open. A doc comment must serve someone hovering - the symbol who will never read the body. -5. **Public members require a summary.** Private members get one only when - genuinely non-obvious. Skip trivial properties, thin wrappers, and - self-evident helpers. -6. **Delete restatements.** A comment repeating what the code plainly says is - noise. State what the code does only when that is not obvious from the - content closely following the comment. When a comment does not clearly - earn its place, delete it. -7. **ASCII only.** No em-dashes, arrows, section signs, or smart quotes in - comments or repo docs -- they render poorly in git tooling. Use "--", - "->", plain quotes. -8. **No ambiguous abbreviations.** Write "ViewModel", never "VM" (VM also - means virtual machine). Same for anything a reader could resolve two - ways: spell it out. This applies to comments, documents, commit - messages, and what you say to the developer. +1. **Accuracy first, then brevity.** A wrong comment is worse than none. + Target 3-4 sentences; the ticket reference carries the background. +2. **WHAT and WHY, never HOW.** Test: would the sentence survive an + equivalent reimplementation? If not, it's a how -- keep the purpose, drop + the mechanism, even behind a "so that" clause. +3. **A member's summary states only its own contract**, never what callers + do with it. +4. **Every comment stands alone.** No reader has this conversation, the PR, + or a design document open. +5. **Public members need a summary; private members only when genuinely + non-obvious.** Skip trivial properties, thin wrappers, self-evident + helpers. +6. **Delete restatements.** If the code already says it plainly, the + comment is noise. +7. **ASCII punctuation only.** No em-dashes, arrows, section signs, smart + quotes -- use "--", "->", plain quotes. **Exception: inside an XML + `` comment, use a single "-", never "--"** -- the XML spec + forbids a literal `--` anywhere in comment content (not just adjacent to + `-->`), so the usual em-dash replacement produces invalid XML there. +8. **No ambiguous abbreviations.** Write "ViewModel", not "VM". Spell out + anything a reader could resolve two ways -- in comments, docs, commits, + and conversation. ## Banned content (mechanical -- no judgment needed) -1. **Migration/process framing**: no "Phase-1", "Stage 3", "this commit", - "this pass", review/creation-process language, or forward-work notes - ("later we'll...", "Stage N wires..."). A comment describes the code that - is HERE. -2. **Internal document pointers**: no references to design/skill/working - `.md` files or their section markers (no "winforms-free-lexeme-editor.md", - "section 19b", "D1/M4/H1" finding codes, task numbers). If the comment - carries a genuine WHY, rewrite it self-contained. Jira `LT-#####` - references ARE allowed -- they are the sanctioned durable pointer. -3. **Absence narration**: no comments whose subject is that code is gone, - was removed, "no longer" does something, or "used to" work differently. - State positively what IS there; the absence needs no narration. - (A legitimate null-input or current-behavior contract is not absence - narration -- keep those.) -4. **Pointers to a comment in another file**: never "see X's note", "as - documented on Y". No tooling checks a comment-to-comment link, so the - target can be reworded or deleted with nothing flagging the break, and the - reader must leave their current file to learn whether anything is there. - Cite the code symbol that carries the behavior, or state the point here. - Same-file pointers are fine. -5. **Consumers and provenance**: no "shared by X and Y", "the only caller - is...", "internal so both create paths can use it", "extracted from Z". - Callers change silently and provenance is not a contract. State what the - member guarantees; callers stay anonymous. +1. **Process framing**: no "Phase-1", "this commit", "later we'll...". + Describe the code that is HERE. +2. **Internal doc pointers**: no `.md` file/section references or finding + codes (`D1`, `M4`). Jira `LT-#####` is the sanctioned durable pointer. +3. **Absence narration**: no "no longer", "used to", "was removed". State + what IS there. (A legitimate current-behavior or null-input contract is + not absence narration -- keep those.) +4. **Cross-file comment pointers**: no "see X's note" -- nothing checks + that link, so it silently rots. Cite the symbol, or state the point + here. Same-file pointers are fine. +5. **Consumers/provenance**: no "shared by X and Y", "the only caller", + "extracted from Z". Callers change silently -- state the guarantee, and + leave the callers anonymous. +6. **Xml comments only**: no literal `--` in an `` comment's content + -- invalid XML, not just a style violation. Use a single `-`. ## Legacy references -Naming legacy code is allowed ONLY as a behavioral-parity WHY that justifies -current behavior: "matches the legacy MatchingObjectsBrowser multi-column -list" stays. Temporal migration framing goes: not "the replacement for X", -not "until we build Y", not the history of how the code got here. Pointers to -real legacy source (`DataTree.cs:2455`) are acceptable parity evidence but -prefer symbol names over line numbers -- lines rot. +Only as a behavioral-parity WHY ("matches the legacy X"). No temporal +framing -- not "the replacement for X", not "until we build Y". Prefer +symbol names over line numbers, which rot. ## References to other code -Reference another symbol only when the reader needs it to understand THIS -one -- never for completeness or navigation. In doc comments write -``; in `//` comments, where tooling does not resolve -it, write the bare symbol path. - -**Link the contract, not the collaborator.** Naming the helper a member -delegates to ("resolves via the shared `BuildSandboxMsa`", "forwards to -`PerformAddAllomorph`") documents HOW it works. State what the delegation -guarantees and leave the callee unnamed. Rewrite rather than delete -- the -guarantee is the useful half. +Only when the reader needs it to understand THIS symbol, never for +completeness or navigation. `` in doc comments; a bare +symbol path in `//` comments, where tooling won't resolve it. -Never reference another member's locals, another type's private internals, or -a test -- describe the behavior instead. +**Link the contract, not the collaborator.** State what a delegation +guarantees; leave the callee unnamed -- naming it documents HOW, not WHAT. +Never reference another member's locals, another type's private internals, +or a test. ## XML doc tags -Omit ``/`` that only restate the name and type. Keep ones -carrying what the declaration cannot: units, what null means, ownership, side -effects, constraints. Never `The cache.`. - -**All-or-nothing (overrides the omit rule):** never document only some -parameters. When one deserves a note, either document every parameter -- when -each has something real to say, never padding -- or fold the note into the -summary prose and drop the tags. Folding edits the summary: rewrite it to -carry the note. - -Which way to go: what the method DOES with an argument ("matched against the -candidate morphemes", "used in the thrown message") folds into prose. What -qualifies the VALUE -- units, meaning of absence, a constraint the type cannot -express -- stays a tag. - -**No double documentation.** A fact lives in the summary or in a tag, never -both. - -**No mirrored member docs.** When a parameter's type documents its own members --- a state or options class -- do not restate them as ``. The member -docs are the single source of truth. - -`` for every error condition the caller must handle; omit it when -the member does not throw. +Omit ``/`` that only restate the name and type; keep ones +carrying units, null meaning, ownership, or constraints. -## Types and enums +**All-or-nothing:** document every parameter, or fold the note into summary +prose and drop the tags -- never document only some. What the method DOES +with an argument folds into prose; what qualifies the VALUE (units, +absence, a constraint) stays a tag. Never state a fact in both the summary +and a tag. When a parameter's type already documents its own members +(a state/options class), don't restate them as ``. -Interfaces, classes, structs, records, and enums follow the same rules. -Document each member whose purpose is not self-evident from its name and type, -individually rather than in the type-level summary. +`` for every error condition the caller must handle. -## File and section comments +## Types, files, and sections +Interfaces, classes, structs, records, and enums follow the same rules; +document non-obvious members individually, not in the type-level summary. No decorative file-header banners beyond the license header. No section -dividers that merely restate the adjacent member name -- a divider must carry -information of its own. +dividers that merely restate the adjacent member name. ## Inline comments -Sparingly: only when the reasoning is not clear from the code, or a bugfix is -non-obvious. Keep them short and located above the level of nesting that the code being described spans. +Sparingly: only when the reasoning isn't clear from the code, or a bugfix +is non-obvious. **200 characters total**, across as many lines as the +line-length limit requires. Past that, it belongs as a doc comment (which +may run long-form), or is trying to explain too much -- cut to the single +sentence that would confuse a reader most if missing, even if that loses +nuance. Mechanically enforced (`comment-too-long`) for `//` and `#` alike. +Place above the nesting level the code spans. + +**Line width is separate, and applies to every comment line**, doc comments +included: no comment line may exceed `.editorconfig`'s `max_line_length` +(98 columns today), counting a tab as `tab_width` columns. The gate reads +those two values from `.editorconfig` itself, so the limit can never drift +from the one the rest of the repo follows. Enforced as +`comment-line-too-long`; a local run re-wraps the line for you, CI only +reports it. + +**The budget rises to 600 characters in dense branching code.** A comment +introducing a region whose decision-point count reaches 10 (McCabe +complexity 11 -- the classic "high" threshold) gets the larger budget +automatically, because a reader there needs the invariants spelled out and +200 characters buys about two sentences. Nothing opts in by hand: the gate +measures the code the comment introduces, stopping at the end of the +enclosing block or 40 lines. This fires on roughly 2% of the comments +already over 200 characters, and is meant to stay that rare -- if a comment +in ordinary straight-line code will not fit, shorten it rather than looking +for a way to qualify. + +**Exemptions from the length cap:** a C#/C/C++/IDL `///` doc comment; a +PowerShell comment-based help block (`<# ... #>`); and, in a project file or +Avalonia view, the file's FIRST `` block, wherever it falls (before +the root element, or as its first child) -- XML has no separate doc-comment +syntax, so that first block is this format's equivalent of a `///` summary +and may run long-form. Every `` block after the first one is an +ordinary implementation comment, budgeted like any other. In tests, two extra tells of noise: restating what the test method name -already says, and justifying a test to the coverage gate ("covers the false -branch of..."). Why a dependency is faked, or why fixture data is shaped a -particular way, stays fine -- that is reasoning the code cannot show. +already says, and justifying a test to the coverage gate. Why a dependency +is faked, or fixture data is shaped a particular way, stays fine -- that is +reasoning the code cannot show. ## XML-doc placement gotchas -- One `` per member; two consecutive doc blocks both attach to the - NEXT declaration -- the first lands on the wrong member. Verify each - summary sits directly above its own declaration. +- One `` per member -- two consecutive doc blocks both attach to + the NEXT declaration. Verify each summary sits directly above its own + declaration. - Public constructors with `` docs also get a one-line ``. -- `` resolves only inside `///` doc comments. In a `//` comment it - renders as literal text -- use the bare symbol path there. +- `` resolves only inside `///`; in `//` it renders as literal + text -- use the bare symbol path there. - Escape `<` and `>` in doc text as `<`/`>`. - String literals are not comments: never edit assertion messages, automation ids, or resx values during a comment pass. -- resx accessor files may carry APPEND-ONLY section rules -- respect them. - -## Worked examples (from the live audit) - -Phase framing stripped: -- Before: `// Phase 3 test (b): picking a style applies it to the selection` -- After: `// Picking a style applies it to the selection` - -Doc marker removed, WHY kept self-contained: -- Before: `// winforms-free-lexeme-editor.md D1: a plugin-claimed custom slice renders its plugin's own control` -- After: `// A plugin-claimed custom slice renders its plugin's own control` - -Absence rewritten to current behavior: -- Before: `// An ORC run no longer forces the whole value read-only.` -- After: `// An ORC run does not force the whole value read-only.` - -Temporal legacy framing trimmed to parity: -- Before: `/// the Avalonia replacement for the legacy ReallySimpleListChooser` -- After: `/// the Avalonia analog of the legacy ReallySimpleListChooser` - -Over-long summary trimmed to purpose + non-obvious contracts: -- A 37-line dialog ViewModel summary enumerating implementation steps becomes ~10 - lines: purpose, the LCModel-free rule, and the two non-obvious contracts - (commit-on-select single-stage; opt-in two-stage auxiliary). ## When auditing existing comments Report every deletion and rewrite (before -> after) so a human can object; when unsure whether a comment is a current-behavior contract or absence -narration, KEEP it and flag it. Never let a comment edit change behavior: +narration, keep it and flag it. Never let a comment edit change behavior: comment-only diffs, string literals untouched. diff --git a/.claude/skills/powershell/SKILL.md b/.claude/skills/powershell/SKILL.md index 480e5d83ae..55bbcd8b9f 100644 --- a/.claude/skills/powershell/SKILL.md +++ b/.claude/skills/powershell/SKILL.md @@ -3,7 +3,7 @@ name: powershell description: > PowerShell best practices for scripts used in FieldWorks (dev scripts & CI helpers). Use when writing or modifying PowerShell scripts in scripts/ or Build/Agent/. -allowed-tools: "Read,Bash(pwsh:*)" +allowed-tools: "Read,PowerShell" version: "1.0.0" --- @@ -13,11 +13,81 @@ Conventions and safety patterns for PowerShell scripts in `scripts/` and CI. ## Style and Linting -- Use `pwsh`/PowerShell Core syntax where possible and `Set-StrictMode -Version Latest`. +- Scripts in `Build/Agent/`, and anything else reached from `build.ps1` or `test.ps1`, + must run under **both** Windows PowerShell 5.1 and PowerShell 7. CI executes the + build and test steps under 5.1, so 6+-only syntax that parses cleanly on 7 can + still fail or silently misbehave there: the backtick u{} escape resolves to + literal text under 5.1 instead of a code point, and `-Encoding utf8BOM`/`utf8NoBOM` + throw a parameter-binding error. Run `Build/Agent/powershell-compat.ps1` to check, + and prefer syntax both engines share over anything PowerShell Core adds. +- Use `Set-StrictMode -Version Latest`. - Use `Write-Host` sparingly; prefer `Write-Output` and `Write-Error` for correct streams. - Use `-ErrorAction Stop` in helper functions when errors should abort execution. - **No Unicode icons or emojis** in output messages (e.g., `βœ“`, `βœ—`, `⚠`, `πŸ”§`). Use plain ASCII text like `[OK]`, `[FAIL]`, `[WARN]`, `ERROR:` instead. Unicode causes encoding issues in CI logs. +## Traps that produce a wrong answer instead of an error + +The first one below is the dangerous one: it yields a plausible result with no +warning, so nothing prompts you to look. The others fail loudly, but only under +`Set-StrictMode -Version Latest`, which this repo requires. + +### An operator after a bare function call binds as an argument + +`-replace`, `-split`, `-match`, and friends written after an unparenthesized +function call are parsed as further *arguments* to that call, not applied to its +result. The operator is silently ignored. + +```powershell +# BAD: -replace and '\s+' become arguments 2 and 3 of Norm; nothing is replaced +$key = Norm ($text) -replace '\s+', '' + +# GOOD: parenthesize the call, then apply the operator to its result +$key = (Norm $text) -replace '\s+', '' +``` + +### Measure-Object -Sum over an empty collection returns $null + +Reading `.Sum` (or `.Maximum`, `.Average`) off that result then throws +"The property 'Sum' cannot be found on this object" -- which surfaces far from +the empty input that caused it. + +```powershell +# BAD: throws whenever $items happens to be empty +$total = ($items | Measure-Object -Property Length -Sum).Sum + +# GOOD +$total = 0 +foreach ($item in $items) { $total += $item.Length } +``` + +### Returning a collection from a function unrolls it + +`return $list` enumerates into the pipeline: an empty collection becomes `$null` +and a single element becomes a scalar, so the caller's `.Count` throws. Prefix +with a comma to return the collection itself. + +```powershell +# BAD: (Get-Ids).Count throws when the list is empty +function Get-Ids { $ids = New-Object System.Collections.Generic.List[int]; return $ids } + +# GOOD +function Get-Ids { $ids = New-Object System.Collections.Generic.List[int]; return ,$ids } +``` + +### The stop-parsing token consumes the rest of the line + +`--%` passes everything after it to the native command verbatim, including any +closing bracket you meant PowerShell to read. It cannot appear inside `@(...)`, +`$(...)`, or any other expression that has to be closed. + +```powershell +# BAD: --% swallows the closing paren; parse error, not a runtime error +$msg = @(git --% log -1 --format=%B) + +# GOOD: keep --% on a statement of its own, or drop it when it is not needed +$msg = @(git log -1 --pretty=%B) +``` + ## Security - Avoid embedding secrets in scripts; read from env vars and prefer platform secret stores. diff --git a/.claude/skills/pr-pitch/SKILL.md b/.claude/skills/pr-pitch/SKILL.md index 599fa2a847..961e2e66f2 100644 --- a/.claude/skills/pr-pitch/SKILL.md +++ b/.claude/skills/pr-pitch/SKILL.md @@ -1,6 +1,6 @@ --- name: pr-pitch -description: Compose a PR body as a pitch that answers the unknowns a reviewer arrives with, with the branch's decisions, provenance, and paths-not-taken folded into collapsed accordions below it, while evicting those files from the repo. Use when writing or refreshing a PR description, when a branch carries working markdown that should not merge, or when pr-preflight reaches its PR step. +description: "NOT an entrypoint -- pr-preflight calls this for the write-up step; use pr-preflight for a fresh 'write/make/open a PR' request. Invoke this directly only to redo the write-up on a PR that already exists. Composes a PR body as a pitch that answers the unknowns a reviewer arrives with, with the branch's decisions, provenance, and paths-not-taken folded into collapsed accordions below it, while evicting those files from the repo." argument-hint: "Optional PR number (defaults to the PR for the current branch)" --- diff --git a/.claude/skills/pr-preflight/SKILL.md b/.claude/skills/pr-preflight/SKILL.md index bff19861b0..f032ea6f1d 100644 --- a/.claude/skills/pr-preflight/SKILL.md +++ b/.claude/skills/pr-preflight/SKILL.md @@ -1,6 +1,6 @@ --- name: pr-preflight -description: "Use when preparing a FieldWorks branch or pull request for review: pre-PR review, branch readiness, author interview, review summary generation, validation evidence, or PR description preparation." +description: "The required entrypoint whenever asked to write, make, open, create, update, or ship a PR for this repo -- do not post a PR body without running this first. Also use for pre-PR review, branch readiness, author interview, review summary generation, or validation evidence." argument-hint: "Optional branch purpose or PR goal" user-invocable: true --- @@ -236,7 +236,7 @@ After writing the summary, tell the author: > > Please review it, make changes where appropriate, and run `/pr-preflight` again until you are ready to post the PR. > -> If you do not want to make any changes and are ready for review, would you like me to commit any uncommitted changes, push, and post the PR? I will check whether one already exists for this branch and update it, or create a new one if not. The write-up runs through `pr-pitch`, which will also triage any research or working markdown on the branch into collapsed PR comments and out of the tree -- you approve that triage before anything is deleted." +> If you do not want to make any changes and are ready for review, would you like me to commit any uncommitted changes, push, and post the PR? I will check whether one already exists for this branch and update it, or create a new one if not. The write-up runs through `pr-pitch`, which will also triage any research or working markdown on the branch into collapsed sections in the PR body and out of the tree -- you approve that triage before anything is deleted." Only create or update a PR after the author confirms. @@ -244,9 +244,10 @@ Only create or update a PR after the author confirms. This skill is the single entrypoint for making a PR, but it does not compose the description itself. Once the author confirms readiness, invoke the -`pr-pitch` skill and let it own the write-up. It produces three artifacts -together: the PR body as a pitch, provenance in collapsed PR comments, and a -commit evicting the branch's research and working markdown from the tree. +`pr-pitch` skill and let it own the write-up. It produces two artifacts +together: the PR body (a pitch above the fold, provenance in collapsed +accordions below it) and a commit evicting the branch's research and working +markdown from the tree. Hand `pr-pitch` the branch purpose, the findings, and `.review/summary.md`. diff --git a/.github/instructions/dotnet-upgrade.instructions.md b/.github/instructions/dotnet-upgrade.instructions.md index e384dc3742..3663b47691 100644 --- a/.github/instructions/dotnet-upgrade.instructions.md +++ b/.github/instructions/dotnet-upgrade.instructions.md @@ -48,11 +48,11 @@ To identify dependencies: - Use the following approaches: - **Visual Studio** β†’ `Dependencies` in Solution Explorer. - **dotnet CLI** β†’ run: - ```bash + ```powershell dotnet list .csproj reference ``` - **Dependency Graph Generator**: - ```bash + ```powershell dotnet msbuild .sln /t:GenerateRestoreGraphFile /p:RestoreGraphOutputPath=graph.json ``` Inspect `graph.json` to see the dependency order. @@ -79,16 +79,16 @@ For each project: - `TargetFramework` β†’ Change to the desired version (e.g., `net8.0`). - `PackageReference` β†’ Verify if each NuGet package supports the new framework. - Run: - ```bash + ```powershell dotnet list package --outdated ``` Update packages: - ```bash + ```powershell dotnet add package --version ``` 3. If `packages.config` is used (legacy), migrate to `PackageReference`: - ```bash + ```powershell dotnet migrate ``` @@ -131,11 +131,11 @@ BlobServiceClient client = new BlobServiceClient(connectionString); 2. Update NuGet packages to versions compatible with the target framework. 3. After upgrading and restoring the latest DLLs, review code for any required changes. 4. Rebuild the project: - ```bash + ```powershell dotnet build .csproj ``` 5. Run unit tests if any: - ```bash + ```powershell dotnet test ``` 6. Fix build or runtime issues before proceeding. @@ -168,7 +168,7 @@ After all projects are upgraded: ## 7. Tools & Automation - **.NET Upgrade Assistant**(Optional): - ```bash + ```powershell dotnet tool install -g upgrade-assistant upgrade-assistant upgrade .sln``` diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index d02adaae43..f0836884d8 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -10,7 +10,7 @@ git log --check --pretty=format:"---% h% s" origin/.. git diff --check --cached ``` -- [ ] Builds/tests pass locally (or I've run the CI-style build via Bash script or MSBuild). +- [ ] Builds/tests pass locally (or I've run the CI-style build via `build.ps1`/`test.ps1` or MSBuild). - [ ] If this is core-developer AI-assisted work, I followed `Docs/workflows/ai-pr-workflow.md` and ran `pr-preflight` or the equivalent branch-readiness review before requesting review. - [ ] For any `Src/**` folders touched, corresponding `AGENTS.md` files are updated or explicitly confirmed still accurate. diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 730eb64502..c1b2d5c4e5 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -23,6 +23,32 @@ jobs: - name: Checkout Files uses: actions/checkout@v7 id: checkout + with: + # comment-hygiene.ps1 diffs against origin/; a + # shallow, single-branch checkout leaves that ref unresolvable + # and the gate fails every build with "bad revision". + fetch-depth: 0 + + # Real-runtime check: this repo is authored on PowerShell 7 but + # build.ps1/test.ps1 run under Windows PowerShell 5.1 below. A script + # can parse identically on both and still resolve differently at + # runtime (see comment-hygiene's ASCII-replacement map, which used to + # do exactly that). windows-2022 ships both engines, so run the + # comment-hygiene fixture suite under each rather than assuming one + # implies the other. + - name: Comment hygiene fixture tests (PowerShell 7) + id: comment-hygiene-tests-pwsh + shell: pwsh + run: | + Build/Agent/CommentHygiene.Tests.ps1 + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + + - name: Comment hygiene fixture tests (Windows PowerShell 5.1) + id: comment-hygiene-tests-winps + shell: powershell + run: | + Build\Agent\CommentHygiene.Tests.ps1 + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - name: Build with tests id: build diff --git a/.github/workflows/CommitMessage.yml b/.github/workflows/CommitMessage.yml index c82fc7aa96..8e8deb891f 100644 --- a/.github/workflows/CommitMessage.yml +++ b/.github/workflows/CommitMessage.yml @@ -13,26 +13,33 @@ jobs: with: fetch-depth: 0 - name: Install dependencies - run: | - pip install --upgrade gitlint + shell: pwsh + run: pip install --upgrade gitlint - name: Lint git commit messages - shell: bash + shell: pwsh # run the linter and tee the output to a file, this will make the check fail but allow us to use the results in summary - run: gitlint --ignore body-is-missing --commits origin/$GITHUB_BASE_REF.. 2>&1 | tee check_results.log + run: | + # Pre-create the file: Tee-Object never creates (or even truncates) its target when the + # piped command emits zero objects, which is exactly what a clean gitlint run does. + New-Item -ItemType File -Path check_results.log -Force | Out-Null + gitlint --ignore body-is-missing --commits "origin/$env:GITHUB_BASE_REF.." 2>&1 | Tee-Object -FilePath check_results.log + exit $LASTEXITCODE - name: Propegate Error Summary if: always() - shell: bash + shell: pwsh # put the output of the commit message linting into the summary for the job and in an environment variable run: | # Change the commit part of the log into a markdown link to the commit - commitsUrl="https:\/\/github.com\/${{ github.repository_owner }}\/${{ github.event.repository.name }}\/commit\/" - sed -i "s/Commit \([0-9a-f]\{7,40\}\)/[commit \1]($commitsUrl\1)/g" check_results.log + $commitsUrl = "https://github.com/${{ github.repository_owner }}/${{ github.event.repository.name }}/commit/" + $replacement = '[commit $1](' + $commitsUrl + '$1)' + $log = (Get-Content check_results.log -Raw) -replace 'Commit ([0-9a-f]{7,40})', $replacement + Set-Content -Path check_results.log -Value $log -NoNewline # Put the results into the job summary - cat check_results.log >> "$GITHUB_STEP_SUMMARY" + Add-Content -Path $env:GITHUB_STEP_SUMMARY -Value $log # Put the results into a multi-line environment variable to use in the next step - echo "check_results<<###LINT_DELIMITER###" >> "$GITHUB_ENV" - echo "$(cat check_results.log)" >> "$GITHUB_ENV" - echo "###LINT_DELIMITER###" >> "$GITHUB_ENV" + Add-Content -Path $env:GITHUB_ENV -Value 'check_results<<###LINT_DELIMITER###' + Add-Content -Path $env:GITHUB_ENV -Value $log + Add-Content -Path $env:GITHUB_ENV -Value '###LINT_DELIMITER###' # add a comment on the PR if the commit message linting failed - name: Comment on PR if: failure() diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml index d420a76db6..dc0e169b66 100644 --- a/.github/workflows/check-whitespace.yml +++ b/.github/workflows/check-whitespace.yml @@ -18,5 +18,5 @@ jobs: fetch-depth: 0 - name: Run whitespace check script - shell: bash - run: bash ./Build/Agent/check-whitespace.sh + shell: pwsh + run: ./Build/Agent/check-whitespace.ps1 diff --git a/.github/workflows/stray-docs.yml b/.github/workflows/stray-docs.yml new file mode 100644 index 0000000000..f7fdce040c --- /dev/null +++ b/.github/workflows/stray-docs.yml @@ -0,0 +1,32 @@ +name: stray-docs + +on: + pull_request: + types: [opened, synchronize] + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + stray-docs: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 0 + + - name: Check for stray planning/spec docs + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + $baseSha = "${{ github.event.pull_request.base.sha }}" + $added = git diff --name-only --diff-filter=A "$baseSha...HEAD" -- 'Docs/superpowers/plans/*.md' 'Docs/superpowers/specs/*.md' + if ($added) { + Write-Host "Stray working docs found in this PR's diff:" + $added | ForEach-Object { Write-Host $_ } + Write-Host '' + Write-Host "These are brainstorming/planning artifacts that should be evicted before merge (see the pr-pitch skill)." + exit 1 + } + Write-Host 'No stray working docs found.' diff --git a/.gitignore b/.gitignore index 2476e52091..5f8caa9db1 100644 --- a/.gitignore +++ b/.gitignore @@ -217,3 +217,5 @@ DataTreeTimingBaselines.json *[Ss]cratchPad* Docs/migration/working/ +Build/Agent/comment-hygiene-report.json +.review/ diff --git a/.serena/memories/project_overview.md b/.serena/memories/project_overview.md index cc2bddcd59..3c2a00bacf 100644 --- a/.serena/memories/project_overview.md +++ b/.serena/memories/project_overview.md @@ -1,7 +1,7 @@ # FieldWorks Project Overview - Purpose: FieldWorks (aka FLEx) is SIL International's Windows-focused linguistics and language data management suite. The repository hosts multiple desktop applications, shared libraries, an installer, tooling, and rich documentation. -- Tech stack: predominantly C#/.NET Framework 4.8 managed code, plus native C++/C++-CLI components, WiX installer assets, PowerShell/bash build scripts, and auxiliary Python tooling. Builds rely on MSBuild traversal (`FieldWorks.proj`). +- Tech stack: predominantly C#/.NET Framework 4.8 managed code, plus native C++/C++-CLI components, WiX installer assets, PowerShell build scripts, and auxiliary Python tooling. Builds rely on MSBuild traversal (`FieldWorks.proj`). - Structure highlights: Src/ contains applications and libraries (with per-folder AGENTS.md docs). Build/ houses shared targets/scripts, FLExInstaller/ contains WiX artifacts, Include/ + Lib/ host native headers/libs, and Build/Agent scripts support worktree automation. Specs/ and Docs/ provide planning/reference material. - Key guidelines: Follow `.github/instructions/*.instructions.md` (build, managed, native, installer, testing). Respect `.editorconfig`, update COPILOT metadata when touching folders, and keep documentation in sync with code. - Tooling environment: development happens on Windows with Visual Studio 2026 or 2022 workloads (Desktop .NET + C++; newest installed wins, per Build/FieldWorks.Toolchain.props), WiX 3.14.x. diff --git a/AGENTS.md b/AGENTS.md index 1b5afa39c5..eefd98985f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,6 +8,10 @@ Minimal, high-signal guidance for coding agents in this repository. - Build with `.\build.ps1`. - Test with `.\test.ps1`. - Do not bypass repository scripts for normal build/test work. +- Commit messages must pass `gitlint` (CI: `.github/workflows/CommitMessage.yml`): + title <=72 characters, body lines <=80 characters, blank line between + them. A heredoc reproduces your line breaks exactly -- wrap body prose + by hand. See `.claude/skills/commit-messages/SKILL.md`. ## Critical constraints @@ -16,6 +20,8 @@ Minimal, high-signal guidance for coding agents in this repository. - Keep localization in `.resx`; do not hardcode translatable UI strings. - Follow the code-comment standard in `.claude/skills/fieldworks-code-commenting/SKILL.md`. +- Follow the commit-message rules in + `.claude/skills/commit-messages/SKILL.md` for every commit. ## Context model diff --git a/Build/Agent/CommentHygiene.Tests.ps1 b/Build/Agent/CommentHygiene.Tests.ps1 new file mode 100644 index 0000000000..119e463a57 --- /dev/null +++ b/Build/Agent/CommentHygiene.Tests.ps1 @@ -0,0 +1,360 @@ +<# +.SYNOPSIS + Fixture-based tests for CommentHygiene.psm1. + +.DESCRIPTION + One true-positive and one near-miss per category. Run directly: + pwsh -File Build/Agent/CommentHygiene.Tests.ps1 +#> + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +Import-Module (Join-Path $PSScriptRoot 'CommentHygiene.psm1') -Force + +function Get-Codepoint { + <# + .SYNOPSIS + Builds a real Unicode character from a code point, portable across + PowerShell 7 and Windows PowerShell 5.1. + + .DESCRIPTION + Fixture strings must not use the backtick-u{} escape: it is + PowerShell 6+ only, and under 5.1 it silently degrades to the + literal text "u{2014}" instead of throwing -- exactly the bug this + suite exists to catch in the module, so the fixtures cannot carry + it themselves. + #> + param([int] $Codepoint) + if ($Codepoint -gt 0xFFFF) { return [char]::ConvertFromUtf32($Codepoint) } + return [string][char]$Codepoint +} + +$tempDir = Join-Path ([System.IO.Path]::GetTempPath()) ("CommentHygieneTests_" + [System.Guid]::NewGuid().ToString('N')) +New-Item -ItemType Directory -Path $tempDir | Out-Null + +$failures = New-Object System.Collections.ArrayList + +function Assert-Category { + param([string] $Name, [string] $Line, [string] $ExpectedCategory, [string] $Extension = '.cs') + + $file = Join-Path $tempDir "$Name$Extension" + Set-Content -LiteralPath $file -Value $Line -Encoding UTF8 + $violations = Get-CommentHygieneViolations -Files @($file) + $hit = $violations | Where-Object { $_.Category -eq $ExpectedCategory } + if (-not $hit) { + [void]$script:failures.Add("FAIL [$Name]: expected category '$ExpectedCategory' for line: $Line") + } +} + +function Assert-Clean { + param([string] $Name, [string] $Line, [string] $Extension = '.cs') + + $file = Join-Path $tempDir "$Name$Extension" + Set-Content -LiteralPath $file -Value $Line -Encoding UTF8 + $violations = Get-CommentHygieneViolations -Files @($file) + if ($violations.Count -gt 0) { + $hitCategories = ($violations | ForEach-Object { $_.Category }) -join ',' + [void]$script:failures.Add("FAIL [$Name]: expected no violations for line: $Line -- got $hitCategories") + } +} + +function Assert-CategoryLines { + param([string] $Name, [string[]] $Lines, [string] $ExpectedCategory, [string] $Extension = '.cs') + + $file = Join-Path $tempDir "$Name$Extension" + Set-Content -LiteralPath $file -Value $Lines -Encoding UTF8 + $violations = Get-CommentHygieneViolations -Files @($file) + $hit = $violations | Where-Object { $_.Category -eq $ExpectedCategory } + if (-not $hit) { + [void]$script:failures.Add("FAIL [$Name]: expected category '$ExpectedCategory' for lines: $($Lines -join ' / ')") + } +} + +function Assert-CleanLines { + param([string] $Name, [string[]] $Lines, [string] $Extension = '.cs') + + $file = Join-Path $tempDir "$Name$Extension" + Set-Content -LiteralPath $file -Value $Lines -Encoding UTF8 + $violations = Get-CommentHygieneViolations -Files @($file) + if ($violations.Count -gt 0) { + $hitCategories = ($violations | ForEach-Object { $_.Category }) -join ',' + [void]$script:failures.Add("FAIL [$Name]: expected no violations for lines: $($Lines -join ' / ') -- got $hitCategories") + } +} + +function Assert-Repair { + param([string] $Name, [string] $Line, [string] $ExpectedFixedLine) + + $actual = Repair-CommentLine -Line $Line + if ($actual -ne $ExpectedFixedLine) { + [void]$script:failures.Add("FAIL [$Name]: expected repaired line '$ExpectedFixedLine', got '$actual' for input: $Line") + } +} + +function Assert-Unrepairable { + param([string] $Name, [string] $Line) + + $actual = Repair-CommentLine -Line $Line + if ($null -ne $actual) { + [void]$script:failures.Add("FAIL [$Name]: expected `$null (unrepairable) for line: $Line -- got '$actual'") + } +} + +# process-framing -- skill worked example +Assert-Category 'phase-framing' '// Phase 3 test (b): picking a style applies it to the selection' 'process-framing' +Assert-Clean 'phase-clean' '// Applies the selected style to the current selection' + +# process-framing must not fire on "stage"/"commit" here: this codebase has a real two-step +# stage-then-commit UI pattern, distinct from migration-phase framing. +Assert-Clean 'stage-domain-term' '// STAGE 1 -- the pick already populated the auxiliary picker.' +Assert-Clean 'commit-domain-term' '// Capture everything staged since the last boundary -- that is what this commit "writes".' + +# doc-pointer -- skill worked example +Assert-Category 'doc-pointer-md' '// winforms-free-lexeme-editor.md D1: a plugin-claimed custom slice renders its plugin''s own control' 'doc-pointer' +Assert-Clean 'doc-pointer-clean' '// LT-22351: a plugin-claimed custom slice renders its plugin''s own control' + +# doc-pointer finding-code must stay case-sensitive: "m3" is a real LCM field name (IMoStemMsa), +# not a design-doc finding code, despite the (?i) flag earlier in the pattern. +Assert-Clean 'doc-pointer-lowercase-field' '// Seed text matches the canonical field label (the m3 InflectionClass field label).' + +# doc-pointer finding-code must not fire on function keys or generic type-parameter names, which +# share the letter-plus-digit shape but are ordinary code vocabulary, not design-doc codes. +Assert-Clean 'doc-pointer-function-key' '// Whether it came from a legacy view, F5/RefreshAllViews-driven, or something else.' +Assert-Clean 'doc-pointer-generic-param' '// Factored to a Func seam so the caller can inject either path.' + +# absence-narration -- skill worked example +Assert-Category 'absence-no-longer' '// An ORC run no longer forces the whole value read-only.' 'absence-narration' +Assert-Clean 'absence-clean' '// An ORC run does not force the whole value read-only.' + +# cross-file-pointer +Assert-Category 'cross-file' "// See BulkEditBar's note about ownership checks." 'cross-file-pointer' +Assert-Clean 'cross-file-clean' '// Ownership checks run before every write in this method.' + +# provenance +Assert-Category 'provenance' '// This helper is shared by BulkEditBar and RecordClerk.' 'provenance' +Assert-Clean 'provenance-clean' '// Applies the pending edit to every selected row.' + +# non-ascii-punctuation +Assert-Category 'non-ascii-punctuation' ("// Uses an em dash {0} inline." -f (Get-Codepoint 0x2014)) 'non-ascii-punctuation' +Assert-Clean 'non-ascii-punctuation-clean' '// Uses a double hyphen -- inline.' + +# non-ascii-punctuation targets only Western-typography punctuation, not non-ASCII in general -- +# real non-English script or IPA/emoji content must not fire. +Assert-Clean 'non-ascii-punctuation-cyrillic' ("// Folds the Cyrillic letter {0} into the wrong letter group." -f (Get-Codepoint 0x0493)) +Assert-Clean 'non-ascii-punctuation-emoji' ('// The input string is "x{0}y" (a surrogate pair).' -f (Get-Codepoint 0x1F600)) + +# Repair-CommentLine -- mapped characters produce a fixed ASCII line +Assert-Repair 'repair-em-dash' ("// Uses an em dash {0} inline." -f (Get-Codepoint 0x2014)) '// Uses an em dash -- inline.' +Assert-Repair 'repair-arrow' ("// Flows left {0} right." -f (Get-Codepoint 0x2192)) '// Flows left -> right.' +Assert-Repair 'repair-ellipsis' ("// And so on {0}" -f (Get-Codepoint 0x2026)) '// And so on ...' +Assert-Repair 'repair-bullet' ("// {0} first item" -f (Get-Codepoint 0x2022)) '// - first item' +Assert-Repair 'repair-multiply' ("// A {0} B grid" -f (Get-Codepoint 0x00d7)) '// A x B grid' +Assert-Repair 'repair-doc-slash' ("/// Uses an em dash {0} inline." -f (Get-Codepoint 0x2014)) '/// Uses an em dash -- inline.' + +# Detection and repair share one character set by construction (the pattern is built from +# the replacement map's keys), so there is no "detected but unmapped" character to fail on. +$cjkLine = "// Some text with {0} inline." -f (Get-Codepoint 0x4e2d) +Assert-Repair 'repair-noop-cjk' $cjkLine $cjkLine + +# Repair-CommentLine -- not a whole-line comment at all +Assert-Unrepairable 'repair-not-a-comment' ("int x = 1; // trailing {0} comment" -f (Get-Codepoint 0x2014)) + +# PowerShell (#) gets the same categories as C# (//) -- the gap that let this tooling's own +# comments ship unscanned. +Assert-Category 'ps-phase-framing' '# Phase 3 test: picking a style applies it to the selection' 'process-framing' '.ps1' +Assert-Category 'ps-non-ascii-punctuation' ("# Uses an em dash {0} inline." -f (Get-Codepoint 0x2014)) 'non-ascii-punctuation' '.ps1' +Assert-Repair 'repair-ps-em-dash' ("# Uses an em dash {0} inline." -f (Get-Codepoint 0x2014)) '# Uses an em dash -- inline.' + +# Guards against a regression back to the PS7-only backtick-u{} escape. +$emDash = Get-Codepoint 0x2014 +if (-not (Get-NonAsciiReplacementMap).Contains($emDash)) { + [void]$script:failures.Add("FAIL [non-ascii-map-real-codepoint]: Get-NonAsciiReplacementMap does not key on the real em dash character (PSVersion $($PSVersionTable.PSVersion))") +} + +# comment-too-long fires on a 200-char budget across the whole block, not a line count -- a +# doc comment or PowerShell help block is exempt and may run long-form regardless of length. +Assert-CategoryLines 'too-long-cs' @( + '// This explains the first reason the approach was chosen over the alternative approach taken here for this specific case.', + '// This explains a second reason that would not fit on the first line at all today either, adding more detail.' +) 'comment-too-long' +Assert-CategoryLines 'too-long-single-line-cs' @( + '// This one very long line explains the reasoning all by itself without wrapping and keeps going for quite a while past what used to be the one-line cap until it is clearly over the two hundred character budget on its own.' +) 'comment-too-long' +Assert-CleanLines 'one-line-cs' @('// A single reason, on a single line, is exactly the budget.') +Assert-CleanLines 'under-budget-multiline-cs' @( + '// A short first reason for the approach, stated plainly.', + '// A short second reason that rounds out the explanation.' +) +Assert-CleanLines 'doc-comment-long-cs' @( + '/// A public API doc comment may run several lines when the contract genuinely needs it,', + '/// because a reader hovering the symbol has nowhere else to find this.' +) +Assert-CategoryLines 'too-long-ps' @( + '# This explains the first reason the approach was chosen over the alternative approach taken here for this specific case.', + '# This explains a second reason that would not fit on the first line at all today either, adding more detail.' +) 'comment-too-long' '.ps1' +Assert-CleanLines 'help-block-long-ps' @( + '<#', + '.SYNOPSIS', + ' Comment-based help is allowed to run long-form, the PowerShell equivalent of a doc comment.', + '#>' +) '.ps1' + +# CLike (C/C++/IDL) shares C#'s // and /// syntax and categories -- one worked example per +# extension. +Assert-Category 'cpp-phase-framing' '// Phase 3 test: picking a style applies it to the selection' 'process-framing' '.cpp' +Assert-Category 'h-non-ascii-punctuation' ("// Uses an em dash {0} inline." -f (Get-Codepoint 0x2014)) 'non-ascii-punctuation' '.h' +Assert-Category 'idl-absence-narration' '// An ORC run no longer forces the whole value read-only.' 'absence-narration' '.idl' +Assert-Clean 'cpp-clean' '// Applies the selected style to the current selection' '.cpp' + +# Xml uses ; banned categories fire regardless of exempt/impl kind, even on the first +# comment. +Assert-Category 'csproj-single-line' '' 'absence-narration' '.csproj' +Assert-Clean 'csproj-single-line-clean' '' '.csproj' +Assert-CategoryLines 'axaml-multi-line' @( + '' +) 'absence-narration' '.axaml' + +# Xml's FIRST block is exempt from the budget; later blocks are not. Over 200 +# chars total, but each physical line fits the width limit: the rules are +# independent. +Assert-CleanLines 'xml-first-comment-exempt-long' @( + '', + '', + '' +) '.csproj' +Assert-CategoryLines 'xml-second-comment-too-long' @( + '', + '', + '', + '', + '' +) 'comment-too-long' '.vcxproj' +Assert-CleanLines 'targets-clean-multiline' @( + '' +) '.targets' + +# Xml comment content may never contain a literal "--" (XML spec, not just adjacent to "-->"); +# a single "-" is the safe ASCII substitute there instead of the usual "--". +Assert-Category 'xml-double-hyphen' '' 'xml-illegal-double-hyphen' '.csproj' +Assert-Clean 'xml-single-hyphen-clean' '' '.csproj' + + +# ---- per-line width, taken from .editorconfig's max_line_length ---- + +$cfg = Get-CommentHygieneEditorConfig -RepoRoot (Resolve-Path (Join-Path $PSScriptRoot '../..')).Path +if ($cfg.MaxLineLength -ne 98 -or $cfg.TabWidth -ne 4) { + [void]$failures.Add("FAIL [editorconfig-read]: expected 98/4, got $($cfg.MaxLineLength)/$($cfg.TabWidth)") +} + +$widthPad = '/' * 100 +Assert-Category 'comment-line-too-long' "// $widthPad" 'comment-line-too-long' +Assert-Clean 'comment-line-at-limit' ('// ' + ('x' * 94)) + +# A tab counts as four display columns, not one: 24 tabs plus a short comment is +# already past the limit even though the string is well under 98 characters. +$tabbed = ("`t" * 24) + '// short' +Assert-Category 'comment-line-tabs-count-as-columns' $tabbed 'comment-line-too-long' + +if ((Get-CommentDisplayWidth -Line "`tab" -TabWidth 4) -ne 6) { + [void]$failures.Add('FAIL [display-width-tab-stop]: expected a leading tab to advance to column 4') +} + +# Doc comments are exempt from the content budget but not from the width rule. +Assert-Category 'doc-comment-too-wide' "/// $widthPad" 'comment-line-too-long' + +$wrapSource = "`t// " + ('the quick brown fox jumps over the lazy dog and keeps running ' * 3) +$wrapped = Format-CommentLineWrap -Line $wrapSource -Language 'CLike' -MaxWidth 98 -TabWidth 4 +if ($null -eq $wrapped -or $wrapped.Count -lt 2) { + [void]$failures.Add('FAIL [wrap-splits]: expected an over-long comment to wrap onto multiple lines') +} +else { + foreach ($line in $wrapped) { + if (-not $line.TrimStart().StartsWith('//')) { + [void]$failures.Add("FAIL [wrap-prefix]: continuation line lost its comment prefix: $line") + } + } + if ((Get-CommentDisplayWidth -Line $wrapped[0] -TabWidth 4) -gt 98) { + [void]$failures.Add('FAIL [wrap-width]: wrapped line still exceeds the limit') + } +} + +if ($null -ne (Format-CommentLineWrap -Line '// short' -Language 'CLike' -MaxWidth 98 -TabWidth 4)) { + [void]$failures.Add('FAIL [wrap-noop]: a line that already fits must not be rewrapped') +} + +# A single-line Xml comment expands into the delimiters-on-their-own-lines form. +$xmlSource = '' +$xmlWrapped = Format-CommentLineWrap -Line $xmlSource -Language 'Xml' -MaxWidth 98 -TabWidth 4 +if ($null -eq $xmlWrapped -or $xmlWrapped.Count -lt 3) { + [void]$failures.Add('FAIL [wrap-xml-expands]: expected an over-long Xml comment to expand to multiple lines') +} +else { + if ($xmlWrapped[0].Trim() -ne '') { + [void]$failures.Add('FAIL [wrap-xml-delimiters]: expected the delimiters to move onto their own lines') + } + foreach ($line in $xmlWrapped) { + if ((Get-CommentDisplayWidth -Line $line -TabWidth 4) -gt 98) { + [void]$failures.Add("FAIL [wrap-xml-width]: wrapped Xml line still exceeds the limit: $line") + } + } +} + +# ---- complexity raises the content budget, but only in dense branching code ---- + +# Wrapped inside the width limit so these fixtures exercise the content budget +# alone; an over-wide single line would trip comment-line-too-long instead. +$longComment = @( + '// The invariant this restores is subtle and needs spelling out for the', + '// next reader, because the branching below depends on it holding at each', + '// step and nothing in the code itself says so. Without this note the next', + '// person reads the guard as redundant and deletes it.' +) + +function New-BranchyFixture { + param([string[]] $Comment) + $out = New-Object System.Collections.ArrayList + foreach ($line in $Comment) { [void]$out.Add($line) } + [void]$out.Add('void M(int n) {') + for ($k = 0; $k -lt 12; $k++) { [void]$out.Add(" if (n == $k && n > 0) { DoWork($k); }") } + [void]$out.Add('}') + return ,$out.ToArray() +} + +$simpleCode = @($longComment) + @('void M() {', ' DoWork();', '}') +Assert-CategoryLines 'long-comment-simple-code-flagged' $simpleCode 'comment-too-long' +Assert-CleanLines 'long-comment-complex-code-allowed' (New-BranchyFixture -Comment $longComment) + +# The extended budget is a higher ceiling, not the removal of one. +$hugeComment = $longComment + $longComment + $longComment +Assert-CategoryLines 'extended-budget-still-capped' (New-BranchyFixture -Comment $hugeComment) 'comment-too-long' + +$cxSimple = Measure-CodeComplexity -Lines @('void M() {', ' DoWork();', '}') -StartIndex 0 -Language 'CLike' +if ($cxSimple -ge 10) { + [void]$failures.Add("FAIL [complexity-simple]: straight-line code scored $cxSimple") +} +if ((Measure-CodeComplexity -Lines @('') -StartIndex 0 -Language 'Xml') -ne 0) { + [void]$failures.Add('FAIL [complexity-xml]: Xml has no control flow and must score zero') +} + +Remove-Item -LiteralPath $tempDir -Recurse -Force + +if ($failures.Count -gt 0) { + Write-Host '' + foreach ($f in $failures) { Write-Host $f -ForegroundColor Red } + Write-Host '' + Write-Host "$($failures.Count) test(s) failed." -ForegroundColor Red + exit 1 +} + +Write-Host 'All CommentHygiene tests passed.' -ForegroundColor Green +exit 0 \ No newline at end of file diff --git a/Build/Agent/CommentHygiene.psm1 b/Build/Agent/CommentHygiene.psm1 new file mode 100644 index 0000000000..4b4216eb90 --- /dev/null +++ b/Build/Agent/CommentHygiene.psm1 @@ -0,0 +1,748 @@ +<# +.SYNOPSIS + Shared comment-hygiene scanning engine for FieldWorks source and project files. + +.DESCRIPTION + Implements the mechanical (regex-detectable) banned-content categories + from the fieldworks-code-commenting skill, the ASCII-punctuation-only rule, + a 200-character budget on implementation comments, and a per-line width + limit read from .editorconfig. Judgment-based rules (accuracy, + WHAT-not-HOW, standalone clarity) are not checked here. + + The content budget rises to 600 characters where Measure-CodeComplexity + scores the code the comment introduces at 10 decision points or more; the + width limit has no such exemption and applies to doc comments too. + + Scans .cs/.cpp/.h/.hpp/.cc/.cxx/.c/.idl (//, ///), .ps1/.psm1 (#, + block-comment), and .csproj/.vcxproj/.vcproj/.props/.targets/.proj/ + .axaml/.xaml () files. Only whole-line comments are scanned; a + trailing same-line comment is not, and a C-style /* */ block comment is + not (unlike its XML counterpart, which this module does parse). + A doc comment, a PowerShell help block, or a file's first Xml + block is exempt from the length cap -- the last one plays the same + file/type-summary role as /// or a help block, since Xml has no separate + doc-comment syntax to mark it by. (This help block cannot spell out that + block-comment syntax literally -- PowerShell does not nest it, and the + first close token would end this block early.) + +.NOTES + Import this module from comment-hygiene.ps1, comment-hygiene-repair.ps1, + and comment-hygiene-blame.ps1: + Import-Module "$PSScriptRoot/CommentHygiene.psm1" -Force +#> + +Set-StrictMode -Version Latest + +# Declared at module scope: under Set-StrictMode, a script-scoped variable read +# before its first assignment throws rather than returning $null. +$script:editorConfigCache = @{} + +function Get-CommentHygieneCategories { + <# + .SYNOPSIS + Returns the ordered category-name to regex-pattern map. + #> + return [ordered]@{ + # "Stage N"/"this commit" excluded: this codebase's own "stage-1/2" and Commit() are + # domain terms, not migration framing. + 'process-framing' = '(?i)\bPhase[\s-]?\d+\b|\blater we\x27ll\b|\bwe\x27ll (?:later|eventually)\b' + # (?-i:...) forces case-sensitivity despite the earlier (?i); F-keys and T-generics are + # excluded as letter-plus-digit look-alikes. + 'doc-pointer' = '\b[\w./-]+\.md\b|(?i)\bsection\s+\d+[a-z]?\b|(?-i:(? + $escaped = (Get-NonAsciiReplacementMap).Keys | ForEach-Object { [regex]::Escape($_) } + return '(?:' + ($escaped -join '|') + ')' +} + +function Get-CommentHygieneEditorConfig { + <# + .SYNOPSIS + Reads max_line_length and tab_width from the repo's .editorconfig [*] section. + + .DESCRIPTION + The comment width limit is not a second opinion about formatting: it is + whatever .editorconfig already declares for every file, so the two can + never drift apart. Only the [*] section is read, since that is where + this repo declares both values. Results are cached per root -- this runs + once per gate invocation, not once per file. + + .OUTPUTS + A hashtable with MaxLineLength and TabWidth. Falls back to 98 and 4 when + .editorconfig is missing or declares neither. + #> + param([Parameter(Mandatory)][string] $RepoRoot) + + if ($script:editorConfigCache.ContainsKey($RepoRoot)) { return $script:editorConfigCache[$RepoRoot] } + + $settings = @{ MaxLineLength = 98; TabWidth = 4 } + $path = Join-Path $RepoRoot '.editorconfig' + if (Test-Path -LiteralPath $path) { + $inStarSection = $false + foreach ($raw in [System.IO.File]::ReadAllLines($path, [System.Text.Encoding]::UTF8)) { + $line = $raw.Trim() + if ($line.StartsWith('#') -or $line.Length -eq 0) { continue } + if ($line.StartsWith('[')) { $inStarSection = ($line -eq '[*]'); continue } + if (-not $inStarSection) { continue } + if ($line -match '^max_line_length\s*=\s*(\d+)$') { $settings.MaxLineLength = [int]$Matches[1] } + if ($line -match '^tab_width\s*=\s*(\d+)$') { $settings.TabWidth = [int]$Matches[1] } + } + } + + $script:editorConfigCache[$RepoRoot] = $settings + return $settings +} + +function Get-CommentDisplayWidth { + <# + .SYNOPSIS + Returns a line's width in display columns, expanding tabs to the next tab stop. + + .DESCRIPTION + String.Length counts a tab as one character; .editorconfig's + max_line_length counts display columns. This repo indents with tabs, so + a comment four levels deep differs by twelve columns between the two + measures -- enough to decide a violation either way. + #> + param( + [Parameter(Mandatory)][AllowEmptyString()][string] $Line, + [Parameter(Mandatory)][int] $TabWidth + ) + + $width = 0 + foreach ($char in $Line.ToCharArray()) { + if ($char -eq "`t") { $width += $TabWidth - ($width % $TabWidth) } + else { $width++ } + } + return $width +} + +function Get-CommentBody { + <# + .SYNOPSIS + Returns the text after // or ///, or $null if the line is not a whole-line C-family + comment. + + .DESCRIPTION + Shared by every C-family language this module scans (C#, C/C++, IDL): + all four use identical // and /// line-comment syntax. + #> + param([Parameter(Mandatory)][AllowEmptyString()][string] $Line) + + $trimmed = $Line.Trim() + if ($trimmed.StartsWith('///')) { return $trimmed.Substring(3) } + if ($trimmed.StartsWith('//')) { return $trimmed.Substring(2) } + return $null +} + +function Get-CommentHygieneLanguage { + <# + .SYNOPSIS + Classifies a file path into a comment syntax family by extension. + + .OUTPUTS + 'PowerShell', 'CLike' (C#/C/C++/IDL), 'Xml' (project files and + Avalonia views), or $null for an unrecognized extension. + #> + param([Parameter(Mandatory)][string] $Path) + + if ($Path -match '\.(ps1|psm1)$') { return 'PowerShell' } + if ($Path -match '\.(cs|cpp|cxx|cc|c|h|hpp|idl)$') { return 'CLike' } + if ($Path -match '\.(csproj|vcxproj|vcproj|props|targets|proj|axaml|xaml)$') { return 'Xml' } + return $null +} + +function Get-NonAsciiReplacementMap { + <# + .SYNOPSIS + Returns the ordered map of non-ASCII characters to their ASCII replacement text. + + .DESCRIPTION + Keys are built from [char] code points, not the backtick-u{} escape + (that escape is PowerShell 6+ only; under Windows PowerShell 5.1 the + backtick is silently dropped and the literal text "u{2014}" remains, + so the map would never match a real em dash). + #> + $map = [ordered]@{} + $map[[string][char]0x2014] = '--' + $map[[string][char]0x2013] = '-' + $map[[string][char]0x2192] = '->' + $map[[string][char]0x2190] = '<-' + $map[[string][char]0x2194] = '<->' + $map[[string][char]0x2026] = '...' + $map[[string][char]0x22ee] = '...' + $map[[string][char]0x2022] = '-' + $map[[string][char]0x00d7] = 'x' + $map[[string][char]0x2018] = "'" + $map[[string][char]0x2019] = "'" + $map[[string][char]0x201c] = '"' + $map[[string][char]0x201d] = '"' + $map[[string][char]0x00a7] = 'Section' + return $map +} + +function Set-CommentHygieneFileContent { + <# + .SYNOPSIS + Writes lines back to a file with an explicit BOM choice. + + .DESCRIPTION + Uses System.Text.UTF8Encoding directly instead of + Set-Content -Encoding utf8BOM/utf8NoBOM: those encoding names are + PowerShell 7+ only and throw a parameter-binding error under + Windows PowerShell 5.1. + + .PARAMETER Utf8Bom + Whether the written file should carry a UTF-8 byte-order mark. + #> + param( + [Parameter(Mandatory)][string] $Path, + [Parameter(Mandatory)][AllowEmptyCollection()][AllowEmptyString()][string[]] $Lines, + [Parameter(Mandatory)][bool] $Utf8Bom + ) + + $encoding = [System.Text.UTF8Encoding]::new($Utf8Bom) + [System.IO.File]::WriteAllLines($Path, $Lines, $encoding) +} + +function Repair-CommentLine { + <# + .SYNOPSIS + Applies the ASCII replacement map to a whole-line comment. + + .PARAMETER Line + A single raw source line. + + .OUTPUTS + The fixed line if the line is a whole-line // or # comment and every + non-ascii-punctuation character in it is covered by + Get-NonAsciiReplacementMap; otherwise $null (not a whole-line comment, + or an unmapped non-ascii-punctuation character remains). Any other + non-ASCII content (real script/IPA text) is left untouched and does + not block repair -- only the specific characters in the replacement + map are ever in scope. + + Only ever called on lines Get-CommentHygieneViolations already + classified as a comment for that file's language, so a bare `#` + here is never a PowerShell string or a C# directive -- the caller + guarantees that, this function does not re-derive the file's language. + #> + param([Parameter(Mandatory)][AllowEmptyString()][string] $Line) + + $trimmed = $Line.Trim() + $prefixLength = $Line.Length - $Line.TrimStart().Length + $leadingWhitespace = $Line.Substring(0, $prefixLength) + + $prefix = $null + $body = $null + if ($trimmed.StartsWith('///')) { $prefix = '///'; $body = $trimmed.Substring(3) } + elseif ($trimmed.StartsWith('//')) { $prefix = '//'; $body = $trimmed.Substring(2) } + elseif ($trimmed.StartsWith('#') -and -not $trimmed.StartsWith('#>')) { $prefix = '#'; $body = $trimmed.Substring(1) } + + if ($null -eq $prefix) { return $null } + + $fixedBody = $body + $replacementMap = Get-NonAsciiReplacementMap + foreach ($key in $replacementMap.Keys) { + $fixedBody = $fixedBody -replace [regex]::Escape($key), $replacementMap[$key] + } + + if ($fixedBody -match (Get-NonAsciiPunctuationPattern)) { return $null } + + return "$leadingWhitespace$prefix$fixedBody" +} + +function ConvertTo-TabIndent { + <# + .SYNOPSIS + Rewrites a tab-indented line's leading whitespace as tabs alone. + + .DESCRIPTION + Continuation lines in this repo often align under an opening delimiter + with a tab followed by spaces. Copying that onto a newly written line + trips git's indent-with-non-tab check, so the run is re-expressed as + whole tabs of the same approximate depth. An indent with no tab in it + belongs to a space-indented file and is returned unchanged. + #> + param( + [Parameter(Mandatory)][AllowEmptyString()][string] $Indent, + [Parameter(Mandatory)][int] $TabWidth + ) + + if (-not $Indent.Contains("`t")) { return $Indent } + $width = Get-CommentDisplayWidth -Line $Indent -TabWidth $TabWidth + return "`t" * [Math]::Max(1, [int][Math]::Floor($width / $TabWidth)) +} + +function Split-CommentWords { + <# + .SYNOPSIS + Greedy word wrap for comment text, given the width its prefix consumes. + + .OUTPUTS + One string per output line, prefix excluded; empty when Body has no + words. A word too long to fit gets a line to itself and stays over the + limit rather than being broken mid-token. + #> + param( + [Parameter(Mandatory)][AllowEmptyString()][string] $Body, + [Parameter(Mandatory)][AllowEmptyString()][string] $Head, + [Parameter(Mandatory)][int] $MaxWidth, + [Parameter(Mandatory)][int] $TabWidth + ) + + $words = @($Body.Trim() -split '\s+' | Where-Object { $_.Length -gt 0 }) + if ($words.Count -eq 0) { return ,@() } + + $headWidth = Get-CommentDisplayWidth -Line $Head -TabWidth $TabWidth + $lines = New-Object System.Collections.ArrayList + $current = $null + + foreach ($word in $words) { + if ($null -eq $current) { $current = $word; continue } + if (($headWidth + $current.Length + 1 + $word.Length) -le $MaxWidth) { $current = "$current $word"; continue } + [void]$lines.Add($current) + $current = $word + } + [void]$lines.Add($current) + + # Unary comma: a one-element array would otherwise unroll to a bare string. + return ,$lines.ToArray() +} + +function Format-CommentLineWrap { + <# + .SYNOPSIS + Re-wraps an over-long whole-line comment to fit a display-column limit. + + .DESCRIPTION + Greedy word wrap that reuses the line's own indentation and comment + prefix for every continuation line, so the result is indistinguishable + from hand-wrapped prose. A single word longer than the available width + is never broken; it gets a line of its own and stays over the limit. + + An Xml comment that carries its own delimiters is expanded into the + multi-line form this repo already uses elsewhere: the delimiters move to + lines of their own and the content is wrapped and indented between them. + The comment's extent is unchanged. + + .OUTPUTS + The wrapped lines, or $null when the line is not a whole-line comment or + already fits. + #> + param( + [Parameter(Mandatory)][AllowEmptyString()][string] $Line, + [Parameter(Mandatory)][ValidateSet('PowerShell', 'CLike', 'Xml')][string] $Language, + [Parameter(Mandatory)][int] $MaxWidth, + [Parameter(Mandatory)][int] $TabWidth + ) + + if ((Get-CommentDisplayWidth -Line $Line -TabWidth $TabWidth) -le $MaxWidth) { return $null } + + $trimmed = $Line.Trim() + $leadingWhitespace = ConvertTo-TabIndent -Indent $Line.Substring(0, $Line.Length - $Line.TrimStart().Length) -TabWidth $TabWidth + + if ($Language -eq 'Xml') { + $opens = $trimmed.StartsWith('') + $content = $trimmed + if ($opens) { $content = $content.Substring(4) } + if ($closes) { $content = $content.Substring(0, $content.Length - 3) } + $content = $content.Trim() + if ($content.Length -eq 0) { return $null } + + $contentIndent = $leadingWhitespace + if ($opens) { $contentIndent = "$leadingWhitespace`t" } + + $out = New-Object System.Collections.ArrayList + if ($opens) { [void]$out.Add("$leadingWhitespace") } + + if ($out.Count -eq 0) { return $null } + if ($out.Count -eq 1 -and $out[0] -eq $Line) { return $null } + return $out.ToArray() + } + + $prefix = $null + $body = $null + if ($Language -eq 'PowerShell') { + # A help block's delimiters are left alone; its content lines carry no + # prefix of their own, so they wrap on indentation like Xml content. + if ($trimmed.StartsWith('<#') -or $trimmed.StartsWith('#>')) { return $null } + if ($trimmed.StartsWith('#')) { $prefix = '#'; $body = $trimmed.Substring(1) } + else { $prefix = ''; $body = $trimmed } + } + else { + if ($trimmed.StartsWith('///')) { $prefix = '///'; $body = $trimmed.Substring(3) } + elseif ($trimmed.StartsWith('//')) { $prefix = '//'; $body = $trimmed.Substring(2) } + else { return $null } + } + + $separator = '' + if ($body.StartsWith(' ')) { $separator = ' ' } + $head = "$leadingWhitespace$prefix$separator" + + $wrapped = New-Object System.Collections.ArrayList + foreach ($piece in (Split-CommentWords -Body $body -Head $head -MaxWidth $MaxWidth -TabWidth $TabWidth)) { + [void]$wrapped.Add("$head$piece") + } + + # A single line is still progress when the run of interior whitespace it + # collapses is what pushed the line over; only an unchanged line is refused. + if ($wrapped.Count -eq 0) { return $null } + if ($wrapped.Count -eq 1 -and $wrapped[0] -eq $Line) { return $null } + return $wrapped.ToArray() +} + +function Measure-CodeComplexity { + <# + .SYNOPSIS + Counts decision points in the code a comment block introduces. + + .DESCRIPTION + A keyword and operator count, not a parsed control-flow graph: enough to + tell a dense branching region from ordinary straight-line code, with no + AST or language service. Scanning starts at the first line after the + comment and stops at the end of the enclosing block (brace depth for + C-family and PowerShell) or after MaxWindow lines, whichever comes + first, so the score describes the code the comment actually introduces. + + Xml scores zero: a project file or view has no control flow to be + complex. + + .OUTPUTS + The decision-point count. McCabe complexity is this plus one. + #> + param( + [Parameter(Mandatory)][AllowEmptyCollection()][AllowEmptyString()][string[]] $Lines, + [Parameter(Mandatory)][int] $StartIndex, + [Parameter(Mandatory)][ValidateSet('PowerShell', 'CLike', 'Xml')][string] $Language, + [int] $MaxWindow = 40 + ) + + if ($Language -eq 'Xml') { return 0 } + + $patterns = if ($Language -eq 'PowerShell') { + @('(?i)\bif\s*\(', '(?i)\belseif\s*\(', '(?i)\bfor\s*\(', '(?i)\bforeach\s*\(', + '(?i)\bwhile\s*\(', '(?i)\bswitch\s*[\(-]', '(?i)\bcatch\b', '(?i)\btrap\b', + '(?i)\bwhere-object\b', '\s-and\s', '\s-or\s') + } + else { + @('\bif\s*\(', '\bfor\s*\(', '\bforeach\s*\(', '\bwhile\s*\(', '\bcase\b', + '\bcatch\s*\(', '&&', '\|\|', '\?\?', '(? -- project files, Avalonia views). A bare # is + never treated as a comment for a CLike file, so a preprocessor + directive (#region, #if) is never misread as one. + + .OUTPUTS + A hashtable with parallel arrays Kinds ('impl', 'exempt', or $null + per line) and Bodies (comment text per line, or $null). Xml's first + block in the file is 'exempt' -- the same role a C# /// or + PowerShell help block plays, since XML has no separate doc-comment + syntax to mark a file/type-level summary. Every later Xml comment is + 'impl', subject to the length budget. + #> + param( + [Parameter(Mandatory)][AllowEmptyCollection()][AllowEmptyString()][string[]] $Lines, + [Parameter(Mandatory)][ValidateSet('PowerShell', 'CLike', 'Xml')][string] $Language + ) + + $kinds = New-Object 'object[]' $Lines.Count + $bodies = New-Object 'object[]' $Lines.Count + $inHelpBlock = $false + $inXmlComment = $false + $xmlCommentBlockCount = 0 + $currentXmlCommentKind = $null + + for ($i = 0; $i -lt $Lines.Count; $i++) { + $trimmed = $Lines[$i].Trim() + + if ($Language -eq 'PowerShell') { + if ($inHelpBlock) { + $kinds[$i] = 'exempt' + $bodies[$i] = $trimmed + if ($trimmed.EndsWith('#>')) { $inHelpBlock = $false } + continue + } + if ($trimmed.StartsWith('<#')) { + $kinds[$i] = 'exempt' + $bodies[$i] = $trimmed.Substring(2).TrimEnd('#', '>', ' ') + if (-not $trimmed.EndsWith('#>')) { $inHelpBlock = $true } + continue + } + if ($trimmed.StartsWith('#')) { + $kinds[$i] = 'impl' + $bodies[$i] = $trimmed.Substring(1) + continue + } + } + elseif ($Language -eq 'Xml') { + if ($inXmlComment) { + $kinds[$i] = $currentXmlCommentKind + $endIndex = $trimmed.IndexOf('-->') + if ($endIndex -ge 0) { + $bodies[$i] = $trimmed.Substring(0, $endIndex) + $inXmlComment = $false + } + else { + $bodies[$i] = $trimmed + } + continue + } + if ($trimmed.StartsWith('') + if ($endIndex -ge 0) { + $bodies[$i] = $rest.Substring(0, $endIndex) + } + else { + $bodies[$i] = $rest + $inXmlComment = $true + } + continue + } + } + else { + # Inlined from Get-CommentBody: this loop runs every line of every diffed file on + # every build, where a per-line function call plus its own redundant Trim measurably + # slows the gate down. + if ($trimmed.StartsWith('///')) { + $kinds[$i] = 'exempt' + $bodies[$i] = $trimmed.Substring(3) + continue + } + if ($trimmed.StartsWith('//')) { + $kinds[$i] = 'impl' + $bodies[$i] = $trimmed.Substring(2) + continue + } + } + + $kinds[$i] = $null + $bodies[$i] = $null + } + + return @{ Kinds = $kinds; Bodies = $bodies } +} + +function Get-CommentHygieneViolations { + <# + .SYNOPSIS + Scans the given files for mechanical comment-hygiene violations. + + .PARAMETER Files + Absolute paths to files to scan. Files whose extension + Get-CommentHygieneLanguage does not recognize are skipped. + + .PARAMETER LineFilter + Optional hashtable mapping an absolute file path to a + HashSet[int] of 1-based line numbers to check. Omit to scan every + line in every file. + + .OUTPUTS + One PSCustomObject per violation: File, Line, Category, Text. + Category 'comment-too-long' additionally covers a run of + consecutive implementation-comment lines whose combined text + exceeds a character budget; a doc comment or a PowerShell help + block is exempt from that one. + #> + param( + [Parameter(Mandatory)][string[]] $Files, + [hashtable] $LineFilter, + [string] $RepoRoot + ) + + if ([string]::IsNullOrWhiteSpace($RepoRoot)) { + $RepoRoot = (Resolve-Path (Join-Path $PSScriptRoot '../..')).Path + } + $editorConfig = Get-CommentHygieneEditorConfig -RepoRoot $RepoRoot + + $categories = Get-CommentHygieneCategories + $violations = New-Object System.Collections.ArrayList + $maxImplCommentChars = 200 + + # Raised budget for a comment introducing a dense branching region, where the + # reader needs the invariants spelled out and 200 characters buys two + # sentences. Measure-CodeComplexity decides; nothing opts in by hand. + $extendedImplCommentChars = 600 + $complexityThreshold = 10 + + foreach ($file in $Files) { + if (-not (Test-Path -LiteralPath $file)) { continue } + + $language = Get-CommentHygieneLanguage -Path $file + if ($null -eq $language) { continue } + + $allowedLines = $null + if ($LineFilter -and $LineFilter.ContainsKey($file)) { + $allowedLines = $LineFilter[$file] + } + + # File.ReadAllLines, not Get-Content: ~50x faster on a large file, and this gate runs + # every build. Still BOM-safe: StreamReader sniffs a real BOM even given an explicit + # encoding. + $lines = [System.IO.File]::ReadAllLines($file, [System.Text.Encoding]::UTF8) + $classification = Get-CommentLineClassification -Lines $lines -Language $language + $kinds = $classification.Kinds + $bodies = $classification.Bodies + + for ($i = 0; $i -lt $lines.Count; $i++) { + if ($null -eq $kinds[$i]) { continue } + $lineNumber = $i + 1 + if ($allowedLines -and -not $allowedLines.Contains($lineNumber)) { continue } + + foreach ($category in $categories.Keys) { + if ($bodies[$i] -match $categories[$category]) { + [void]$violations.Add([PSCustomObject]@{ + File = $file + Line = $lineNumber + Category = $category + Text = $bodies[$i].Trim() + }) + } + } + + # Physical width, from .editorconfig's max_line_length, applied to every comment + # line including doc comments: a doc comment is exempt from the content budget + # because of what it says, not because it may run off the screen. + $displayWidth = Get-CommentDisplayWidth -Line $lines[$i] -TabWidth $editorConfig.TabWidth + if ($displayWidth -gt $editorConfig.MaxLineLength) { + [void]$violations.Add([PSCustomObject]@{ + File = $file + Line = $lineNumber + Category = 'comment-line-too-long' + Text = ("{0} columns (max {1}): {2}" -f $displayWidth, $editorConfig.MaxLineLength, $bodies[$i].Trim()) + }) + } + + # Xml-only: XML forbids a literal "--" anywhere in comment content, not just next to + # "-->", so the usual em-dash-to-"--" fix is invalid here; use a single "-" instead. + if ($language -eq 'Xml' -and $bodies[$i] -match '--') { + [void]$violations.Add([PSCustomObject]@{ + File = $file + Line = $lineNumber + Category = 'xml-illegal-double-hyphen' + Text = $bodies[$i].Trim() + }) + } + } + + $blockStart = -1 + $blockLength = 0 + for ($i = 0; $i -le $lines.Count; $i++) { + $isImplLine = ($i -lt $lines.Count) -and ($kinds[$i] -eq 'impl') + if ($isImplLine) { + if ($blockStart -lt 0) { $blockStart = $i } + $blockLength++ + continue + } + + if ($blockLength -gt 0) { + $blockIndexes = $blockStart..($blockStart + $blockLength - 1) + $totalChars = [int]($blockIndexes | ForEach-Object { $bodies[$_].Trim().Length } | Measure-Object -Sum).Sum + + $budget = $maxImplCommentChars + if ($totalChars -gt $budget) { + $complexity = Measure-CodeComplexity -Lines $lines -StartIndex ($blockStart + $blockLength) -Language $language + if ($complexity -ge $complexityThreshold) { $budget = $extendedImplCommentChars } + } + + if ($totalChars -gt $budget) { + # Blame only this diff's contribution: skip if the untouched lines alone already exceeded budget. + $untouchedChars = 0 + if ($allowedLines) { + $untouchedIndexes = $blockIndexes | Where-Object { -not $allowedLines.Contains($_ + 1) } + if ($untouchedIndexes) { + $untouchedChars = [int]($untouchedIndexes | ForEach-Object { $bodies[$_].Trim().Length } | Measure-Object -Sum).Sum + } + } + if ($untouchedChars -le $budget) { + [void]$violations.Add([PSCustomObject]@{ + File = $file + Line = $blockStart + 1 + Category = 'comment-too-long' + Text = ("{0} chars (budget {1}): {2}" -f $totalChars, $budget, $bodies[$blockStart].Trim()) + }) + } + } + } + $blockStart = -1 + $blockLength = 0 + } + } + + # The unary comma prevents PowerShell's pipeline from unrolling a + # zero- or one-element array into $null or a bare scalar on return. + return ,$violations.ToArray() +} + +Export-ModuleMember -Function @( + 'Get-CommentHygieneCategories', + 'Get-CommentBody', + 'Get-CommentDisplayWidth', + 'Get-CommentHygieneEditorConfig', + 'Get-CommentHygieneLanguage', + 'Get-CommentLineClassification', + 'Get-CommentHygieneViolations', + 'Get-NonAsciiReplacementMap', + 'Get-NonAsciiPunctuationPattern', + 'Format-CommentLineWrap', + 'Measure-CodeComplexity', + 'Repair-CommentLine', + 'Set-CommentHygieneFileContent' +) diff --git a/Build/Agent/check-and-fix-whitespace.sh b/Build/Agent/check-and-fix-whitespace.sh deleted file mode 100644 index 1aa31c88d2..0000000000 --- a/Build/Agent/check-and-fix-whitespace.sh +++ /dev/null @@ -1,6 +0,0 @@ -#!/usr/bin/env bash -set +e -bash "$(dirname "$0")/check-whitespace.sh" -ec=$? -bash "$(dirname "$0")/fix-whitespace.sh" -exit $ec diff --git a/Build/Agent/check-whitespace.sh b/Build/Agent/check-whitespace.sh deleted file mode 100644 index 0c7be3034e..0000000000 --- a/Build/Agent/check-whitespace.sh +++ /dev/null @@ -1,72 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# shellcheck source=lib_git.sh -source "${SCRIPT_DIR}/lib_git.sh" - -# Determine base commit/ref -BASE_SHA="${1:-}" -if [[ -z "$BASE_SHA" ]]; then - if [[ -n "${GITHUB_EVENT_PULL_REQUEST_BASE_SHA:-}" ]]; then - BASE_SHA="$GITHUB_EVENT_PULL_REQUEST_BASE_SHA" - elif [[ -n "${GITHUB_BASE_REF:-}" ]]; then - BASE_SHA="origin/${GITHUB_BASE_REF}" - else - BASE_SHA="$(git_default_branch_ref)" - fi -fi - -echo "Checking whitespace with git log --check from ${BASE_SHA}..HEAD" -git log --check --pretty=format:"---% h% s" "${BASE_SHA}.." | tee check-results.log - -# Parse results to prepare a summary if running in GitHub Actions -problems=() -commit="" -commitText="" -commitTextmd="" -while IFS='' read -r line || [[ -n "$line" ]]; do - case "$line" in - "--- "*) - # format: --- - read -r _ commit commitText <<<"$line" - if [[ -n "${GITHUB_REPOSITORY:-}" ]]; then - commitTextmd="[${commit}](https://github.com/${GITHUB_REPOSITORY}/commit/${commit}) ${commitText}" - else - commitTextmd="${commit} ${commitText}" - fi - ;; - "") ;; - *:[1-9]*:*) - file="${line%%:*}" - afterFile="${line#*:}" - lineNumber="${afterFile%%:*}" - problems+=("[${commitTextmd}]") - if [[ -n "${GITHUB_REPOSITORY:-}" ]] && [[ -n "${GITHUB_REF_NAME:-}" ]]; then - problems+=("[${line}](https://github.com/${GITHUB_REPOSITORY}/blob/${GITHUB_REF_NAME}/${file}#L${lineNumber})") - else - problems+=("${line}") - fi - problems+=("") - ;; - esac -done < check-results.log - -if [[ ${#problems[@]} -gt 0 ]]; then - echo "Whitespace issues were found." >&2 - if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then - { - echo "⚠️ Please review the Summary output for further information." - echo "### A whitespace issue was found in one or more of the commits." - echo - echo "Errors:" - for i in "${problems[@]}"; do - echo "$i" - done - } >> "$GITHUB_STEP_SUMMARY" - fi - exit 1 -fi - -echo "No problems found" -exit 0 diff --git a/Build/Agent/comment-hygiene-blame.ps1 b/Build/Agent/comment-hygiene-blame.ps1 new file mode 100644 index 0000000000..75fe4b166c --- /dev/null +++ b/Build/Agent/comment-hygiene-blame.ps1 @@ -0,0 +1,103 @@ +<# +.SYNOPSIS + Attributes every full-repo comment-hygiene violation to its introducing commit. + +.DESCRIPTION + Runs Get-CommentHygieneViolations in full-repo mode, then git-blames each + violating line to find who introduced it and when, so a human can triage + existing debt deliberately instead of the ratchet gate accepting it + silently forever. Writes one JSON array to -OutputPath. + +.PARAMETER OutputPath + Path to write the JSON report. + +.EXAMPLE + Build/Agent/comment-hygiene-blame.ps1 -OutputPath triage.json +#> +[CmdletBinding()] +param( + [Parameter(Mandatory)][string] $OutputPath +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +$repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '../..')).Path +Import-Module (Join-Path $PSScriptRoot 'CommentHygiene.psm1') -Force + +function Test-ExcludedPath { + param([string] $Path) + return ($Path -match '\.g\.cs$') -or ($Path -match 'Designer\.cs$') +} + +function ConvertTo-RepoPath { + param([string] $RelativePath) + return Join-Path $repoRoot ($RelativePath -replace '/', [IO.Path]::DirectorySeparatorChar) +} + +function Get-BlameInfo { + param([string] $File, [int] $Line) + + $porcelain = git blame -L "$Line,$Line" --porcelain -- $File 2>$null + if (-not $porcelain) { return $null } + + $sha = ($porcelain[0] -split ' ')[0] + $authorLine = $porcelain | Where-Object { $_ -like 'author *' } | Select-Object -First 1 + $emailLine = $porcelain | Where-Object { $_ -like 'author-mail *' } | Select-Object -First 1 + $timeLine = $porcelain | Where-Object { $_ -like 'author-time *' } | Select-Object -First 1 + + $author = if ($authorLine) { $authorLine.Substring(7) } else { 'unknown' } + $email = if ($emailLine) { $emailLine.Substring(12).Trim('<', '>') } else { '' } + $epoch = if ($timeLine) { [int64]($timeLine.Substring(12)) } else { 0 } + $date = if ($epoch -gt 0) { [DateTimeOffset]::FromUnixTimeSeconds($epoch).UtcDateTime.ToString('yyyy-MM-dd') } else { '' } + + $subject = (git log -1 --format=%s $sha 2>$null) + $bodyLines = git log -1 --format=%B $sha 2>$null + $body = ($bodyLines -join "`n") + $hasAiTrailer = $body -match '(?i)co-authored-by:.*claude|generated with claude' + + return [PSCustomObject]@{ + CommitSha = $sha + Author = "$author <$email>" + Date = $date + Subject = $subject + HasAiCoAuthorTrailer = [bool]$hasAiTrailer + } +} + +$scopedGlobs = @( + '*.cs', '*.ps1', '*.psm1', + '*.cpp', '*.cxx', '*.cc', '*.c', '*.h', '*.hpp', '*.idl', + '*.csproj', '*.vcxproj', '*.vcproj', '*.props', '*.targets', '*.proj', '*.axaml', '*.xaml' +) +$files = git ls-files $scopedGlobs | ForEach-Object { ConvertTo-RepoPath $_ } | Where-Object { -not (Test-ExcludedPath $_) } +Write-Host "comment-hygiene-blame: scanning $($files.Count) file(s)..." + +$violations = Get-CommentHygieneViolations -Files $files +Write-Host "comment-hygiene-blame: $($violations.Count) violation(s) found; resolving blame..." + +$report = New-Object System.Collections.ArrayList +$index = 0 + +foreach ($v in $violations) { + $index++ + if ($index % 50 -eq 0) { Write-Host " ...$index/$($violations.Count)" } + + $blame = Get-BlameInfo -File $v.File -Line $v.Line + $relative = $v.File.Substring($repoRoot.Length + 1) -replace '\\', '/' + + [void]$report.Add([PSCustomObject]@{ + file = $relative + line = $v.Line + category = $v.Category + text = $v.Text + commitSha = if ($blame) { $blame.CommitSha } else { $null } + author = if ($blame) { $blame.Author } else { $null } + date = if ($blame) { $blame.Date } else { $null } + subject = if ($blame) { $blame.Subject } else { $null } + hasAiCoAuthorTrailer = if ($blame) { $blame.HasAiCoAuthorTrailer } else { $false } + }) +} + +Set-CommentHygieneFileContent -Path $OutputPath -Lines @($report | ConvertTo-Json -Depth 4) -Utf8Bom $false +Write-Host "comment-hygiene-blame: wrote $($report.Count) record(s) to $OutputPath" diff --git a/Build/Agent/comment-hygiene-repair.ps1 b/Build/Agent/comment-hygiene-repair.ps1 new file mode 100644 index 0000000000..27436897be --- /dev/null +++ b/Build/Agent/comment-hygiene-repair.ps1 @@ -0,0 +1,74 @@ +<# +.SYNOPSIS + One-time non-ascii-punctuation comment repair sweep for an explicit file list. + +.DESCRIPTION + Runs Get-CommentHygieneViolations against the given files, filtered to + the non-ascii-punctuation category, and applies Repair-CommentLine to each hit, + writing fixes back to disk. Reports how many lines were fixed, in how + many files, and lists any (file, line, text) that could not be + auto-fixed because the line carries a character outside + Get-NonAsciiReplacementMap, so a human can handle those individually. + + Accepts an explicit file list rather than a commit SHA or -Full scope, + since the caller is expected to already know which files are in scope + (e.g. from a comment-hygiene-blame.ps1 triage report). + +.PARAMETER Files + Paths (absolute or relative to the current directory) to sweep. Any + extension Get-CommentHygieneLanguage recognizes is supported. + +.EXAMPLE + Build/Agent/comment-hygiene-repair.ps1 -Files (Get-Content scoped-files.txt) +#> +[CmdletBinding()] +param( + [Parameter(Mandatory)][string[]] $Files +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +Import-Module (Join-Path $PSScriptRoot 'CommentHygiene.psm1') -Force + +function Test-Utf8Bom { + param([string] $Path) + $bytes = [System.IO.File]::ReadAllBytes($Path) + return ($bytes.Length -ge 3) -and ($bytes[0] -eq 0xEF) -and ($bytes[1] -eq 0xBB) -and ($bytes[2] -eq 0xBF) +} + +$resolvedFiles = $Files | ForEach-Object { (Resolve-Path -LiteralPath $_).Path } + +# Assign before piping -- Get-CommentHygieneViolations's comma-return makes a direct pipe into +# Where-Object deliver one bundled array instead of filtering per-element. +$allViolations = Get-CommentHygieneViolations -Files $resolvedFiles +$violations = $allViolations | Where-Object { $_.Category -eq 'non-ascii-punctuation' } + +$fixedFiles = @{} +$fixedCount = 0 +$unrepairable = New-Object System.Collections.ArrayList + +foreach ($v in $violations) { + $hadBom = Test-Utf8Bom -Path $v.File + $fileLines = @(Get-Content -LiteralPath $v.File -Encoding UTF8) + $fixedLine = Repair-CommentLine -Line $fileLines[$v.Line - 1] + if ($null -eq $fixedLine) { + [void]$unrepairable.Add($v) + continue + } + + $fileLines[$v.Line - 1] = $fixedLine + Set-CommentHygieneFileContent -Path $v.File -Lines $fileLines -Utf8Bom $hadBom + $fixedFiles[$v.File] = $true + $fixedCount++ +} + +Write-Host "comment-hygiene-repair: fixed $fixedCount non-ascii-punctuation comment line(s) in $($fixedFiles.Keys.Count) file(s)." + +if ($unrepairable.Count -gt 0) { + Write-Host '' + Write-Host "comment-hygiene-repair: $($unrepairable.Count) violation(s) could NOT be auto-fixed (unmapped character(s)); handle these individually:" -ForegroundColor Yellow + foreach ($v in $unrepairable) { + Write-Host (" {0}:{1} {2}" -f $v.File, $v.Line, $v.Text) + } +} diff --git a/Build/Agent/comment-hygiene.ps1 b/Build/Agent/comment-hygiene.ps1 new file mode 100644 index 0000000000..15585e2445 --- /dev/null +++ b/Build/Agent/comment-hygiene.ps1 @@ -0,0 +1,227 @@ +<# +.SYNOPSIS + Diff-scoped comment-hygiene gate for FieldWorks source and project comments. + +.DESCRIPTION + Enforces the mechanical banned-content categories, plus a one-line cap on + implementation comments, against lines a diff ADDS, not the whole + repository. Legacy comments are never flagged unless their line is + touched again. Covers C#/C/C++/IDL, PowerShell, and the XML comments in + project files and Avalonia views -- see Get-CommentHygieneLanguage. + +.PARAMETER BaseRef + Git ref to diff against. Defaults to the PR base in CI + (GITHUB_EVENT_PULL_REQUEST_BASE_SHA, then GITHUB_BASE_REF), else the + local merge-base with the origin default branch. + +.PARAMETER Full + Report-only mode: scans every tracked file in scope at HEAD instead of + the diff, and never exits non-zero. + +.PARAMETER List + Show every violation. Implied by -Full. + +.EXAMPLE + Build/Agent/comment-hygiene.ps1 + Gate the current diff against the local merge-base with the default branch. + +.EXAMPLE + Build/Agent/comment-hygiene.ps1 -Full -List + Report every mechanical violation in the whole repo, without failing. +#> +[CmdletBinding()] +param( + [string] $BaseRef, + [switch] $Full, + [switch] $List +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +$repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '../..')).Path +Import-Module (Join-Path $PSScriptRoot 'CommentHygiene.psm1') -Force + +# Kept in sync with Get-CommentHygieneLanguage's recognized extensions. +$scopedGlobs = @( + '*.cs', '*.ps1', '*.psm1', + '*.cpp', '*.cxx', '*.cc', '*.c', '*.h', '*.hpp', '*.idl', + '*.csproj', '*.vcxproj', '*.vcproj', '*.props', '*.targets', '*.proj', '*.axaml', '*.xaml' +) + +function Test-ExcludedPath { + param([string] $Path) + return ($Path -match '\.g\.cs$') -or ($Path -match 'Designer\.cs$') +} + +function Test-Utf8Bom { + param([string] $Path) + $bytes = [System.IO.File]::ReadAllBytes($Path) + return ($bytes.Length -ge 3) -and ($bytes[0] -eq 0xEF) -and ($bytes[1] -eq 0xBB) -and ($bytes[2] -eq 0xBF) +} + +function ConvertTo-RepoPath { + param([string] $RelativePath) + return Join-Path $repoRoot ($RelativePath -replace '/', [IO.Path]::DirectorySeparatorChar) +} + +function Resolve-BaseRef { + param([string] $Explicit) + + if (-not [string]::IsNullOrWhiteSpace($Explicit)) { return $Explicit } + if (-not [string]::IsNullOrWhiteSpace($env:GITHUB_EVENT_PULL_REQUEST_BASE_SHA)) { return $env:GITHUB_EVENT_PULL_REQUEST_BASE_SHA } + if (-not [string]::IsNullOrWhiteSpace($env:GITHUB_BASE_REF)) { return "origin/$env:GITHUB_BASE_REF" } + + # git rev-parse against the local origin/HEAD ref, not `git remote show origin`: the + # latter contacts the remote (a real network round-trip) on every local gate run. + $originHead = git rev-parse --abbrev-ref origin/HEAD 2>$null + if ($LASTEXITCODE -eq 0 -and -not [string]::IsNullOrWhiteSpace($originHead)) { return $originHead.Trim() } + return 'origin/main' +} + +function Get-AddedLineFilter { + param([string] $Base) + + $diff = git diff --unified=0 "$Base...HEAD" -- $scopedGlobs 2>$null + if ($LASTEXITCODE -ne 0) { + throw "git diff against '$Base' failed. Is the base ref fetched? (CI needs fetch-depth: 0.)" + } + + $filter = @{} + $currentFile = $null + $currentLine = 0 + + foreach ($rawLine in $diff) { + if ($rawLine -match '^\+\+\+ b/(.+)$') { + $currentFile = ConvertTo-RepoPath $Matches[1] + continue + } + if ($rawLine -match '^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@') { + $currentLine = [int]$Matches[1] + continue + } + if ($null -eq $currentFile) { continue } + if ($rawLine.StartsWith('+') -and -not $rawLine.StartsWith('+++')) { + if (-not (Test-ExcludedPath $currentFile)) { + if (-not $filter.ContainsKey($currentFile)) { + $filter[$currentFile] = [System.Collections.Generic.HashSet[int]]::new() + } + [void]$filter[$currentFile].Add($currentLine) + } + $currentLine++ + } + } + + return $filter +} + +function Write-Violation { + param($Violation) + $relative = $Violation.File.Substring($repoRoot.Length + 1) + Write-Host (" {0}:{1} [{2}] {3}" -f $relative, $Violation.Line, $Violation.Category, $Violation.Text) +} + +if ($Full) { + $files = git ls-files $scopedGlobs | ForEach-Object { ConvertTo-RepoPath $_ } | Where-Object { -not (Test-ExcludedPath $_) } + $violations = Get-CommentHygieneViolations -Files $files + + Write-Host "comment-hygiene -Full: $($violations.Count) violation(s) across $($files.Count) file(s)" + foreach ($v in $violations) { Write-Violation $v } + exit 0 +} + +$base = Resolve-BaseRef -Explicit $BaseRef +Write-Host "comment-hygiene: scanning lines added since $base" + +$lineFilter = Get-AddedLineFilter -Base $base +if ($lineFilter.Count -eq 0) { + Write-Host 'comment-hygiene: no added lines in scope to check.' + exit 0 +} + +$violations = Get-CommentHygieneViolations -Files @($lineFilter.Keys) -LineFilter $lineFilter + +# CI can't commit a fix back, so only auto-fix where a human can review and commit it. +$isCI = ($env:GITHUB_ACTIONS -eq 'true') -or ($env:CI -eq 'true') + +$remainingViolations = New-Object System.Collections.ArrayList +$fixedFiles = @{} +$fixedCount = 0 +$wrappedCount = 0 + +$autoFixable = @('non-ascii-punctuation', 'comment-line-too-long') +$pending = New-Object System.Collections.ArrayList +foreach ($v in $violations) { + if ($isCI -or ($autoFixable -notcontains $v.Category)) { + [void]$remainingViolations.Add($v) + continue + } + [void]$pending.Add($v) +} + +$editorConfig = Get-CommentHygieneEditorConfig -RepoRoot $repoRoot + +# Per file, bottom-up: re-wrapping adds lines and shifts every line number below +# it. Within one line the punctuation fix runs first, since it changes the width +# that decides whether a wrap is still needed. +$sortOrder = @( + @{ Expression = 'Line'; Descending = $true }, + @{ Expression = { if ($_.Category -eq 'non-ascii-punctuation') { 0 } else { 1 } }; Descending = $false } +) + +foreach ($group in ($pending | Group-Object File)) { + $path = $group.Name + $language = Get-CommentHygieneLanguage -Path $path + $hadBom = Test-Utf8Bom -Path $path + $fileLines = New-Object 'System.Collections.Generic.List[string]' + $fileLines.AddRange([string[]][System.IO.File]::ReadAllLines($path, [System.Text.Encoding]::UTF8)) + $changed = $false + + foreach ($v in ($group.Group | Sort-Object $sortOrder)) { + $index = $v.Line - 1 + + if ($v.Category -eq 'non-ascii-punctuation') { + $fixedLine = Repair-CommentLine -Line $fileLines[$index] + if ($null -eq $fixedLine) { [void]$remainingViolations.Add($v); continue } + $fileLines[$index] = $fixedLine + $changed = $true + $fixedCount++ + continue + } + + $width = Get-CommentDisplayWidth -Line $fileLines[$index] -TabWidth $editorConfig.TabWidth + if ($width -le $editorConfig.MaxLineLength) { continue } + + $wrapped = Format-CommentLineWrap -Line $fileLines[$index] -Language $language ` + -MaxWidth $editorConfig.MaxLineLength -TabWidth $editorConfig.TabWidth + if ($null -eq $wrapped) { [void]$remainingViolations.Add($v); continue } + + $fileLines.RemoveAt($index) + $fileLines.InsertRange($index, [string[]]$wrapped) + $changed = $true + $wrappedCount++ + } + + if ($changed) { + Set-CommentHygieneFileContent -Path $path -Lines $fileLines.ToArray() -Utf8Bom $hadBom + $fixedFiles[$path] = $true + } +} + +$violations = $remainingViolations.ToArray() + +if ($fixedCount -gt 0 -or $wrappedCount -gt 0) { + Write-Host ("comment-hygiene: auto-fixed {0} punctuation and re-wrapped {1} over-long comment line(s) in {2} file(s) (review and include in your commit)." -f $fixedCount, $wrappedCount, $fixedFiles.Keys.Count) -ForegroundColor Yellow +} + +if ($violations.Count -eq 0) { + Write-Host 'comment-hygiene: clean.' + exit 0 +} + +Write-Host '' +Write-Host "comment-hygiene: $($violations.Count) violation(s) in added lines" -ForegroundColor Red +foreach ($v in $violations) { Write-Violation $v } +Write-Host '' +Write-Host 'Fix per .claude/skills/fieldworks-code-commenting/SKILL.md, or rewrite the comment.' -ForegroundColor Red +exit 1 diff --git a/Build/Agent/commit-messages.sh b/Build/Agent/commit-messages.sh deleted file mode 100644 index 1d1ba309c1..0000000000 --- a/Build/Agent/commit-messages.sh +++ /dev/null @@ -1,32 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# shellcheck source=lib_git.sh -source "${SCRIPT_DIR}/lib_git.sh" - -# Determine base ref for commit range -BASE_REF="" -if [[ -n "${GITHUB_BASE_REF:-}" ]]; then - BASE_REF="origin/${GITHUB_BASE_REF}" -else - BASE_REF="$(git_default_branch_ref)" -fi - -# Ensure gitlint is available -if ! command -v gitlint >/dev/null 2>&1; then - if command -v python3 >/dev/null 2>&1; then - python3 -m pip install --upgrade gitlint - else - pip install --upgrade gitlint - fi -fi - -echo "Running gitlint against range: ${BASE_REF}..HEAD" -# Run gitlint and tee output to check_results.log (used by CI summary/comment) -set +e -gitlint --ignore body-is-missing --commits "${BASE_REF}.." 2>&1 | tee check_results.log -exit_code=${PIPESTATUS[0]} -set -e - -exit ${exit_code} diff --git a/Build/Agent/fix-whitespace.sh b/Build/Agent/fix-whitespace.sh deleted file mode 100644 index 2b5c2c5259..0000000000 --- a/Build/Agent/fix-whitespace.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# shellcheck source=lib_git.sh -source "${SCRIPT_DIR}/lib_git.sh" - -# Determine base ref -if [[ -n "${GITHUB_BASE_REF:-}" ]]; then - base="origin/${GITHUB_BASE_REF}" -else - base="$(git_default_branch_ref)" -fi - -files=() -if [[ -f "check-results.log" ]]; then - # Extract unique file paths from check results - mapfile -t files < <(awk -F':' '/^[^:]+:[1-9][0-9]*:/ {print $1}' check-results.log | awk '!seen[$0]++') - [[ ${#files[@]} -gt 0 ]] && echo "Fixing whitespace for files listed in check-results.log" -fi - -if [[ ${#files[@]} -eq 0 ]]; then - echo "Fixing whitespace for files changed since ${base}..HEAD" - mapfile -t files < <(git diff --name-only "$base"..HEAD) -fi - -for f in "${files[@]}"; do - [[ -f "$f" ]] || continue - # Strip trailing spaces/tabs on each line and ensure exactly one trailing newline - # Using perl for robust in-place editing across platforms - perl -0777 -pe 's/[ \t]+$//mg; s/\s*\z/\n/s' -i "$f" || true - echo "Fixed whitespace: $f" -done - -echo "Whitespace fix completed. Review changes, commit, and rebase as needed." -exit 0 diff --git a/Build/Agent/lib_git.sh b/Build/Agent/lib_git.sh deleted file mode 100644 index 6933ee3c9e..0000000000 --- a/Build/Agent/lib_git.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/usr/bin/env bash -# Shared Git helpers for Build/Agent bash scripts -set -euo pipefail - -# Returns the default branch name configured for origin (e.g., main, develop) -git_default_branch_name() { - git remote show origin 2>/dev/null | awk -F': ' '/HEAD branch/ {print $2}' || true -} - -# Returns the full ref for the origin default branch (e.g., origin/main) -git_default_branch_ref() { - local name - name="$(git_default_branch_name)" - if [[ -n "$name" ]]; then - echo "origin/${name}" - else - echo "origin/develop" - fi -} diff --git a/Build/Agent/powershell-compat.ps1 b/Build/Agent/powershell-compat.ps1 new file mode 100644 index 0000000000..97e51f1423 --- /dev/null +++ b/Build/Agent/powershell-compat.ps1 @@ -0,0 +1,148 @@ +<# +.SYNOPSIS + Static PowerShell-version-compatibility gate for the comment-hygiene tooling. + +.DESCRIPTION + Two independent static layers, neither of which requires more than one + PowerShell engine to actually be installed: + + 1. A dependency-free regex scan for known gotchas where 5.1 and 7 both + parse the same text successfully but disagree on its meaning, so no + AST-based tool can see the difference: the backtick u{} escape + (5.1 silently drops it instead of resolving the code point) and the + utf8BOM/utf8NoBOM -Encoding values (5.1 does not recognize them at + all). This layer always runs and never needs installing anything. + + 2. PSScriptAnalyzer's PSUseCompatibleSyntax rule, which catches + structural grammar additions (ternary, null-coalescing, the + null-conditional operators, pipeline chain operators, and more) by + checking the parsed script against Microsoft's maintained per-version + grammar profiles -- it does not need 5.1 itself to be installed, + only its own module. This layer is best-effort: if the module is + missing and cannot be installed (offline, restricted network), this + script warns and skips it rather than failing the build over a + missing optional dependency. + + Neither layer is a substitute for actually running under both engines: + that is the only way to catch every possible semantic difference, and + it requires both engines to be present, which this script does not + assume. CI (windows-2022 runners ship both powershell.exe and pwsh) runs + CommentHygiene.Tests.ps1 under both as that real-runtime check; this + script is the cheaper static complement, runnable anywhere. + +.PARAMETER Full + Report every hit without failing. + +.EXAMPLE + Build/Agent/powershell-compat.ps1 +#> +[CmdletBinding()] +param( + [switch] $Full +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +Import-Module (Join-Path $PSScriptRoot 'CommentHygiene.psm1') -Force + +$repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '../..')).Path +$targetFiles = @( + 'Build/Agent/CommentHygiene.psm1', + 'Build/Agent/CommentHygiene.Tests.ps1', + 'Build/Agent/comment-hygiene.ps1', + 'Build/Agent/comment-hygiene-repair.ps1', + 'Build/Agent/comment-hygiene-blame.ps1', + 'Build/Agent/powershell-compat.ps1' +) | ForEach-Object { Join-Path $repoRoot $_ } | Where-Object { Test-Path -LiteralPath $_ } + +$violations = New-Object System.Collections.ArrayList + +# Layer 1: dependency-free regex scan for known parse-both-ways-differently gotchas. +$gotchaPatterns = [ordered]@{ + 'backtick-unicode-escape' = @{ + Pattern = '`[uU]\{[0-9a-fA-F]+\}' + Message = 'Backtick u{} escape is PowerShell 6+ only; 5.1 drops the backtick and keeps the literal text. Use [char] / [char]::ConvertFromUtf32 instead.' + } + 'ps7-only-encoding-value' = @{ + Pattern = '-Encoding\s+([''"]?)(utf8BOM|utf8NoBOM)\1\b' + Message = '-Encoding utf8BOM/utf8NoBOM is PowerShell 6+ only and throws a parameter-binding error under 5.1. Use System.Text.UTF8Encoding directly for explicit BOM control.' + } +} + +# Excludes itself: $gotchaPatterns must spell out each pattern's literal +# text, which would otherwise flag the definition line as an instance of it. +$regexScanFiles = $targetFiles | Where-Object { $_ -ne (Join-Path $repoRoot 'Build/Agent/powershell-compat.ps1') } + +foreach ($file in $regexScanFiles) { + $lines = @(Get-Content -LiteralPath $file -Encoding UTF8) + # Comment lines are excluded: this tooling's own doc comments describe the + # gotcha patterns in prose, which would otherwise self-match as a violation. + $classification = Get-CommentLineClassification -Lines $lines -Language 'PowerShell' + for ($i = 0; $i -lt $lines.Count; $i++) { + if ($null -ne $classification.Kinds[$i]) { continue } + foreach ($gotchaName in $gotchaPatterns.Keys) { + if ($lines[$i] -match $gotchaPatterns[$gotchaName].Pattern) { + [void]$violations.Add([PSCustomObject]@{ + File = $file.Substring($repoRoot.Length + 1) + Line = $i + 1 + Message = $gotchaPatterns[$gotchaName].Message + }) + } + } + } +} + +# Layer 2: PSScriptAnalyzer's PSUseCompatibleSyntax, best-effort. $Global: +# persists across a session, so a failed install costs one timeout, not one +# per build. Seeded first: StrictMode throws on unset. +if (-not (Test-Path Variable:Global:FwPowerShellCompatAnalyzerUnavailable)) { + $Global:FwPowerShellCompatAnalyzerUnavailable = $false +} + +if ((-not (Get-Module -ListAvailable -Name PSScriptAnalyzer)) -and -not $Global:FwPowerShellCompatAnalyzerUnavailable) { + try { + Write-Host 'powershell-compat: installing PSScriptAnalyzer (first run only)...' + Install-Module -Name PSScriptAnalyzer -Scope CurrentUser -Force -AllowClobber -ErrorAction Stop | Out-Null + } + catch { + Write-Host "powershell-compat: PSScriptAnalyzer unavailable and could not be installed ($($_.Exception.Message)); skipping the PSUseCompatibleSyntax layer for the rest of this session." -ForegroundColor Yellow + $Global:FwPowerShellCompatAnalyzerUnavailable = $true + } +} + +if (Get-Module -ListAvailable -Name PSScriptAnalyzer) { + Import-Module PSScriptAnalyzer -Force + $settings = @{ + IncludeRules = @('PSUseCompatibleSyntax') + Rules = @{ + PSUseCompatibleSyntax = @{ + Enable = $true + TargetVersions = @('5.1', '7.0') + } + } + } + foreach ($file in $targetFiles) { + $results = Invoke-ScriptAnalyzer -Path $file -Settings $settings + foreach ($r in $results) { + [void]$violations.Add([PSCustomObject]@{ + File = $file.Substring($repoRoot.Length + 1) + Line = $r.Line + Message = $r.Message + }) + } + } +} + +if ($violations.Count -eq 0) { + Write-Host 'powershell-compat: clean (5.1 and 7.0).' + exit 0 +} + +Write-Host "powershell-compat: $($violations.Count) syntax incompatibility(ies) found" -ForegroundColor Red +foreach ($v in $violations) { + Write-Host (" {0}:{1} {2}" -f $v.File, $v.Line, $v.Message) +} + +if ($Full) { exit 0 } +exit 1 diff --git a/Build/PackageRestore.targets b/Build/PackageRestore.targets index f2380cd00c..e58d0f0448 100644 --- a/Build/PackageRestore.targets +++ b/Build/PackageRestore.targets @@ -35,9 +35,9 @@ 4.500 - + 6.200 $(fwrt)/packages @@ -295,9 +295,8 @@ DownloadsDir="$(DownloadsDir)" Condition="'$(OS)'=='Windows_NT' AND !Exists('$(DownloadsDir)/TonePars64.exe')" /> - + - + /// The assembly element (must be the root assembly, not a file element). /// The CLSID string. diff --git a/Directory.Packages.props b/Directory.Packages.props index 97851b4a60..17911b2c02 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -100,10 +100,9 @@ --> - + @@ -173,9 +172,8 @@ --> - + @@ -187,30 +185,13 @@ - + - + @@ -220,9 +201,8 @@ - + diff --git a/Docs/CONTRIBUTING.md b/Docs/CONTRIBUTING.md index d00ba02912..497f072a34 100644 --- a/Docs/CONTRIBUTING.md +++ b/Docs/CONTRIBUTING.md @@ -55,18 +55,20 @@ FieldWorks builds can be significantly slowed by Windows Defender real-time scan This adds exclusions for build outputs, NuGet caches and development tools. Use `-DryRun` to preview changes without applying them. +If you also work in sibling repos in the same parent folder (e.g. PanGloss, motif, foma-rs), run `..\Setup-DefenderExclusions.ps1` instead (one level up) β€” it covers this repo plus the Rust toolchain (`.cargo`/`.rustup`) and CMake/Rust process exclusions the FieldWorks-only script doesn't need. + ### 2. Clone the Repository Clone the FieldWorks repository using HTTPS or SSH: **HTTPS:** -```bash +```powershell git clone https://github.com/sillsdev/FieldWorks.git cd FieldWorks ``` **SSH:** -```bash +```powershell git clone git@github.com:sillsdev/FieldWorks.git cd FieldWorks ``` @@ -75,7 +77,7 @@ cd FieldWorks If you're working on translations: -```bash +```powershell git clone https://github.com/sillsdev/FwLocalizations.git Localizations ``` @@ -158,7 +160,7 @@ See [VS Code Stability Profile](vscode-stability-profile.md) for current workspa It is helpful to increase the rename limits for Git to properly detect renames in large commits: -```bash +```powershell git config diff.renameLimit 10000 git config merge.renameLimit 10000 ``` @@ -180,7 +182,7 @@ We welcome any contribution! To get started: 1. **Fork** the FieldWorks repository on GitHub 2. **Clone** your fork locally 3. **Create a branch** for your changes: - ```bash + ```powershell git checkout -b feature/my-feature-name ``` 4. **Make your changes** and commit them with clear messages diff --git a/Docs/architecture/local-library-debugging.md b/Docs/architecture/local-library-debugging.md index 4baee6e7de..02b2fba90d 100644 --- a/Docs/architecture/local-library-debugging.md +++ b/Docs/architecture/local-library-debugging.md @@ -40,7 +40,7 @@ The script automatically registers this folder as a NuGet source in your user-le ### 3. Clone the library you need -```bash +```powershell git clone https://github.com/sillsdev/liblcm.git git clone https://github.com/sillsdev/libpalaso.git git clone https://github.com/sillsdev/chorus.git diff --git a/Docs/core-developer-setup.md b/Docs/core-developer-setup.md index 0161bd6aaa..9ddabb18d4 100644 --- a/Docs/core-developer-setup.md +++ b/Docs/core-developer-setup.md @@ -80,7 +80,7 @@ Contact the team lead to request permissions if needed. For streamlined pushing and pulling, set up an SSH key: 1. Generate an SSH key if you don't have one: - ```bash + ```powershell ssh-keygen -t ed25519 -C "your_email@example.com" ``` @@ -89,12 +89,12 @@ For streamlined pushing and pulling, set up an SSH key: - Paste your public key (`~/.ssh/id_ed25519.pub`) 3. Test the connection: - ```bash + ```powershell ssh -T git@github.com ``` 4. Update your remote to use SSH: - ```bash + ```powershell git remote set-url origin git@github.com:sillsdev/FieldWorks.git ``` @@ -102,14 +102,14 @@ For streamlined pushing and pulling, set up an SSH key: #### Set Identity -```bash +```powershell git config user.name "Your Name" git config user.email "your.email@example.com" ``` #### Increase Rename Limits -```bash +```powershell git config diff.renameLimit 10000 git config merge.renameLimit 10000 ``` @@ -118,7 +118,7 @@ git config merge.renameLimit 10000 Set up tracking for release branches you'll be working on: -```bash +```powershell # Fetch all branches git fetch --all @@ -176,7 +176,7 @@ If you use Claude Code, create the worktree with the repo task first, then launc #### Creating Feature Branches -```bash +```powershell # Create a new feature branch from the default branch git checkout release/9.3 git pull @@ -186,7 +186,7 @@ git checkout -b feature/my-feature-name #### Submitting Changes 1. Push your branch to origin: - ```bash + ```powershell git push -u origin feature/my-feature-name ``` @@ -209,7 +209,7 @@ If you are a release manager, additional setup may be required. Contact Jason Na ### Recommended Global Settings -```bash +```powershell # Use rebase by default when pulling git config --global pull.rebase true @@ -227,7 +227,7 @@ git config --global color.ui auto These are set in the FieldWorks repository: -```bash +```powershell # Increase rename detection limits git config diff.renameLimit 10000 git config merge.renameLimit 10000 @@ -245,7 +245,7 @@ If you get "Permission denied" when pushing: ### Branch Not Found If a branch you're looking for isn't available: -```bash +```powershell git fetch --all git branch -a # List all branches including remote ``` diff --git a/Docs/migration/adjust-the-layout.md b/Docs/migration/adjust-the-layout.md index 7dc15727f7..0be482beed 100644 --- a/Docs/migration/adjust-the-layout.md +++ b/Docs/migration/adjust-the-layout.md @@ -95,7 +95,7 @@ nothing is wasted, because option 2 needs the same startup project. view at its real client size with the same compact density the runtime applies. Not live, but seconds per cycle and no FLEx launch. -```bash +```powershell Src\Common\FwAvaloniaPreviewHost\bin\Debug\net48\FwAvaloniaPreviewHost.exe --module create-feature ``` @@ -138,7 +138,7 @@ which is why the host project references `FwAvaloniaDialogs`. Run the conversion's visual test and look at the PNGs in `Output/Snapshots/`: -```bash +```powershell ./test.ps1 -SkipNative -TestProject FwAvaloniaDialogsTests -TestFilter "FullyQualifiedName~OptionsDialogTests" ``` diff --git a/Docs/superpowers/specs/2026-07-31-retired-avalonia-lesson-cards-design.md b/Docs/superpowers/specs/2026-07-31-retired-avalonia-lesson-cards-design.md deleted file mode 100644 index 8f5b375332..0000000000 --- a/Docs/superpowers/specs/2026-07-31-retired-avalonia-lesson-cards-design.md +++ /dev/null @@ -1,75 +0,0 @@ -# Retired Avalonia lesson cards - -## Purpose - -Preserve verified knowledge from retired Avalonia follow-up work without preserving its code, treating its implementation choices as current requirements, or turning it into an autonomous roadmap. - -The immediate sources are PRs #965, #966, and #967, the removal and retirement work in PR #964, and the corresponding historical commits and OpenSpec material. - -## Repository artifacts - -Create a repository-wide lesson library under `Docs/lessons/`: - -- `README.md`: the repository-wide index of lesson areas. -- `TEMPLATE.md`: the shared structure and human-review fields for every lesson area. -- `avalonia-migration/README.md`: a capability-oriented index for this migration. -- `avalonia-migration/interlinear-analysis.md`: lessons from retired PR #965. -- `avalonia-migration/rule-formula-editors.md`: lessons from retired PR #966. -- `avalonia-migration/browse-table-activation.md`: lessons from retired PR #967 and the later removal of the dormant browse implementation. -- `avalonia-migration/migration-pivot.md`: cross-cutting lessons from PR #964's scope correction and retirement work. - -Each card records status, sources, human ownership, the question tested, observations, retired approaches, no more than five durable lessons, evidence required next time, its decision boundary, and explicit conclusions that must not be inferred. - -Cards must be capability-first and code-free. Historical type names may appear only in source citations when needed for archaeology. - -## Discovery - -Future humans and agents must be able to find the cards without knowing an old PR number or branch name. - -Discovery paths are: - -1. `Docs/lessons/README.md`, which lets future lesson areas sit alongside Avalonia migration rather than treating one migration as the permanent top-level category. -2. `Docs/lessons/avalonia-migration/README.md`, indexed by problem and capability vocabulary. -3. One general link from the root `AGENTS.md` to `Docs/lessons/README.md`; repository guidance must not encode topic-specific lesson routing. -4. Avalonia migration skills linking directly to the Avalonia lesson index, framed as historical constraints rather than implementation authority. -5. A concise link from PR #964's main description, with expanded context in its existing sticky provenance comment. - -## Git and pull-request workflow - -The lesson framework and migration-skill references land directly on `phase1-base` as part of PR #964. This keeps the new skills and the lessons they depend on in one review and one merge boundary. The temporary `document-retired-avalonia-lessons` branch is not published as a separate PR. - -After the lesson commits are pushed to PR #964: - -1. Update PR #964's main description with a short lesson-index reference. -2. Update its existing sticky provenance comment in place; do not create another provenance comment. -3. Close PRs #965, #966, and #967 as superseded, linking PR #964 and the relevant lesson cards. -4. Leave the three remote branches intact as temporary archaeological references. - -No product code, old tests, archived task lists, or branch commits are copied into this branch. - -## Jira boundary - -Lesson cards are durable institutional memory. Jira issues are execution records created only after a human approves a product outcome or a bounded discovery spike. - -The lesson-card PR does not create implementation stories for the three retired PRs. A future Jira issue may cite a lesson card for constraints and evidence, but the issue must independently state its desired outcome and must not treat the historical implementation as authorized. - -## Validation - -Before publishing: - -- Check every source commit and PR reference. -- Search the current tree to avoid claiming retired types or routes are present. -- Ensure each card distinguishes observation, rejected approach, durable lesson, and unresolved hypothesis. -- Ensure no card contains copied source code or an implementation checklist. -- Verify all repository links resolve. -- Inspect the final diff for scope and encoding damage. -- Use repository build/test scripts only if a changed validation surface requires them; documentation-only changes do not require a product build. - -## Non-goals - -- Do not restore, rebase, cherry-pick, or rewrite code from PRs #965-#967. -- Do not delete their branches. -- Do not endorse their UI architecture, class layout, activation scope, or completion claims. -- Do not recreate the removed OpenSpec changes or their task checklists. -- Do not create Jira implementation work without a separate human product decision. -- Do not use cards as a substitute for current-tree discovery, legacy characterization, domain-owner decisions, or real product validation. diff --git a/Docs/workflows/pull-request-workflow.md b/Docs/workflows/pull-request-workflow.md index 6664c11b3d..ccd2f5beef 100644 --- a/Docs/workflows/pull-request-workflow.md +++ b/Docs/workflows/pull-request-workflow.md @@ -32,7 +32,7 @@ Include the issue number when applicable (e.g., `bugfix/LT-12345-description`). ### Step 1: Create a Feature Branch -```bash +```powershell # Ensure you're on the latest default branch git checkout release/9.3 git pull origin release/9.3 @@ -50,7 +50,7 @@ git checkout -b feature/my-feature-name ### Step 3: Push and Create the PR -```bash +```powershell # Push your branch to GitHub git push -u origin feature/my-feature-name ``` @@ -105,7 +105,7 @@ Before a PR can be merged: If your branch has conflicts or is behind: -```bash +```powershell # Fetch latest changes git fetch origin @@ -140,7 +140,7 @@ Once all requirements are met: 2. Update any related issues 3. Verify the changes in the target branch -```bash +```powershell # Delete local branch git branch -d feature/my-feature-name @@ -175,7 +175,7 @@ For large features, consider breaking into smaller PRs: If a merged PR causes issues: -```bash +```powershell # Create a revert PR git checkout release/9.3 git pull diff --git a/Docs/workflows/release-process.md b/Docs/workflows/release-process.md index eadc4c8778..94d4056d68 100644 --- a/Docs/workflows/release-process.md +++ b/Docs/workflows/release-process.md @@ -10,7 +10,7 @@ This document describes the release workflow for FieldWorks. It covers creating When it's time to prepare a new release, create a new release branch named after the upcoming version. -```bash +```powershell # Create and checkout release branch from the develop branch git checkout develop git pull origin develop @@ -26,7 +26,7 @@ git push -u origin release/9.3 If someone else has already started a release branch: -```bash +```powershell # Fetch all branches git fetch --all @@ -38,7 +38,7 @@ git checkout release/9.3 #### Starting a Bugfix -```bash +```powershell # Ensure you're on the release branch git checkout release/9.3 git pull @@ -51,14 +51,14 @@ git checkout -b bugfix/LT-12345-fix-description 1. Make your changes and commit them 2. Push your branch: - ```bash + ```powershell git push -u origin bugfix/LT-12345-fix-description ``` 3. Create a Pull Request targeting the release branch #### After the PR is Merged -```bash +```powershell # Clean up local branch git checkout release/9.3 git pull @@ -71,7 +71,7 @@ When the release is ready: 1. Ensure all pending PRs for the release branch are merged 2. Create a release tag: - ```bash + ```powershell git checkout release/9.3 git pull git tag -a v9.3.0 -m "Release 9.3.0" @@ -86,7 +86,7 @@ Hotfixes are for critical bugs in released versions that can't wait for the next ### Creating a Hotfix Branch -```bash +```powershell # Create hotfix from the release tag git checkout v9.2.0 git checkout -b hotfix/9.2.1 @@ -99,7 +99,7 @@ git push -u origin hotfix/9.2.1 Same process as release branch bugfixes: -```bash +```powershell git checkout hotfix/9.2.1 git checkout -b bugfix/LT-12345-critical-fix # Make changes, commit, push, create PR @@ -109,7 +109,7 @@ git checkout -b bugfix/LT-12345-critical-fix 1. Complete all fixes on the hotfix branch 2. Create the hotfix release tag: - ```bash + ```powershell git checkout hotfix/9.2.1 git tag -a v9.2.1 -m "Hotfix release 9.2.1" git push origin v9.2.1 @@ -123,7 +123,7 @@ For maintaining older versions (e.g., fixing bugs in version 9.0 when 9.2 is cur ### Creating a Support Branch -```bash +```powershell # Create support branch from the old release tag git checkout v9.0.0 git checkout -b support/9.0 @@ -134,7 +134,7 @@ git push -u origin support/9.0 Process is similar to hotfixes, but hotfix branches are based on the support branch: -```bash +```powershell git checkout support/9.0 git checkout -b hotfix/9.0.1 ``` @@ -147,7 +147,7 @@ If you get merge failures when releasing: 2. Commit the resolution 3. Push and continue with the release -```bash +```powershell # After resolving conflicts git add . git commit -m "Resolve merge conflicts for release" diff --git a/FieldWorks.proj b/FieldWorks.proj index 3107caf53b..898340093b 100644 --- a/FieldWorks.proj +++ b/FieldWorks.proj @@ -20,11 +20,8 @@ - + private System.Globalization.CultureInfo m_cultureInfo; } -} \ No newline at end of file +} diff --git a/Src/CacheLight/MetaDataCache.cs b/Src/CacheLight/MetaDataCache.cs index 2915d8d6c1..3eae87e935 100644 --- a/Src/CacheLight/MetaDataCache.cs +++ b/Src/CacheLight/MetaDataCache.cs @@ -988,4 +988,4 @@ public MetaFieldRec() m_fieldXml = null; } } -} \ No newline at end of file +} diff --git a/Src/Common/Controls/DetailControls/DataTree.cs b/Src/Common/Controls/DetailControls/DataTree.cs index e7facb4100..fd9b85dd05 100644 --- a/Src/Common/Controls/DetailControls/DataTree.cs +++ b/Src/Common/Controls/DetailControls/DataTree.cs @@ -426,7 +426,7 @@ private void AdjustSliceSplitPosition(Slice otherSlice) protected override void OnSizeChanged(EventArgs e) { base.OnSizeChanged(e); - // Skip O(N) splitter adjustment during bulk slice construction β€” + // Skip O(N) splitter adjustment during bulk slice construction -- // HandleLayout1 will set correct widths + positions after construction. if (ConstructingSlices) return; @@ -3698,13 +3698,13 @@ private int FindFirstPotentiallyVisibleSlice(int clipTop) int sliceBottom = slice.Top + slice.Height; if (sliceBottom <= clipTop) { - // Slice ends at or before the clip top β€” entirely above viewport. + // Slice ends at or before the clip top -- entirely above viewport. result = mid + 1; lo = mid + 1; } else { - // Slice extends below clip top β€” could be visible. + // Slice extends below clip top -- could be visible. hi = mid - 1; } } @@ -4298,14 +4298,9 @@ protected override void OnPaint(PaintEventArgs e) protected override void WndProc(ref Message m) { base.WndProc(ref m); - // After any scroll input (scrollbar drag, mouse wheel, horizontal wheel), - // force the parent background to repaint so separator lines are redrawn at - // correct positions. Without this, Windows bitblts stale line pixels - // from the old scroll position and only repaints the newly-exposed strip. - // Invalidate(false) skips child invalidation β€” slice HWNDs repaint - // themselves β€” so only the gap areas between slices are redrawn. - // Update() forces synchronous processing so stale lines don't accumulate - // across multiple scroll events before the low-priority WM_PAINT fires. + // Without this, Windows bitblts stale separator-line pixels from the old scroll + // position; Invalidate(false)+Update() forces a synchronous repaint of just the + // inter-slice gaps before WM_PAINT coalesces. const int WM_VSCROLL = 0x0115; const int WM_HSCROLL = 0x0114; const int WM_MOUSEWHEEL = 0x020A; diff --git a/Src/Common/Controls/DetailControls/DetailControlsTests/DataTreeRenderTests.cs b/Src/Common/Controls/DetailControls/DetailControlsTests/DataTreeRenderTests.cs index 10fb9895f3..9e7b731318 100644 --- a/Src/Common/Controls/DetailControls/DetailControlsTests/DataTreeRenderTests.cs +++ b/Src/Common/Controls/DetailControls/DetailControlsTests/DataTreeRenderTests.cs @@ -114,8 +114,8 @@ private void CreateSimpleEntry() /// /// Creates a lex entry with triple-nested senses (depth 3, breadth 2). - /// 2 senses Γ— 2 subsenses Γ— 2 sub-sub-senses = 14 total senses (2+4+8). - /// This is the "slow" scenario β€” realistic deeply nested entry. + /// 2 senses x 2 subsenses x 2 sub-sub-senses = 14 total senses (2+4+8). + /// This is the "slow" scenario -- realistic deeply nested entry. /// private void CreateDeepEntry() { @@ -255,7 +255,7 @@ private void EnrichEntry(ILexEntry entry, string testName) /// /// Creates a minimal lex entry with a single sense and no optional fields. - /// Exercises the "collapsed" view β€” bare minimum rendering path. + /// Exercises the "collapsed" view -- bare minimum rendering path. /// private void CreateCollapsedEntry() { @@ -271,7 +271,7 @@ private void CreateCollapsedEntry() m_entry.CitationForm.VernacularDefaultWritingSystem = MakeRenderString( $"CitationForm - {testName}", Cache.DefaultVernWs); - // Single sense β€” minimal entry, no enrichment + // Single sense -- minimal entry, no enrichment var senseFactory = Cache.ServiceLocator.GetInstance(); var sense = senseFactory.Create(); m_entry.SensesOS.Add(sense); @@ -281,7 +281,7 @@ private void CreateCollapsedEntry() /// /// Creates a fully enriched lex entry with all available optional fields populated. /// 4 senses with all sense-level fields, plus full entry enrichment. - /// Exercises the "expanded" view β€” maximum slice count for fields we can safely render. + /// Exercises the "expanded" view -- maximum slice count for fields we can safely render. /// private void CreateExpandedEntry() { @@ -462,7 +462,7 @@ public async Task DataTreeRender_Simple() /// /// Verifies the full DataTree rendering for a triple-nested lex entry. - /// 2 senses Γ— 2 subsenses Γ— 2 sub-sub-senses = 14 total senses. + /// 2 senses x 2 subsenses x 2 sub-sub-senses = 14 total senses. /// This is the "slow" scenario for realistic deep nesting. /// [Test] @@ -661,7 +661,7 @@ public async Task DataTreeRender_Extreme() /// /// Verifies the DataTree rendering for a minimal entry with a single sense. - /// Exercises the bare minimum rendering path β€” collapsed view. + /// Exercises the bare minimum rendering path -- collapsed view. /// [Test] public async Task DataTreeRender_Collapsed() @@ -802,7 +802,7 @@ public void DataTreeTiming(int depth, int breadth, string label) /// /// Measures paint/capture time for the extreme scenario. - /// Exercises the full OnPaint β†’ HandlePaintLinesBetweenSlices pipeline + /// Exercises the full OnPaint -> HandlePaintLinesBetweenSlices pipeline /// via DrawToBitmap. This provides a baseline for paint optimizations /// (clip-rect culling, double-buffering). /// @@ -821,7 +821,7 @@ public void DataTreeTiming_PaintPerformance() Assert.That(warmup, Is.Not.Null, "Warm-up capture should succeed"); warmup.Dispose(); - // Timed capture: DrawToBitmap β†’ OnPaint β†’ HandlePaintLinesBetweenSlices + // Timed capture: DrawToBitmap -> OnPaint -> HandlePaintLinesBetweenSlices var sw = System.Diagnostics.Stopwatch.StartNew(); var bitmap = harness.CaptureCompositeBitmap(); sw.Stop(); @@ -886,7 +886,7 @@ public void DataTreeOpt_WidthStabilityAfterLayout() for (int i = 0; i < dt.Slices.Count; i++) initialWidths[i] = ((Slice)dt.Slices[i]).Width; - // Force a second paint/layout pass β€” widths should remain identical + // Force a second paint/layout pass -- widths should remain identical var bitmap = harness.CaptureCompositeBitmap(); Assert.That(bitmap, Is.Not.Null, "Second paint should succeed"); bitmap.Dispose(); @@ -1100,7 +1100,7 @@ public void DataTreeOpt_SequentialPaintsProduceIdenticalOutput() Assert.That(capture2.Height, Is.EqualTo(capture1.Height), "Bitmap heights should match"); - // Compare pixel-by-pixel β€” paint must be deterministic + // Compare pixel-by-pixel -- paint must be deterministic int mismatchCount = 0; for (int y = 0; y < capture1.Height; y++) { @@ -1220,7 +1220,8 @@ public void DataTreeOpt_FullLayoutAndPaintPathPositionsAgree() var dt = harness.DataTree; - // Record positions after full layout (set by OnLayout β†’ HandleLayout1(fFull=true)) + // Record positions after full layout (set by OnLayout -> + // HandleLayout1(fFull=true)) var fullLayoutPositions = new int[dt.Slices.Count]; var fullLayoutHeights = new int[dt.Slices.Count]; for (int i = 0; i < dt.Slices.Count; i++) @@ -1253,7 +1254,7 @@ public void DataTreeOpt_FullLayoutAndPaintPathPositionsAgree() /// /// Verifies that AutoScrollPosition does not drift across multiple paint passes. /// The paint path adjusts scroll position when slices above the viewport change - /// height (e.g., DummyObjectSlice β†’ real slice). After initial convergence, + /// height (e.g., DummyObjectSlice -> real slice). After initial convergence, /// scroll position must be stable. /// Failure mode: binary search skips the desiredScrollPosition adjustment for /// above-viewport slices, causing scroll jumps. @@ -1270,7 +1271,7 @@ public void DataTreeOpt_ScrollPositionStableAcrossPaints() var dt = harness.DataTree; - // Warm up β€” first paint triggers layout convergence + // Warm up -- first paint triggers layout convergence var warmup = harness.CaptureCompositeBitmap(); Assert.That(warmup, Is.Not.Null); warmup.Dispose(); @@ -1458,7 +1459,7 @@ public void DataTreeOpt_NoDummySlicesInViewportAfterPaint() /// Verifies that slice heights are stable after layout convergence. /// A binary search for the first visible slice depends on accumulated /// heights being deterministic: if heights change between paint calls - /// (e.g., because DummyObjectSliceβ†’real changes weren't finalized), + /// (e.g., because DummyObjectSlice->real changes weren't finalized), /// the binary search would compute wrong yTop offsets and skip or /// double-show slices. /// After the initial full-layout pass, heights should never change @@ -1475,7 +1476,7 @@ public void DataTreeOpt_SliceHeightsStableAfterConvergence() harness.PopulateSlices(1024, 800, false); Assert.That(harness.SliceCount, Is.GreaterThan(0), "Should have slices"); - // Force full convergence β€” first paint makes everything real + // Force full convergence -- first paint makes everything real var warmup = harness.CaptureCompositeBitmap(); Assert.That(warmup, Is.Not.Null); warmup.Dispose(); diff --git a/Src/Common/Controls/DetailControls/DetailControlsTests/DataTreeReshowTimingTests.cs b/Src/Common/Controls/DetailControls/DetailControlsTests/DataTreeReshowTimingTests.cs index e84d471c43..280e92b739 100644 --- a/Src/Common/Controls/DetailControls/DetailControlsTests/DataTreeReshowTimingTests.cs +++ b/Src/Common/Controls/DetailControls/DetailControlsTests/DataTreeReshowTimingTests.cs @@ -15,7 +15,8 @@ namespace SIL.FieldWorks.Common.Framework.DetailControls { /// - /// Measures the legacy DataTree's re-show cost β€” a second ShowObject on the live tree after + /// Measures the legacy DataTree's re-show cost -- a second ShowObject on the live tree + /// after /// a model edit, which exercises the slice-reuse (ObjSeqHashMap) refresh path RecordEditView drives /// on record navigation and refresh. Numbers accumulate into the same /// Output/RenderBenchmarks/datatree-timings.json artifact the entry-open baselines use. diff --git a/Src/Common/Controls/DetailControls/DetailControlsTests/MorphTypeAtomicLauncherTests.cs b/Src/Common/Controls/DetailControls/DetailControlsTests/MorphTypeAtomicLauncherTests.cs index 4c3f3a2629..f05dda1524 100644 --- a/Src/Common/Controls/DetailControls/DetailControlsTests/MorphTypeAtomicLauncherTests.cs +++ b/Src/Common/Controls/DetailControls/DetailControlsTests/MorphTypeAtomicLauncherTests.cs @@ -99,8 +99,8 @@ public override void TestTearDown() /// DoNotRefresh window. Callers (like SwapValues) must explicitly set /// RefreshListNeeded=true before releasing DoNotRefresh. /// - /// RED phase: comment out RefreshListNeeded=true β†’ test FAILS (stale slices). - /// GREEN phase: RefreshListNeeded=true present β†’ test PASSES. + /// Without that RefreshListNeeded=true call, the bibliography slice + /// stays stale after the DoNotRefresh window closes. /// [Test] public void DoNotRefresh_SlicesMustReflectChanges_AfterRelease_LT22414() @@ -128,7 +128,8 @@ public void DoNotRefresh_SlicesMustReflectChanges_AfterRelease_LT22414() m_dtree.DoNotRefresh = false; - // Assert: after refresh, bibliography slice should be gone (no data β†’ ifdata hides it) + // Assert: after refresh, bibliography slice should be gone (no data -> ifdata hides + // it) Assert.That(m_dtree.Controls.Count, Is.EqualTo(1), "LT-22414: After DoNotRefresh=false, slices should reflect data changes. " + "Bibliography has no data so ifdata should hide it. " + @@ -158,7 +159,7 @@ public void DoNotRefresh_WithoutRefreshListNeeded_DoesNotRefresh_LT22414_BugDemo // Intentionally NOT setting RefreshListNeeded (simulates buggy SwapValues) m_dtree.DoNotRefresh = false; - // Assert: slices are STALE β€” bibliography still visible despite no data + // Assert: slices are STALE -- bibliography still visible despite no data Assert.That(m_dtree.Controls.Count, Is.EqualTo(2), "Without RefreshListNeeded, DoNotRefresh=false does not trigger refresh; " + "slices remain stale (bibliography still visible despite no data)."); diff --git a/Src/Common/Controls/DetailControls/ObjSeqHashMap.cs b/Src/Common/Controls/DetailControls/ObjSeqHashMap.cs index b878bfddd7..22620ac6dd 100644 --- a/Src/Common/Controls/DetailControls/ObjSeqHashMap.cs +++ b/Src/Common/Controls/DetailControls/ObjSeqHashMap.cs @@ -188,4 +188,4 @@ bool IEqualityComparer.Equals(object xArg, object yArg) return true; } } -} \ No newline at end of file +} diff --git a/Src/Common/Controls/DetailControls/Slice.cs b/Src/Common/Controls/DetailControls/Slice.cs index 79bdb223e3..1a82e89791 100644 --- a/Src/Common/Controls/DetailControls/Slice.cs +++ b/Src/Common/Controls/DetailControls/Slice.cs @@ -91,7 +91,8 @@ public class Slice : UserControl, IxCoreColleague protected bool m_widthHasBeenSetByDataTree = false; protected IPersistenceProvider m_persistenceProvider; - // Cached XML configuration attributes β€” parsed once from ConfigurationNode on first access. + // Cached XML configuration attributes -- parsed once from ConfigurationNode on first + // access. // Invalidated when ConfigurationNode is re-set (rare). private bool? m_cachedIsHeader; private bool? m_cachedSkipSpacerLine; diff --git a/Src/Common/Framework/MainWindowDelegate.cs b/Src/Common/Framework/MainWindowDelegate.cs index 0c25dfe186..22ac64f4b3 100644 --- a/Src/Common/Framework/MainWindowDelegate.cs +++ b/Src/Common/Framework/MainWindowDelegate.cs @@ -970,4 +970,4 @@ private static string MakeLauncherPath(string directory, string projectName, return Path.Combine(directory, projectName + tail + pathExtension); } } -} \ No newline at end of file +} diff --git a/Src/Common/FwAvalonia/AvaloniaDialogHost.cs b/Src/Common/FwAvalonia/AvaloniaDialogHost.cs index 18bf818f66..901bfd96fb 100644 --- a/Src/Common/FwAvalonia/AvaloniaDialogHost.cs +++ b/Src/Common/FwAvalonia/AvaloniaDialogHost.cs @@ -13,7 +13,8 @@ namespace SIL.FieldWorks.Common.FwAvalonia { /// /// Reusable host that shows an Avalonia dialog body (a UserControl) inside a WinForms-owned - /// modal during coexistence β€” the turn-key piece for the MVVM dialog stack. Because + /// modal during coexistence -- the turn-key piece for the MVVM dialog + /// stack. Because /// Avalonia modal windows are not supported while WinForms owns the message loop, /// the dialog body is hosted in a WinForms modal window owned by the caller's form; the view-model /// closes it by raising (no windowing in the VM). @@ -26,12 +27,16 @@ public static class AvaloniaDialogHost /// Shows modally over . Returns the accepted /// result (true = OK, false = Cancel), or null if the window was closed without an OK/Cancel. /// - /// The optional parameters extend the fixed-size default WITHOUT changing it for existing callers: - /// * β€” when true the modal gets a sizable border and a minimum size - /// (defaulting to the initial / unless - /// / are supplied). Default false keeps the + /// The optional parameters extend the fixed-size default WITHOUT changing it for existing + /// callers: + /// * -- when true the modal gets a sizable border and a + /// minimum size + /// (defaulting to the initial / unless + /// / are supplied). Default false + /// keeps the /// legacy behavior. - /// * / β€” an optional + /// * / -- an + /// optional /// size-persistence hook (mirrors the label-column-width persistence pattern: the caller owns the /// remembered value, keyed by dialog identity, so a resized dialog reopens at its last size). The /// get-hook (when it returns a value) seeds the initial client size in place of @@ -82,7 +87,8 @@ public static class AvaloniaDialogHost ShowInTaskbar = false }) { - // Border / min-size / initial (possibly remembered) size β€” extracted so it is unit-testable + // Border / min-size / initial (possibly remembered) size -- extracted so it + // is unit-testable // without spinning a real modal window. ApplySizing(form, width, height, resizable, minWidth, minHeight, getRememberedSize); @@ -145,7 +151,8 @@ public static class AvaloniaDialogHost /// /// Picks the owner to show a modal dialog over: (the form truly - /// topmost/focused right now β€” ) when there is one, else the caller- + /// topmost/focused right now -- ) when there is one, else + /// the caller- /// supplied . Factored out of so the nested-modal /// owner-chain decision is unit-testable without spinning a real modal window; see the call site for /// why a stale breaks pointer input on a dialog opened from another dialog. @@ -183,12 +190,15 @@ public static void DisposeDialogResources(AvControl dialogBody, IDialogViewModel } /// - /// Applies the border style, minimum size and initial (possibly remembered) client size to the hosting + /// Applies the border style, minimum size and initial (possibly remembered) client size + /// to the hosting /// modal . Factored out of so the sizing/persistence /// contract is unit-testable without spinning a real modal window: - /// * false β†’ and no min-size + /// * false -> and + /// no min-size /// (the legacy default; //get-hook ignored); - /// * true β†’ with a min client size + /// * true -> with a + /// min client size /// (/, defaulting to the initial /// /), and the get-hook (when it returns a value) /// seeds the initial client size in place of /. @@ -229,7 +239,8 @@ public static void ApplySizing( var initialH = Math.Max(initial.Height, minH); form.ClientSize = new System.Drawing.Size(initialW, initialH); - // Derive the window-frame delta from the realized form so MinimumSize (an outer size) corresponds to the + // Derive the window-frame delta from the realized form so MinimumSize (an outer size) + // corresponds to the // requested CLIENT minimum. Falls back to the client minimum if the handle is not yet realized. var chromeW = form.Width - form.ClientSize.Width; var chromeH = form.Height - form.ClientSize.Height; @@ -239,7 +250,8 @@ public static void ApplySizing( /// /// Focuses the first keyboard-focusable INPUT inside so a dialog opens /// with the caret in its first field (legacy WinForms parity). Buttons (OK/Cancel/Help) are - /// never the initial focus β€” initial focus belongs to an input, and Enter/Escape already activate the + /// never the initial focus -- initial focus belongs to an input, and Enter/Escape already + /// activate the /// default/cancel buttons. Returns the control it focused, or null if none qualifies. Factored out /// (like / ) so the selection contract is unit-testable /// headlessly without spinning a real WinForms-hosted modal window; invokes it @@ -268,7 +280,8 @@ int EffectiveTabIndex(AvControl c) return max; } - // The first focusable INPUT in tab order β€” never a command button. If a picker-driven dialog + // The first focusable INPUT in tab order -- never a command button. If a + // picker-driven dialog // exposes no focusable field (the owned FwOptionChooser is deliberately Focusable=false and // handles keys directly), focus nothing rather than landing on OK, where Enter would accept the // dialog. So this is a no-op for picker dialogs (no regression) and focuses the first text field diff --git a/Src/Common/FwAvalonia/CompactDialogStyles.cs b/Src/Common/FwAvalonia/CompactDialogStyles.cs index 597f732fea..fbe10479c1 100644 --- a/Src/Common/FwAvalonia/CompactDialogStyles.cs +++ b/Src/Common/FwAvalonia/CompactDialogStyles.cs @@ -12,10 +12,13 @@ namespace SIL.FieldWorks.Common.FwAvalonia { /// - /// Compact density for Avalonia dialogs β€” the design baseline so migrated dialogs match the legacy + /// Compact density for Avalonia dialogs -- the design baseline so migrated dialogs match the + /// legacy /// WinForms dialog density (small font, tight padding, no Fluent min-height floors) rather than the - /// roomy Fluent defaults. Applied once by to every hosted dialog - /// body, so EVERY dialog shown through the host inherits it automatically β€” new dialogs need no + /// roomy Fluent defaults. Applied once by to every hosted + /// dialog + /// body, so EVERY dialog shown through the host inherits it automatically -- new dialogs need + /// no /// per-dialog density work. Scoped to the dialog's control subtree (added to its Styles), so /// it never affects the detail/table views, which own their own density (). /// @@ -67,10 +70,12 @@ private static IEnumerable Build() // Tabs size to content (drop the Fluent min-height floor) for compact rows. yield return Templated(new Thickness(8, 3), 0); - // NOTE: the deterministic CheckBox style (FwCheckBoxStyle) is NOT added here. It is applied once, - // to every dialog body, by DialogThemeBootstrap.Apply (called from each dialog ctor in BOTH the + // NOTE: the deterministic CheckBox style (FwCheckBoxStyle) is NOT added here. It is + // applied once, + // to every dialog body, by DialogThemeBootstrap.Apply (called from each dialog ctor + // in BOTH the // runtime host and the headless dialog tests), so it reaches the headless path that never runs this - // runtime chokepoint β€” and stays a single application rather than a double one. + // runtime chokepoint -- and stays a single application rather than a double one. yield return new Style(s => s.OfType()) { diff --git a/Src/Common/FwAvalonia/Detail/DataTree.cs b/Src/Common/FwAvalonia/Detail/DataTree.cs index 2ab8b3b2d9..0c5d555813 100644 --- a/Src/Common/FwAvalonia/Detail/DataTree.cs +++ b/Src/Common/FwAvalonia/Detail/DataTree.cs @@ -24,7 +24,8 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail /// /// Editing: when an is supplied, /// field editors stage writes through it (which opens the fenced LCModel session on the first - /// edit) and the session auto-commits on focus loss β€” the legacy save-as-you-go behavior, one + /// edit) and the session auto-commits on focus loss -- the legacy save-as-you-go behavior, + /// one /// undo step per field, no Save/Cancel buttons. Validation failures show inline and block the /// commit; Escape rolls the session back. Without a context the view is read-only display. /// @@ -33,9 +34,9 @@ public sealed class DataTree : UserControl private readonly IDetailEditContext _editContext; private readonly Action _writingSystemFocused; private readonly List> _rowControls = new List>(); - // Collapsible section toggle buttons, keyed by field stable id β€” captured at build time so - // WireCollapsibleHeaders finds the toggle directly (the header is now wrapped in the field-menu - // gutter, and the kebab is also a Button, so a tree search would be ambiguous). + // Collapsible section toggles, keyed by field stable id, captured at build + // time: WireCollapsibleHeaders finds them since the header now wraps in + // the field-menu gutter, where the kebab is also a Button. private readonly Dictionary _collapsibleToggles = new Dictionary(); private readonly Action _labelColumnWidthChanged; private TextBlock _validationBlock; @@ -50,10 +51,10 @@ public sealed class DataTree : UserControl /// Optional expansion-state hooks (11.8): supplies the /// persisted state per header stable id (overriding the layout's initial state) and /// records toggles, so collapse state survives record - /// switches/re-shows β€” the legacy PropertyTable expansion persistence. + /// switches/re-shows -- the legacy PropertyTable expansion persistence. /// / persist /// the splitter position the same way (11.15): the host owns the remembered width so it - /// survives re-shows WITHOUT a process-global field β€” each host/window keeps its own. + /// survives re-shows WITHOUT a process-global field -- each host/window keeps its own. /// public DataTree(DetailModel model, IDetailEditContext editContext = null, Action writingSystemFocused = null, @@ -80,9 +81,9 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, AutomationProperties.SetAutomationId(this, "DataTree"); AutomationProperties.SetName(this, FwAvaloniaStrings.DetailAreaName); - // WinForms-density font baseline for the detail view, applied to this view's own control - // subtree so it renders in both the runtime host and the headless tests. The view stays FLAT with - // subtle field separators (FwAvaloniaDensity) β€” this only drops the Fluent ~14px default font. + // WinForms-density font baseline for the detail view, applied to this view's + // own subtree so runtime and headless hosts render it the same. Stays FLAT + // (FwAvaloniaDensity); only drops the Fluent ~14px font. FwSurfaceStyles.Apply(this); // Viewing parity (11.15): a draggable splitter divides the label and value columns like @@ -141,8 +142,9 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, // the same StackPanel, whether or not an edit context is present. A bare grid placed straight in // the ScrollViewer is arranged against the full viewport extent, while a grid inside a StackPanel // is arranged against its own desired height; those two arrange contexts round the grid's Auto - // content rows to whole-pixel heights 1px differently, so wrapping only in the editable state - // would shift every row by 1px on the edit toggle β€” a visible rhythm mismatch. + // content rows to whole-pixel heights 1px differently, so wrapping only in the + // editable state + // would shift every row by 1px on the edit toggle -- a visible rhythm mismatch. // Wrapping identically in both states keeps the rows pixel-for-pixel stable across the toggle; the // validation footer is the only edit-only child added. var panel = new StackPanel(); @@ -161,12 +163,14 @@ public DataTree(DetailModel model, IDetailEditContext editContext = null, Content = scroller; // Screen-local command shortcuts: - // Enter commits (validation-gated), Escape cancels β€” handled at the view so they work + // Enter commits (validation-gated), Escape cancels -- handled at the view so they + // work // from any field editor. AddHandler(Avalonia.Input.InputElement.KeyDownEvent, OnViewKeyDown, Avalonia.Interactivity.RoutingStrategies.Bubble); - // Auto-save (14.4): legacy slices commit as the user moves on β€” any editor losing focus + // Auto-save (14.4): legacy slices commit as the user moves on -- any editor losing + // focus // while a session is open commits it (validation-gated; one undo step per field). AddHandler(Avalonia.Input.InputElement.LostFocusEvent, (s, e) => { @@ -201,7 +205,8 @@ private void OnViewKeyDown(object sender, Avalonia.Input.KeyEventArgs e) /// public event EventHandler EditCompleted; - // 14.4: no Save/Cancel buttons β€” the legacy view saves as you go. The footer carries only the + // 14.4: no Save/Cancel buttons -- the legacy view saves as you go. The footer carries + // only the // inline validation messages (a failed autosave is never silent). private Control CreateEditFooter() { @@ -283,13 +288,15 @@ private void AddField(Grid grid, int row, DetailField field) AutomationProperties.SetAutomationId(header, automationId); AutomationProperties.SetName(header, field.Label ?? string.Empty); - // 13.3/13.5: the section menu/hotlinks open from the hover "β‹" field-menu button (which + // 13.3/13.5: the section menu/hotlinks open from the hover "..." field-menu + // button (which // replaced right-click), in a thin gutter to the left of the header. var headerCell = WrapWithFieldMenu(header, field, automationId, out var headerKebab); // Discoverability parity (legacy SummaryCommandControl): a section header with hotlinks // shows its commands as an ALWAYS-VISIBLE inline command-link strip directly beneath the - // header β€” the kebab alone is a hover-gated discoverability regression. The strip raises + // header -- the kebab alone is a hover-gated discoverability regression. The + // strip raises // the SAME hotlinks request the kebab does (DetailMenuKind.Hotlinks), so it dispatches // through the existing host bridge identically. var hotlinkStrip = CreateHotlinkStrip(field, automationId, indent); @@ -342,14 +349,16 @@ private void AddField(Grid grid, int row, DetailField field) TextAlignment = TextAlignment.Left, // legacy labels are left-aligned in the label panel Foreground = FwAvaloniaDensity.LabelBrush, FontSize = FwAvaloniaDensity.LabelFontSize, - // 14.2: a null background only hit-tests the glyphs; the whole label area must take + // 14.2: a null background only hit-tests the glyphs; the whole label area must + // take // the right-click for the slice menu. Background = Brushes.Transparent }; AutomationProperties.SetAutomationId(labelBlock, automationId + ".Label"); AutomationProperties.SetName(labelBlock, field.Label ?? field.Field ?? string.Empty); ToolTip.SetTip(labelBlock, field.Label ?? field.Field); // 11.17: legacy label tooltips - // 13.3: the field's slice menu opens from the hover "β‹" button in the left gutter (which + // 13.3: the field's slice menu opens from the hover "..." button in the left gutter + // (which // replaced right-click on the label). var labelCell = WrapWithFieldMenu(labelBlock, field, automationId, out var labelKebab); Grid.SetRow(labelCell, row * 2); @@ -364,9 +373,9 @@ private void AddField(Grid grid, int row, DetailField field) grid.Children.Add(editor); _rowControls[row].Add(editor); - // Hover-reveal affordances: the WHOLE row (label cell + editor) is the hover/focus surface for its - // secondary affordances β€” the field-options "β‹" and any editor affordances (chooser gear, - // vector bars/launcher). Both attach against the same sources so they reveal together. + // Hover-reveal: the WHOLE row (label cell + editor) is the hover/focus + // surface for the field-options "..." and any editor affordance (chooser + // gear, vector bars/launcher); both reveal together. var hoverSources = new Control[] { labelCell, editor }; if (labelKebab != null) HoverReveal.Attach(hoverSources, new[] { labelKebab }); @@ -374,18 +383,14 @@ private void AddField(Grid grid, int row, DetailField field) HoverReveal.Attach(hoverSources, provider.HoverAffordances); } - // The width of the left gutter that holds the per-row field-options "β‹" button. Reserved on + // The width of the left gutter that holds the per-row field-options "..." button. + // Reserved on // every row (when a host bridge is present) so labels align whether or not a row has a menu. private const double FieldMenuGutterWidth = 18; - // Section 13: each field/header row surfaces its legacy slice menu (or the section's hotlinks when - // only those exist) through the host bridge β€” the same menu ids legacy DTMenuHandler resolves from - // the layout. The affordance is a hover/keyboard-focus-revealed "β‹" button in a thin left gutter - // (it REPLACED right-click): clicking or pressing Enter/Space on it raises the SAME DetailMenuRequest - // as right-click, anchored at the icon. Returns wrapped with that - // gutter for the row, and reports the revealed kebab (or null) so the caller folds it into the row's - // hover group. With no host bridge the content is returned unwrapped, so non-product views - // (previews/tests with no menu callback) are unchanged. + // The hover/keyboard-revealed "..." kebab replaces right-click, raising the + // same DetailMenuRequest; with no host bridge the content returns unwrapped + // so preview/test hosts stay unchanged. private Control WrapWithFieldMenu(Control inner, DetailField field, string automationId, out Control kebab) { @@ -416,7 +421,8 @@ private Control WrapWithFieldMenu(Control inner, DetailField field, string autom // affordance is fully keyboard-operable once Tab focus reveals it. button.Click += (s, e) => { - // Anchor the menu to the icon (drop from its bottom-left) β€” the screen-coordinate + // Anchor the menu to the icon (drop from its bottom-left) -- the + // screen-coordinate // contract the host's DetailMenuRequest handler positions the xCore menu by. var screen = button.PointToScreen(new Point(0, button.Bounds.Height)); _menuRequested(new DetailMenuRequest(field, kind, screen.X, screen.Y)); @@ -436,15 +442,9 @@ private Control WrapWithFieldMenu(Control inner, DetailField field, string autom private static readonly IBrush HotlinkBrush = new SolidColorBrush(Color.FromRgb(0x00, 0x66, 0xCC)); - // Discoverability parity: the always-visible inline hotlinks command strip beneath a section - // header (legacy SummaryCommandControl). The host bridge resolves the hotlinks MENU id at click - // time and exposes no per-command labels to this layer, so we render a SINGLE always-visible flat - // command link (not per-command links) that raises the SAME DetailMenuRequest(kind=Hotlinks) the - // kebab raises β€” it dispatches through the existing host bridge identically, and the host's - // hotlinks handler then surfaces the individual commands. Returns null when the header has no - // hotlinks or no host bridge is wired (previews/tests with no menu callback), so those hosts - // are unchanged. The strip is NOT hover-gated β€” it stays fully visible and clickable at rest, - // which is the whole point versus the kebab. + // Renders a single always-visible flat command link (not per-command) + // because the host bridge exposes no per-command labels, and stays + // un-hover-gated -- unlike the kebab -- since visibility is the point. private Control CreateHotlinkStrip(DetailField field, string automationId, Thickness indent) { if (_menuRequested == null || string.IsNullOrEmpty(field.HotlinksId)) @@ -479,7 +479,7 @@ private Control CreateHotlinkStrip(DetailField field, string automationId, Thick } // Viewing parity (11.x): a collapsible header owns every following row with greater indent, - // up to the next field at its own indent or shallower β€” collapsing hides them (nested + // up to the next field at its own indent or shallower -- collapsing hides them (nested // sections collapse with their parent), expanding restores them, and the layout's expansion // attribute supplies the initial state. // @@ -563,8 +563,9 @@ void RecomputeVisibility() RecomputeVisibility(); } - // Bookkeeping for one collapsible header: its toggle button, ownership range over _rowControls, - // and current expanded state. Used to recompute whole-view visibility (nested-collapse fidelity). + // Bookkeeping for one collapsible header: its toggle button, ownership range + // over _rowControls, and current expanded state, which together recompute + // whole-view visibility (nested-collapse fidelity). private sealed class CollapsibleHeader { public Button Button; @@ -575,7 +576,7 @@ private sealed class CollapsibleHeader public bool Expanded; } - // The fieldβ†’control dispatch is shared with the browse in-cell editor through + // The field->control dispatch is shared with the browse in-cell editor through // SliceFactory. The detail pane passes its full callback set (per-WS keyboard, slice // menu, link, clipboard) and routes reference-vector gesture completion to its validation-gated // OnSave (the autosave). New DetailFieldKinds are added once, in the factory. diff --git a/Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs b/Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs index f55020c3a4..751889f68d 100644 --- a/Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs +++ b/Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs @@ -16,7 +16,8 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail /// this, tabbing out of a field (which auto-commits, 14.4) would tear down the editor the user /// just moved into and dump focus on the floor. Capture reads the focused editor's stable /// automation id (and caret) from the outgoing view; restore finds the same id in the incoming - /// view and gives it focus β€” automation ids are stable per field/writing system by design, so + /// view and gives it focus -- automation ids are stable per field/writing system by design, + /// so /// they are the right cross-rebuild identity. /// public static class DetailFocusMemory @@ -82,7 +83,7 @@ public static bool TryRestoreScroll(Control root, Memento memento) /// /// Focuses the control with the memento's automation id inside - /// (which must already be attached to a TopLevel). Returns false when no match exists β€” + /// (which must already be attached to a TopLevel). Returns false when no match exists -- /// e.g. the field disappeared in the re-show, or the memento had scroll-only state. /// public static bool TryRestoreFocus(Control root, Memento memento) @@ -90,7 +91,8 @@ public static bool TryRestoreFocus(Control root, Memento memento) if (root == null || string.IsNullOrEmpty(memento?.AutomationId)) return false; - // First pass: the exact stable id (the common case β€” the same field survived the re-show). + // First pass: the exact stable id (the common case -- the same field survived the + // re-show). foreach (var visual in root.GetVisualDescendants()) { if (!(visual is Control control) @@ -104,7 +106,8 @@ public static bool TryRestoreFocus(Control root, Memento memento) } // Post-ghost-commit continuity (legacy RestoreSelection): when a ghost add-prompt commits, - // the host recomposes and the new REAL editor's id differs from the "/ghost" id β€” the ghost + // the host recomposes and the new REAL editor's id differs from the "/ghost" id -- + // the ghost // id carries the OWNER's hvo and the "/ghost" marker, the successor carries the newly created // object's hvo and no marker. So the exact match above misses and focus would land on the // floor. When the captured id is a ghost id, fall back to its successor matcher so focus @@ -135,7 +138,8 @@ private static void FocusMatch(Control control, Memento memento) // Maps a "/ghost" editor automation id to a predicate that recognizes the real successor editor // the ghost commit produced. The ghost id has the shape "{node}@{ownerHvo}/ghost.{wsKey}" (the - // owner hvo because the object did not exist yet); the successor has "{node}@{newHvo}.{wsKey}" β€” + // owner hvo because the object did not exist yet); the successor has + // "{node}@{newHvo}.{wsKey}" -- // same node-stable prefix and same writing-system suffix, only the owned object's hvo (and the // "/ghost" marker) change. We therefore match on the prefix up to and including "@" plus the WS // suffix after "/ghost", tolerating the hvo difference. Returns null when the id is not a ghost id diff --git a/Src/Common/FwAvalonia/Detail/DetailMenuFlyout.cs b/Src/Common/FwAvalonia/Detail/DetailMenuFlyout.cs index 34d5bf5cea..f768c94453 100644 --- a/Src/Common/FwAvalonia/Detail/DetailMenuFlyout.cs +++ b/Src/Common/FwAvalonia/Detail/DetailMenuFlyout.cs @@ -10,7 +10,7 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail { /// /// Framework-neutral context-menu item (15.1): what the host resolved from its menu system - /// (for FieldWorks, the xCore ChoiceGroup β€” labels, enablement, checkmarks, submenus, and an + /// (for FieldWorks, the xCore ChoiceGroup -- labels, enablement, checkmarks, submenus, and an /// execute action that dispatches through the mediator). FwAvalonia renders these natively; /// it knows nothing about xCore, preserving the engine-isolation boundary. /// @@ -45,7 +45,8 @@ private DetailMenuItem() /// /// Renders host-built trees as a native Avalonia - /// (15.1) β€” the same items, enablement, checkmarks, and submenus the + /// (15.1) -- the same items, enablement, checkmarks, and submenus + /// the /// legacy WinForms adapter menu shows, rendered with native Avalonia controls. Density: every item carries the /// explicit compact padding/height of the legacy WinForms menus /// (/, diff --git a/Src/Common/FwAvalonia/Detail/DetailModel.cs b/Src/Common/FwAvalonia/Detail/DetailModel.cs index 1343ab96cc..7dadbcc489 100644 --- a/Src/Common/FwAvalonia/Detail/DetailModel.cs +++ b/Src/Common/FwAvalonia/Detail/DetailModel.cs @@ -34,7 +34,8 @@ public enum DetailFieldKind /// /// An editable reference vector: current items plus the possibility list's options /// (hierarchy on ), edited through - /// / β€” + /// / -- /// the legacy possibility-vector slice with its trailing type-ahead add slot. /// ReferenceVector, @@ -49,7 +50,7 @@ public enum DetailFieldKind Custom, /// - /// An editable multi-paragraph structured-text (StText) field β€” the legacy + /// An editable multi-paragraph structured-text (StText) field -- the legacy /// StTextSlice's RootSite rich editor. The row carries an ordered /// list (each a run-aware /// with a per-paragraph named style); the owned @@ -61,7 +62,7 @@ public enum DetailFieldKind StructuredText, /// - /// A literal / "lit" slice (legacy MessageSlice) β€” static label text rendered + /// A literal / "lit" slice (legacy MessageSlice) -- static label text rendered /// read-only in the value column (the label/message text IS the content). Carries no editable /// value and no setter. /// @@ -72,7 +73,8 @@ public enum DetailFieldKind /// The kind of an embedded object (ORC) a run carries, classified LCModel-free from the /// FIRST character of (the value the xWorks adapter projects /// from the TsString's ktptObjData). The numeric tags mirror - /// SIL.LCModel.Core.KernelInterfaces.FwObjDataTypes β€” the view layer is LCModel-free, so it + /// SIL.LCModel.Core.KernelInterfaces.FwObjDataTypes -- the view layer is LCModel-free, + /// so it /// reads the opaque ObjectData string the adapter produced rather than the enum itself. /// public enum DetailOrcKind @@ -133,7 +135,8 @@ public DetailTextRun(string text, string writingSystemTag = null, string namedSt internal const char ObjDataFootnoteOwn = (char)5; internal const char ObjDataFootnoteName = (char)3; - /// Whether this run carries an embedded object (ORC) β€” any non-empty ObjectData. + /// Whether this run carries an embedded object (ORC) -- any non-empty + /// ObjectData. public bool IsOrc => !string.IsNullOrEmpty(ObjectData); /// @@ -181,13 +184,9 @@ public DetailRichTextValue(string plainText, IReadOnlyList runs, RichXml = richXml; RequiresRichEditor = requiresRichEditor; LossyProperties = lossyProperties; - // An embedded object (ORC) does not force the value read-only β€” a link ORC is fully - // editable (insert/edit/delete) and ANY ORC run is deletable, so the run-replay path rebuilds - // the value with its ObjectData preserved. A value is held read-only ONLY when an edit would - // SILENTLY DROP data: a run carrying a TsString property the DetailTextRun model does not - // round-trip (colour, offset, superscript β€” flagged lossyProperties) since the first plain-text - // edit skips the lossless RichXml fast-path. The explicit canEditRichText flag still lets a - // caller force read-only for a reason unrelated to runs (e.g. a voice/audio alternative). + // A value goes read-only only when an edit would silently drop data -- a run + // with a non-round-trippable TsString property (lossyProperties), not for + // carrying an ORC, fully editable via run-replay. CanEditRichText = canEditRichText && !lossyProperties; GraphemeClusterStarts = DetailTextGraphemeClusters.GetClusterStarts(PlainText); } @@ -201,7 +200,8 @@ public DetailRichTextValue(string plainText, IReadOnlyList runs, /// /// Whether at least one run carries a TsString text property the /// model does NOT round-trip (e.g. foreground/background colour, character offset, - /// super/subscript β€” anything beyond ws/named-style/font-family/font-size/bold/italic/underline/ + /// super/subscript -- anything beyond + /// ws/named-style/font-family/font-size/bold/italic/underline/ /// object-data). The neutral run-replay in DetailRichTextAdapter.ToTsString re-emits only /// the supported set, so a first edit (which skips the lossless RichXml fast-path) would silently /// drop the extra property. Such a value is shown read-only with the embedded-object tooltip @@ -547,11 +547,13 @@ public static class DetailRichTextEditAlgorithms /// Applies (or clears) one character-formatting attribute over the half-open span /// [start, end), returning a NEW with the same plain /// text. Runs are split at the selection boundaries (reusing the same run-span machinery as - /// ); every run fully covered by the span gets the attribute set + /// ); every run fully covered by the span gets the + /// attribute set /// to while runs outside the span keep their metadata untouched. /// The selection is snapped OUTWARD to Unicode grapheme-cluster boundaries (the same /// boundaries the bidi navigation uses) so a combining cluster is never split mid-character. - /// A zero-length (collapsed) selection β€” after clamping/snapping β€” is a no-op (the original + /// A zero-length (collapsed) selection -- after clamping/snapping -- is a no-op + /// (the original /// value is returned); there is no pending caret format. /// The result intentionally carries NO RichXml: the lossless XML fast-path in /// DetailRichTextAdapter.ToTsString would otherwise re-emit the ORIGINAL runs (the plain @@ -611,13 +613,16 @@ public static DetailRichTextValue ApplySpanFormatting(DetailRichTextValue value, /// Applies (or clears) a NAMED CHARACTER STYLE over the half-open span [start, end), /// returning a NEW with the same plain text. Reuses the same /// run-split + grapheme-cluster-safe machinery as : every run fully - /// covered by the (cluster-snapped) span has its set to - /// while runs outside the span keep their metadata untouched. + /// covered by the (cluster-snapped) span has its + /// set to + /// while runs outside the span keep their metadata + /// untouched. /// A null/empty CLEARS the named style over the span (the /// covered runs revert to the default/no-style paragraph style), matching the picker's /// "Default/None" entry. /// The span is snapped OUTWARD to Unicode grapheme-cluster boundaries so a combining cluster - /// is never split mid-character. A zero-length (collapsed) selection β€” after clamping/snapping β€” is + /// is never split mid-character. A zero-length (collapsed) selection -- after + /// clamping/snapping -- is /// a no-op (the original value is returned). Lossy / read-only values are returned unchanged. /// The result carries NO RichXml (same reason as ): /// the lossless XML fast-path would otherwise re-emit the ORIGINAL runs (plain text is unchanged), @@ -676,7 +681,8 @@ public static DetailRichTextValue ApplySpanNamedStyle(DetailRichTextValue value, /// /// Retags the WRITING SYSTEM over the half-open span [start, end), returning a /// NEW with the same plain text. Reuses the same run-split + - /// grapheme-cluster-safe machinery as /: + /// grapheme-cluster-safe machinery as /: /// every run fully covered by the (cluster-snapped) span has its /// set to while runs outside /// the span keep their metadata untouched. The per-run ws tag is exactly what @@ -685,7 +691,8 @@ public static DetailRichTextValue ApplySpanNamedStyle(DetailRichTextValue value, /// A null/empty is a no-op (a run must always carry a writing system; /// the picker only offers real project writing systems, never a "clear"). /// The span is snapped OUTWARD to Unicode grapheme-cluster boundaries so a combining cluster - /// is never split mid-character. A zero-length (collapsed) selection β€” after clamping/snapping β€” is + /// is never split mid-character. A zero-length (collapsed) selection -- after + /// clamping/snapping -- is /// a no-op (the original value is returned). Lossy / read-only values are returned unchanged. /// The result carries NO RichXml (same reason as ): /// the lossless XML fast-path would otherwise re-emit the ORIGINAL runs (plain text is unchanged), @@ -743,7 +750,8 @@ public static DetailRichTextValue RetagSpanWritingSystem(DetailRichTextValue val /// /// Applies an EXTERNAL-LINK ORC (a hyperlink) over the half-open span [start, end), /// returning a NEW with the same plain text whose covered runs - /// carry the link's ObjectData (the kodtExternalPathName tag char + the URL) β€” the + /// carry the link's ObjectData (the kodtExternalPathName tag char + the + /// URL) -- the /// model side of FwEditingHelper.AddHyperlink. Reuses the same run-split + cluster-snap /// machinery as the style/ws helpers. A collapsed selection or a null/empty URL is a no-op (the /// original value is returned). Lossy / read-only values are returned unchanged. The result drops @@ -815,7 +823,8 @@ public static DetailRichTextValue EditHyperlinkUrl(DetailRichTextValue value, in /// /// Deletes the ORC run that STARTS at plain-text position - /// (removing its text β€” typically the single object-replacement char), returning a NEW value. + /// (removing its text -- typically the single object-replacement char), returning a NEW + /// value. /// Generic delete: ANY ORC kind (link, picture, footnote, other) is removable. A position that is /// not the start of an ORC run is a no-op. The result drops RichXml so the adapter re-emits /// via run-replay. @@ -988,7 +997,8 @@ public static string SpanNamedStyle(DetailRichTextValue value, int start, int en /// /// Toggle probe: true when EVERY run overlapping the (cluster-snapped, half-open) span /// [start, end) already carries . The UI uses this to decide a - /// Ctrl+B/I/U gesture's direction β€” an all-on selection toggles off, otherwise it turns on. + /// Ctrl+B/I/U gesture's direction -- an all-on selection toggles off, otherwise it turns + /// on. /// An empty / collapsed span returns false (nothing to toggle off). /// public static bool SpanFullyHasFormat(DetailRichTextValue value, int start, int end, DetailRunFormat which) @@ -1131,7 +1141,7 @@ public static DetailRichTextValue ApplyPlainTextEdit(DetailRichTextValue current } // A pure insertion (nothing removed) defers to legacy TsString behavior: the inserted text - // inherits the PRECEDING run's properties β€” it attaches to the run that ends at the + // inherits the PRECEDING run's properties -- it attaches to the run that ends at the // insertion point, not the following run. (Position 0 falls to the first run, since // nothing precedes it.) Replacements/deletions keep the containing-run logic below. var startRun = originalEditEnd == prefix @@ -1289,7 +1299,8 @@ public DetailWsValue(string wsAbbrev, string value, string fontFamily = null, do public string FontFamily { get; } public double FontSize { get; } - /// Whether this writing system's script is right-to-left (sets editor flow direction). + /// Whether this writing system's script is right-to-left (sets editor flow + /// direction). public bool RightToLeft { get; } /// Stable writing-system tag (e.g. BCP-47 id) for per-WS keyboard activation on focus. @@ -1301,7 +1312,8 @@ public DetailWsValue(string wsAbbrev, string value, string fontFamily = null, do /// /// ITEM 3: whether this alternative belongs to a voice/audio (IsVoice) writing system. The new /// view cannot yet play or record audio, so such a row is composed READ-ONLY with an audio - /// placeholder β€” the recording stays visible/diagnosable instead of presenting a blank editable + /// placeholder -- the recording stays visible/diagnosable instead of presenting a blank + /// editable /// box whose first keystroke would corrupt the stored recording. Editing stays in the classic view. /// public bool IsAudio { get; } @@ -1312,7 +1324,8 @@ public DetailWsValue(string wsAbbrev, string value, string fontFamily = null, do public bool RequiresRichEditor => RichText != null && RichText.RequiresRichEditor; /// - /// Whether the current rich-text content can be edited by the managed rich-text field. Values + /// Whether the current rich-text content can be edited by the managed rich-text field. + /// Values /// carrying unsupported object data remain read-only. /// public bool CanEditRichText => RichText == null || RichText.CanEditRichText; @@ -1325,7 +1338,7 @@ public DetailWsValue(string wsAbbrev, string value, string fontFamily = null, do /// read-only safety carry over verbatim); the /// per-paragraph named style is the legacy StPara.StyleName. An ORC-bearing / lossy paragraph /// is held read-only ( false) and preserved, exactly as a lossy single-WS - /// value is β€” full editing of such a paragraph stays in the classic view. + /// value is -- full editing of such a paragraph stays in the classic view. /// public sealed class DetailParagraph { @@ -1408,7 +1421,8 @@ public DetailChoiceOption(string key, string name, int depth = 0) /// /// Hierarchy level for deep possibility lists: 0 for top-level items, +1 per - /// sub-possibility nesting, in the list's own document order β€” drives the legacy indented + /// sub-possibility nesting, in the list's own document order -- drives the legacy + /// indented /// chooser tree. Flat lists (and chooserInfo FlatList specs) stay 0 throughout. /// public int Depth { get; } @@ -1416,7 +1430,7 @@ public DetailChoiceOption(string key, string name, int depth = 0) /// /// A list-editor jump link on a chooser/reference-vector row: the legacy chooser dialog's - /// "Edit the … list" LinkLabel (ReallySimpleListChooser.AddLink with + /// "Edit the ... list" LinkLabel (ReallySimpleListChooser.AddLink with /// LinkType.kGotoLink), composed from the layout's chooserLink type="goto" /// metadata. Clicking it asks the host to jump to the tool that edits the underlying list. /// @@ -1436,7 +1450,8 @@ public DetailChooserLink(string label, string tool, string targetGuid = null) public string Tool { get; } /// - /// The jump's target object guid string, or null for a plain tool jump β€” the legacy chooser + /// The jump's target object guid string, or null for a plain tool jump -- the legacy + /// chooser /// passes Guid.Empty (m_guidLink) unless a flidTextParam resolved one, /// and none of the lexeme-editor parts carry that. /// @@ -1445,7 +1460,8 @@ public DetailChooserLink(string label, string tool, string targetGuid = null) /// /// A request to follow a chooser jump link: the host dispatches it the way the legacy - /// chooser does on link click β€” mediator FollowLink with FwLinkArgs(tool, target) + /// chooser does on link click -- mediator FollowLink with FwLinkArgs(tool, + /// target) /// (ReallySimpleListChooser.HandleAnyJump). /// public sealed class DetailLinkRequest @@ -1553,10 +1569,12 @@ public DetailField( /// False for display-only fields (e.g. reference fields without chooser write-back yet). public bool IsEditable { get; } - /// Nesting depth for full-layout composition (indents the row like legacy slices). + /// Nesting depth for full-layout composition (indents the row like legacy + /// slices). public int Indent { get; } - /// Whether a header row toggles collapse/expand of the rows nested under it. + /// Whether a header row toggles collapse/expand of the rows nested under + /// it. public bool IsCollapsible { get; } /// Initial expansion state of a collapsible header (from the layout's expansion attr). @@ -1572,7 +1590,8 @@ public DetailField( public string HotlinksId { get; } /// - /// True when this row is a multi-writing-system text row β€” the legacy multistring editor + /// True when this row is a multi-writing-system text row -- the legacy multistring + /// editor /// (MultiStringSlice), as opposed to a single-ws string editor. It mirrors the /// legacy slice is MultiStringSlice test so the in-string context menu can add the shared /// mnuDataTree-MultiStringSlice group (with the Writing Systems submenu) for exactly those @@ -1585,16 +1604,19 @@ public DetailField( /// /// The class of the compiled view definition this row was projected from (advanced-entry-view): - /// the entry's own fields carry "LexEntry"; a row from a descended object (a sense, an allomorph) + /// the entry's own fields carry "LexEntry"; a row from a descended object (a sense, an + /// allomorph) /// carries that object's layout class. Paired with it keys the per-project - /// ViewDefinitionOverride store so the per-field gear-menu commands (Field Visibility / Move + /// ViewDefinitionOverride store so the per-field gear-menu commands (Field + /// Visibility / Move /// Field) target the right layout. Set by the composer at compose time (null on rows built outside /// the full-entry composer, e.g. the first-slice fallback). /// public string ClassName { get; set; } /// - /// The layout name of the compiled view definition this row was projected from (e.g. "Normal"). + /// The layout name of the compiled view definition this row was projected from (e.g. + /// "Normal"). /// See . /// public string LayoutName { get; set; } @@ -1602,7 +1624,8 @@ public DetailField( /// /// The project's available CHARACTER-type style names /// the per-WS editor offers when restyling a selection (sourced by the composer from the project's - /// styles β€” Cache.LangProject.StylesOC filtered to character styles). Empty when no + /// styles -- Cache.LangProject.StylesOC filtered to character styles). Empty when + /// no /// stylesheet is reachable or the field is not a styleable text row; the style picker affordance is /// then suppressed. The host seam: a settable list the composer populates at compose time (like /// /), keeping this FwAvalonia layer LCModel-free. @@ -1612,10 +1635,12 @@ public DetailField( /// /// The project's available writing systems - /// (stable IETF tag + display name) the per-WS editor offers when retagging a selection β€” sourced + /// (stable IETF tag + display name) the per-WS editor offers when retagging a selection + /// -- sourced /// by the composer from Cache (analysis + vernacular writing systems). Empty when no /// writing-system list is reachable or the field is not a retaggable text row; the WS picker - /// affordance is then suppressed. The host seam: a settable list the composer populates at compose + /// affordance is then suppressed. The host seam: a settable list the composer populates + /// at compose /// time (like ), keeping this FwAvalonia layer LCModel-free. A /// test can supply its own list directly. /// @@ -1626,7 +1651,8 @@ public DetailField( /// A map from writing-system tag to the font that ws renders with /// (), supplied by the composer from each ws's DefaultFontName. /// The owned editors use it to draw the inline-display-on-blur per-run font layer for a value / - /// paragraph whose runs differ by ws or style. Empty when no font info is reachable; the display + /// paragraph whose runs differ by ws or style. Empty when no font info is reachable; the + /// display /// layer then falls back to the editor's single font. Kept a settable map so this layer stays /// LCModel-free (like ); a test can supply its own. /// @@ -1641,9 +1667,10 @@ public DetailField( public IReadOnlyList Paragraphs { get; } /// - /// The project's available PARAGRAPH-type style names the structured-text editor offers in + /// The project's available PARAGRAPH-type style names the structured-text editor offers + /// in /// its per-paragraph style picker (the host seam the composer populates from - /// Cache.LangProject.StylesOC filtered to paragraph styles β€” like + /// Cache.LangProject.StylesOC filtered to paragraph styles -- like /// for character styles). Empty when no styles are reachable; /// the per-paragraph style picker affordance is then suppressed. A test can supply its own list. /// @@ -1660,8 +1687,10 @@ public DetailField( /// /// For a row whose targets are searched rather /// than enumerated (possibility lists enumerate, lexicons - /// search): a type-ahead search delegate the composer supplied (e.g. a headword-prefix search - /// over the entry repository). When non-null the add slot opens a search flyout instead of the + /// search): a type-ahead search delegate the composer supplied (e.g. a headword-prefix + /// search + /// over the entry repository). When non-null the add slot opens a search flyout instead + /// of the /// full list; selecting a result stages through /// with the result's key. Like /// , a plain delegate keeps this layer LCModel-free. @@ -1670,14 +1699,14 @@ public DetailField( /// /// The list-editor jump links of a chooser/reference-vector row: composed from the - /// layout's chooserLink type="goto" metadata (e.g. "Edit the Publications list" β†’ + /// layout's chooserLink type="goto" metadata (e.g. "Edit the Publications list" -> /// publicationsEdit). The gear flyout surfaces them below the options; clicking raises the /// host's DetailLinkRequest callback. Empty for rows without chooser metadata. /// public IReadOnlyList ChooserLinks { get; } } - /// Which legacy menu a right-click maps to (section 13). + /// Which legacy menu a right-click maps to. public enum DetailMenuKind { /// The slice menu (layout `menu=`), legacy right-click on the tree node/label. @@ -1691,7 +1720,7 @@ public enum DetailMenuKind } /// - /// A request to show a legacy-defined context menu for a detail row (section 13): the host + /// A request to show a legacy-defined context menu for a detail row: the host /// resolves the menu id against the xCore window configuration and shows the same menu the /// legacy slice shows, at the given screen point, with the row's bound object as command target. /// diff --git a/Src/Common/FwAvalonia/Detail/DetailModelProjector.cs b/Src/Common/FwAvalonia/Detail/DetailModelProjector.cs index 55b2852e3b..d37e533574 100644 --- a/Src/Common/FwAvalonia/Detail/DetailModelProjector.cs +++ b/Src/Common/FwAvalonia/Detail/DetailModelProjector.cs @@ -122,9 +122,9 @@ private static DetailField CreateField( /// /// Maps a node's editor to a renderable kind. Obsolete editors are unsupported; the - /// chooser categories render as choosers; everything else is treated as text β€” the + /// chooser categories render as choosers; everything else is treated as text -- the /// deliberately small first-slice projection. The editor-string knowledge itself lives - /// ONCE, in β€” this method keeps no + /// ONCE, in -- this method keeps no /// heuristics of its own. /// private static DetailFieldKind ClassifyKind(ViewNode node) diff --git a/Src/Common/FwAvalonia/Detail/DetailRichTextChrome.cs b/Src/Common/FwAvalonia/Detail/DetailRichTextChrome.cs index ff9a213c72..854ff4e429 100644 --- a/Src/Common/FwAvalonia/Detail/DetailRichTextChrome.cs +++ b/Src/Common/FwAvalonia/Detail/DetailRichTextChrome.cs @@ -55,7 +55,8 @@ internal static TextBlock CreatePerRunFontDisplay(DetailRichTextValue rich, TextWrapping = TextWrapping.Wrap, VerticalAlignment = VerticalAlignment.Top, // Flat like the editors it stands in for (the box collapses out of layout while the display - // shows, so no overlay is needed β€” a pointer press swaps the editable box back in). + // shows, so no overlay is needed -- a pointer press swaps the editable box back + // in). Background = Brushes.Transparent, FlowDirection = rightToLeft ? FlowDirection.RightToLeft : FlowDirection.LeftToRight }; @@ -104,9 +105,9 @@ internal static Button CreateSpanPicker(IReadOnlyList option Foreground = FwAvaloniaDensity.WsAbbrevBrush, FontSize = FwAvaloniaDensity.WsAbbrevFontSize, VerticalAlignment = VerticalAlignment.Top, - // The trigger must NOT take focus β€” clicking it would blur the editor, and Avalonia - // collapses the TextBox selection to the caret on LostFocus, so onOpen would snapshot an EMPTY - // selection and the gesture would stage nothing. Keeping focus on the editor preserves the span. + // The trigger must NOT take focus: clicking it blurs the editor and + // collapses the selection to the caret on LostFocus. onOpen would then + // snapshot EMPTY and stage nothing; keeping focus preserves the span. Focusable = false }; AutomationProperties.SetAutomationId(button, automationId); diff --git a/Src/Common/FwAvalonia/Detail/DetailStructureRules.cs b/Src/Common/FwAvalonia/Detail/DetailStructureRules.cs index 8f8054d0fd..8bacaf67d1 100644 --- a/Src/Common/FwAvalonia/Detail/DetailStructureRules.cs +++ b/Src/Common/FwAvalonia/Detail/DetailStructureRules.cs @@ -7,11 +7,13 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail { /// - /// The shared structural projection rules used by BOTH detail projectors β€” the thin - /// (view-definition β†’ detail model, LCModel-free) and the full + /// The shared structural projection rules used by BOTH detail projectors -- the thin + /// (view-definition -> detail model, LCModel-free) and the + /// full /// xWorks DetailComposer (LCModel-backed). The section-header row - /// construction and the child-indent rule live ONCE here so the two paths cannot drift. (The third - /// structural rule β€” editor β†’ renderable kind β€” is + /// construction and the child-indent rule live ONCE here so the two paths cannot drift. (The + /// third + /// structural rule -- editor -> renderable kind -- is /// likewise shared, in .) /// public static class DetailStructureRules @@ -25,7 +27,7 @@ public static int ChildIndent(string label, int depth) => string.IsNullOrEmpty(label) ? depth : depth + 1; /// - /// Builds the canonical section-header row β€” the single construction site for + /// Builds the canonical section-header row -- the single construction site for /// rows across both projectors. The thin mapper passes the /// defaults (no collapse affordance, no menu/HVO); the composer passes its LCModel-enriched /// values (collapsible state from expansion, slice menu/hotlinks, owning object HVO). diff --git a/Src/Common/FwAvalonia/Detail/DetailViewingServices.cs b/Src/Common/FwAvalonia/Detail/DetailViewingServices.cs index d5a6409967..8adf2eefc6 100644 --- a/Src/Common/FwAvalonia/Detail/DetailViewingServices.cs +++ b/Src/Common/FwAvalonia/Detail/DetailViewingServices.cs @@ -96,7 +96,8 @@ public static class DetailViewingServices /// /// Every native viewing capability the detail view now provides managed, with its owner and the /// native symbol it supersedes. Owners all live in the FwAvalonia production assembly, which - /// (per EngineIsolationAuditTests) cannot load native Views β€” so by construction these + /// (per EngineIsolationAuditTests) cannot load native Views -- so by construction + /// these /// replacements use Avalonia's own Skia/HarfBuzz text stack, not the C++ engine. /// public static IReadOnlyList Replacements { get; } = diff --git a/Src/Common/FwAvalonia/Detail/FwFieldControls.cs b/Src/Common/FwAvalonia/Detail/FwFieldControls.cs index 4c0a16d00e..d01ae90304 100644 --- a/Src/Common/FwAvalonia/Detail/FwFieldControls.cs +++ b/Src/Common/FwAvalonia/Detail/FwFieldControls.cs @@ -19,21 +19,24 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail { /// /// FieldWorks-owned multi-writing-system text field over an IR-projected detail field - /// (tasks 6.1/6.2): one compact row per writing-system alternative β€” abbreviation gutter plus a + /// (tasks 6.1/6.2): one compact row per writing-system alternative -- abbreviation gutter + /// plus a /// text editor carrying the project WS font, right-to-left flow direction for RTL scripts, and /// per-WS keyboard activation on focus through the supplied callback (the same behavior legacy /// slices get from EditingHelper.SetKeyboardForWs). Write-through staging goes to the /// edit context when one is supplied; otherwise the field is read-only display. /// Multi-run/styled content IS editable here as plain-text-over-preserved-runs: the original /// TsString runs are projected into , a keystroke replays the - /// untouched runs around the edit, and the edit context rebuilds the TsString. A value is held - /// read-only ONLY when that replay would corrupt it β€” an embedded object the runs cannot rebuild, + /// untouched runs around the edit, and the edit context rebuilds the TsString. A value is + /// held + /// read-only ONLY when that replay would corrupt it -- an embedded object the runs cannot + /// rebuild, /// or a run carrying a TsString property the model does not round-trip /// (); such a value shows the explanatory tooltip /// and stays full-fidelity in the classic view. /// Menus: a row whose layout binds a slice menu (`menu=`, e.g. the Lexeme Form's /// mnuDataTree-LexemeForm with Swap/Convert commands) surfaces it on RIGHT-CLICK only (the - /// label/value right-click paths) β€” text rows draw NO gear. The gear is reserved for the + /// label/value right-click paths) -- text rows draw NO gear. The gear is reserved for the /// "configure the supporting list" jump on chooser/vector rows; it never opens a menu. /// Rich-text operations (character style, per-run writing-system retag, insert/edit external link, /// delete embedded object) are NOT always-visible inline controls: like the legacy detail slice, which @@ -47,7 +50,8 @@ public sealed class FwMultiWsTextField : StackPanel, IHoverAffordanceProvider, I { // Teardown registered as each handler/subscription is wired, so a recycled or // active-cell-deactivated field can detach EVERY handler (several capture closures over box, - // currentRich, clipboard) and release its flyouts β€” preventing the handler-closure leak on the + // currentRich, clipboard) and release its flyouts -- preventing the handler-closure leak + // on the // editor path when VirtualizingStackPanel discards the container. private readonly CompositeDisposable _teardown = new CompositeDisposable(); private bool _disposed; @@ -83,17 +87,9 @@ private void AddValueRow(DetailField field, string automationId, var currentRich = value.RichText; var abbrev = CreateWsAbbrev(value); - // Legacy look (12.2): values render flat like RootSite views β€” no box, no fill. - // Local values outrank the theme's pointer-over/focus setters, so the editor stays flat. - // Data-safety read-only: a value whose plain-text run-replay would corrupt it stays - // READ-ONLY β€” and says so explicitly (tooltip) β€” rather than presenting an editable box - // whose first keystroke silently drops content. Two cases feed CanEditRichText: a run - // carrying an embedded object (ORC) the managed editor cannot rebuild, and a run carrying - // a TsString property the DetailTextRun model does not round-trip (e.g. fore/back colour, - // offset, superscript). The original TsString is preserved losslessly (RichXml), so the - // field round-trips and remains fully editable in the classic view. - // A voice/audio writing-system alternative renders as READ-ONLY text (the audio filename); - // there is no in-pane player. Full audio editing stays in the classic view. + // Values render flat with no box/fill, and go read-only with a tooltip + // -- instead of corrupting on the first keystroke -- when a run carries + // an ORC or non-round-trippable TsString property, or is audio. var valueIsReadOnly = editContext == null || !field.IsEditable || !value.CanEditRichText || value.IsAudio; var box = CreateValueBox(field, value, valueIsReadOnly); @@ -165,11 +161,14 @@ private void AddValueRow(DetailField field, string automationId, ? currentCaret : box.SelectionStart; - // The moving edge (nextCaret) already lands on a whole grapheme-cluster boundary - // (MoveCaret steps by clusters) and the anchor is a cluster-aligned caret/selection + // The moving edge (nextCaret) already lands on a whole + // grapheme-cluster boundary + // (MoveCaret steps by clusters) and the anchor is a cluster-aligned + // caret/selection // edge, so the span [anchor..nextCaret] never splits a cluster. Set the caret FIRST: // Avalonia's CaretIndex setter clears the selection, so assigning SelectionStart/ - // SelectionEnd AFTER leaves the caret on the moving edge without re-collapsing β€” the + // SelectionEnd AFTER leaves the caret on the moving edge without + // re-collapsing -- the // original order (selection then CaretIndex) collapsed the span to an empty caret, // which is why Shift+Arrow moved the caret with nothing selected. box.CaretIndex = nextCaret; @@ -244,11 +243,13 @@ private void AddValueRow(DetailField field, string automationId, _teardown.Add(() => box.TextChanged -= textChanged); // Character formatting over a selection. Ctrl+B/I/U toggle bold/italic/ - // underline on the TextBox's current selection. We chose keyboard shortcuts over a + // underline on the TextBox's current selection. We chose keyboard shortcuts + // over a // floating toolbar: they match the legacy Views editor (FwEditingHelper's // Ctrl+B/I/U), need no extra decorations in the dense detail rows, and act on the same // SelectionStart..SelectionEnd the bidi/clipboard handlers already use. The gesture - // only stages when the selection is non-empty (a collapsed caret is a no-op β€” + // only stages when the selection is non-empty (a collapsed caret is a no-op + // -- // there is no pending format for the next insert) and only on an editable, // non-lossy value (this whole block is gated on value.CanEditRichText already). EventHandler formatKeyDown = (s, e) => @@ -299,15 +300,9 @@ private void AddValueRow(DetailField field, string automationId, box.AddHandler(InputElement.KeyDownEvent, formatKeyDown, Avalonia.Interactivity.RoutingStrategies.Tunnel); _teardown.Add(() => box.RemoveHandler(InputElement.KeyDownEvent, formatKeyDown)); - // Apply/clear a NAMED CHARACTER STYLE over the selection. A right-click menu - // item "Character style…" opens the shared FwOptionChooser (single-select) seeded with a - // leading "Default (no style)" entry that CLEARS the style, followed by the project's - // available character style names. It acts on the TextBox's current - // SelectionStart..SelectionEnd; committing calls ApplySpanNamedStyle and stages through - // TrySetRichText β€” exactly the rich-text seam Ctrl+B/I/U uses. Only built when the field - // actually carries available styles (so plain-text-only projects show no item), and only - // off a bridged row (whose host menu owns the field commands); the whole block is already - // gated on the editable, non-lossy value. + // Committing routes through ApplySpanNamedStyle + TrySetRichText -- the same + // rich-text seam Ctrl+B/I/U uses -- and is only built off a bridged row when + // the field actually has available styles. if (!hasBridge && field.AvailableNamedStyles != null && field.AvailableNamedStyles.Count > 0) { // The picker's option set: a clear-style entry (empty key) plus one option per @@ -409,7 +404,8 @@ private void AddValueRow(DetailField field, string automationId, // System" button opening the shared FwOptionChooser (single-select) seeded with the // project's available writing systems (tag = key, display name = caption). It acts on the // TextBox's current SelectionStart..SelectionEnd; committing calls RetagSpanWritingSystem - // and stages through TrySetRichText β€” the same rich-text seam Ctrl+B/I/U and the style + // and stages through TrySetRichText -- the same rich-text seam Ctrl+B/I/U and + // the style // picker use. Built only when the field carries available writing systems; the whole block // is already gated on the editable, non-lossy value. There is no "clear" entry: a run must // always carry a writing system, so the picker offers only real project writing systems. @@ -506,7 +502,8 @@ private void AddValueRow(DetailField field, string automationId, } } - // External-link insert / edit prompt. A right-click menu item "Insert/edit link…" + // External-link insert / edit prompt. A right-click menu item + // "Insert/edit link..." // opens a flyout with a URL TextBox + Apply (the dialog-light prompt the decision calls // for). On open it snapshots the selection and, when that selection sits on an existing // link run, pre-fills the URL for editing. Apply over a real selection inserts/edits the @@ -681,7 +678,8 @@ private void AddValueRow(DetailField field, string automationId, WireWritingSystemKeyboard(box, value, writingSystemFocused); // The value box's right-click menu (legacy MultiStringSlice parity: operations live OFF the - // row, not as always-visible inline controls). A non-bridged row carries a local menu β€” Copy + // row, not as always-visible inline controls). A non-bridged row carries a local + // menu -- Copy // plus whatever rich-text operations its gate built (character style / writing-system retag / // insert-or-edit link / delete embedded object). A bridged row's host xCore menu is // authoritative and already wired above, so it gets no local menu here. @@ -772,7 +770,8 @@ private void WireGhostPrompt(TextBox box, DetailField field) { if (!string.IsNullOrEmpty(field.GhostPrompt)) { - // 14.1: the legacy ghost add-prompt is a watermark β€” it disappears the moment the + // 14.1: the legacy ghost add-prompt is a watermark -- it disappears the moment + // the // user clicks in (focus), and reappears only if they leave without typing. box.Watermark = field.GhostPrompt; EventHandler ghostGot = (s2, e2) => box.Watermark = string.Empty; @@ -787,7 +786,7 @@ private void WireGhostPrompt(TextBox box, DetailField field) } } - // Section 13: a row with a legacy `contextMenu=` binding shows the SAME xCore-defined + // A row with a legacy `contextMenu=` binding shows the SAME xCore-defined // menu the legacy string view shows (MultiStringSlice.HandleRightMouseClickedEvent // path), routed through the host bridge. That host menu owns this field's commands, so it // stays the single right-click menu for a bridged row; the relocated rich-text operations @@ -809,9 +808,9 @@ private bool WireBridgeContextMenu(TextBox box, DetailField field, EventHandler swallowContext = (s2, e2) => e2.Handled = true; box.AddHandler(InputElement.PointerPressedEvent, menuPressed, Avalonia.Interactivity.RoutingStrategies.Tunnel); - // 15.2: exactly ONE menu β€” drop the TextBox theme flyout (Cut/Copy/Paste, which - // opens from ContextRequested on right-button RELEASE) so only the bridged menu - // shows, and swallow the request so nothing else opens. + // 15.2: exactly ONE menu -- drop the TextBox flyout (Cut/Copy/Paste, + // which opens from ContextRequested on right-button RELEASE) so only the + // bridged menu shows; swallow the request so nothing else opens. box.ContextFlyout = null; box.AddHandler(Control.ContextRequestedEvent, swallowContext, Avalonia.Interactivity.RoutingStrategies.Tunnel); @@ -847,7 +846,7 @@ private static Grid CreateRowPanel(TextBlock abbrev, Control valueContent, bool { var rowPanel = new Grid { - // 14.2: a null background only hit-tests the glyphs β€” the whole row must + // 14.2: a null background only hit-tests the glyphs -- the whole row must // receive hover/right-click over the gaps too. Background = Brushes.Transparent }; @@ -938,7 +937,8 @@ private static DetailTextRun RunAt(DetailRichTextValue rich, int start) public IReadOnlyList HoverAffordances => Array.Empty(); /// - /// The count of still-attached handler/subscription teardowns β€” zero after . + /// The count of still-attached handler/subscription teardowns -- zero after . /// Exposed so a recycling test can assert the editor released every handler it wired. /// public int AttachedHandlerCount => _teardown.Count; @@ -979,9 +979,9 @@ private static async System.Threading.Tasks.Task CopySelectionAsync(TextBox box, /// /// GEAR = CONFIGURE: the shared gear semantics of the chooser and reference-vector rows. - /// Clicking the gear DIRECTLY dispatches the list-editor jump β€” the host's + /// Clicking the gear DIRECTLY dispatches the list-editor jump -- the host's /// callback rides the same path the legacy chooser dialog's - /// "Edit the … list" LinkLabel rides (ReallySimpleListChooser.AddLink kGotoLink β†’ + /// "Edit the ... list" LinkLabel rides (ReallySimpleListChooser.AddLink kGotoLink -> /// FollowLink). NO flyout, NO context menu opens from the gear; option flyouts carry zero /// link items. The gear renders ONLY when a list-edit target resolved at compose time (the /// row carries at least one goto ); the FIRST link wins when @@ -1017,15 +1017,17 @@ internal static Button CreateConfigureGear(DetailField field, string automationI /// /// FieldWorks-owned chooser field: a button opening a flyout of service-backed options /// (the options come from the LCModel-sourced detail model, not the control). The flyout is the - /// shared compact β€” an AutoCompleteBox-based OPTIONS ONLY selector, + /// shared compact -- an AutoCompleteBox-based OPTIONS ONLY + /// selector, /// no link items. Committing an /// option stages it through the edit context, closes the flyout, and returns focus to the button - /// β€” the popup-focus-return behavior the seam specs require. Without an edit context the chooser + /// -- the popup-focus-return behavior the seam specs require. Without an edit context the + /// chooser /// is a read-only display of the current selection. - /// Styling: the button is transparent/borderless β€” the value text reads flat like the legacy + /// Styling: the button is transparent/borderless -- the value text reads flat like the legacy /// combo. When the row's supporting list resolved a list-editor target (a composed goto /// ), a hover-revealed CONFIGURE gear sits after the value and - /// directly dispatches the host jump () β€” it never opens the + /// directly dispatches the host jump () -- it never opens the /// options. Rows without a resolvable list editor draw no gear. /// public sealed class FwChooserField : Button, IHoverAffordanceProvider, IDisposable @@ -1068,11 +1070,9 @@ public FwChooserField( if (_gear != null) content.Children.Add(_gear); Content = content; - // Read-only rows stay ENABLED: disabling the whole button would suppress its pointer - // events (killing hover-reveal) and disable the nested configure gear β€” which is - // NAVIGATION (the "Edit the … list" jump), not editing. Like FwDialogLauncherField, - // only the value-editing affordance is withheld: no option flyout is wired below, so - // clicking the value of a read-only row does nothing. + // Read-only rows stay ENABLED because disabling the button would kill hover-reveal + // and disable the nested configure gear, which is navigation, not editing; only the + // value-editing flyout is withheld. AutomationProperties.SetAutomationId(this, automationId); AutomationProperties.SetName(this, field.Label ?? field.Field ?? automationId); @@ -1116,7 +1116,8 @@ public FwChooserField( }); } - /// The count of still-attached subscriptions β€” zero after . + /// The count of still-attached subscriptions -- zero after . public int AttachedHandlerCount => _teardown.Count; /// @@ -1133,7 +1134,8 @@ public void Dispose() _teardown.Clear(); } - // Restyled appearance only β€” the control keeps the Button theme (template, flyout-on-click, + // Restyled appearance only -- the control keeps the Button theme (template, + // flyout-on-click, // focus, automation peer), not a lookup by this derived type's key. protected override Type StyleKeyOverride => typeof(Button); @@ -1143,7 +1145,8 @@ public void Dispose() /// The display text of the current selection (what the value TextBlock shows). public string ValueText => _valueText.Text; - /// The configure gear (only when a list-edit target resolved); empty otherwise. + /// The configure gear (only when a list-edit target resolved); empty + /// otherwise. public IReadOnlyList HoverAffordances => _gear == null ? Array.Empty() : new Control[] { _gear }; @@ -1157,15 +1160,16 @@ private static string CurrentName(DetailField field) /// /// FieldWorks-owned editable reference-vector field: the current items rendered /// inline, each followed by the thin grey separator bar legacy reference slices draw - /// (VwSeparatorBox), with the TRAILING bar fronting the add slot β€” a "+" launcher whose flyout + /// (VwSeparatorBox), with the TRAILING bar fronting the add slot -- a "+" launcher whose + /// flyout /// is the shared compact (AutoCompleteBox-based OPTIONS ONLY, /// zero link items): the /// possibility tree indented by for enumerated lists, /// or the host search delegate's results for search-backed vectors (lexicons search, lists /// enumerate), both behind the same filter box and virtualized capped list. /// Right-clicking an item offers Remove. Without an edit context the row is read-only display. - /// Hover-reveal polish: the separator bars, the "+" launcher, and β€” only when the - /// row's list resolved a list-editor target β€” the CONFIGURE gear (which directly dispatches + /// Hover-reveal polish: the separator bars, the "+" launcher, and -- only when the + /// row's list resolved a list-editor target -- the CONFIGURE gear (which directly dispatches /// the host jump, never a flyout: ) fade in on row hover; the /// items/text stay always visible. /// @@ -1173,7 +1177,8 @@ public sealed class FwReferenceVectorField : StackPanel, IHoverAffordanceProvide { private readonly List _affordances = new List(); // Teardown for the per-item Remove handlers, the add picker's OptionCommitted/Dismissed - // subscriptions, the gear click, and the option flyout β€” so a recycled vector cell releases + // subscriptions, the gear click, and the option flyout -- so a recycled vector cell + // releases // every closure it wired and drops its flyout, mirroring FwChooserField/FwMultiWsTextField // (wiring these with NO teardown leaks the editor path // when VirtualizingStackPanel discards the container). Empty for read-only rows. @@ -1183,7 +1188,7 @@ public sealed class FwReferenceVectorField : StackPanel, IHoverAffordanceProvide /// /// (optional, like the other field callbacks): invoked /// after a SUCCESSFUL add/remove stage, so the host view can commit the gesture immediately - /// β€” legacy commits each chooser-dialog gesture as it lands, and the row's Items are a + /// -- legacy commits each chooser-dialog gesture as it lands, and the row's Items are a /// compose-time snapshot, so without a commit + re-show nothing visibly changes. /// Failed stages never fire it. /// @@ -1195,7 +1200,8 @@ public FwReferenceVectorField( Action linkRequested = null) { Orientation = Orientation.Horizontal; - // 14.2-style hit-testing rule: a null background only hit-tests the glyphs β€” the WHOLE + // 14.2-style hit-testing rule: a null background only hit-tests the glyphs -- the + // WHOLE // row must receive hover so the reveal affordances work over the gaps between items. Background = Brushes.Transparent; AutomationProperties.SetAutomationId(this, automationId); @@ -1209,7 +1215,8 @@ public FwReferenceVectorField( Text = item.Name, VerticalAlignment = VerticalAlignment.Center, Margin = new Thickness(0, 0, 4, 0), - // 14.2: a null background only hit-tests the glyphs β€” the whole item must take + // 14.2: a null background only hit-tests the glyphs -- the whole item must + // take // the right-click or the Remove flyout only opens over ink. Background = Brushes.Transparent }; @@ -1262,7 +1269,7 @@ public FwReferenceVectorField( AutomationProperties.SetAutomationId(addButton, automationId + ".Add"); AutomationProperties.SetName(addButton, FwAvaloniaStrings.AddItem); - // "+" = OPTIONS ONLY: the one compact filterable picker β€” static options enumerate + // "+" = OPTIONS ONLY: the one compact filterable picker -- static options enumerate // (with Depth hierarchy), search-backed vectors ride the host search delegate. // No link items ever ride this flyout. The vector add slot opens in MULTI-SELECT mode // (checkboxes + an "Add" button): the user checks several candidates and commits the @@ -1306,7 +1313,7 @@ public FwReferenceVectorField( _affordances.Add(addButton); // GEAR = CONFIGURE (only when the row's list resolved a list-editor target): clicking - // dispatches the host jump directly β€” it does NOT open the add flyout. + // dispatches the host jump directly -- it does NOT open the add flyout. var gearButton = DetailGearChrome.CreateConfigureGear(field, automationId, linkRequested); if (gearButton != null) { @@ -1323,7 +1330,8 @@ public FwReferenceVectorField( public IReadOnlyList HoverAffordances => _affordances; /// - /// The count of still-attached subscriptions/handlers β€” zero after . + /// The count of still-attached subscriptions/handlers -- zero after . /// Exposed so a recycling test can assert the editor released every handler it wired. /// public int AttachedHandlerCount => _teardown.Count; @@ -1345,7 +1353,7 @@ public void Dispose() } // The legacy VwSeparatorBox: a ~2px, font-height, light grey vertical bar after each item - // (and fronting the add slot) β€” the affordance that marks where content can be added. + // (and fronting the add slot) -- the affordance that marks where content can be added. private void AddSeparatorBar() { var bar = new Border @@ -1366,7 +1374,8 @@ private void AddSeparatorBar() /// trailing launcher button, drawn as the SAME hover-revealed settings gear the chooser and /// reference vector draw. The button invokes a host-injected callback (a plain delegate; this /// layer stays LCModel-free). Without a callback the gear renders DISABLED with an explanatory - /// tooltip β€” the value still shows, the affordance is visibly unavailable once hover reveals it. + /// tooltip -- the value still shows, the affordance is visibly unavailable once hover reveals + /// it. /// public sealed class FwDialogLauncherField : DockPanel, IHoverAffordanceProvider { @@ -1378,19 +1387,20 @@ public FwDialogLauncherField(string value, string label, Action launch) _launch = launch; Value = value ?? string.Empty; LastChildFill = true; - // A null background only hit-tests the glyphs β€” the WHOLE row must receive hover + // A null background only hit-tests the glyphs -- the WHOLE row must receive hover // so the gear reveal works over the gaps. Background = Brushes.Transparent; AutomationProperties.SetName(this, label ?? string.Empty); - // The legacy ButtonLauncher launch affordance, docked at the row's end like m_panel β€” + // The legacy ButtonLauncher launch affordance, docked at the row's end like m_panel + // -- // drawn as the shared settings gear, hover-revealed like the chooser/vector ones. _button = DetailChrome.CreateGearButton(); _button.IsEnabled = launch != null; AutomationProperties.SetName(_button, FwAvaloniaStrings.LaunchDialog); if (launch == null) { - // Degraded mode: no host dialog service β€” the gear shows but cannot launch. + // Degraded mode: no host dialog service -- the gear shows but cannot launch. ToolTip.SetTip(_button, FwAvaloniaStrings.LauncherUnavailable); } _button.Click += (s, e) => Launch(); diff --git a/Src/Common/FwAvalonia/Detail/FwOptionChooser.cs b/Src/Common/FwAvalonia/Detail/FwOptionChooser.cs index c9658be13d..3e9b291a53 100644 --- a/Src/Common/FwAvalonia/Detail/FwOptionChooser.cs +++ b/Src/Common/FwAvalonia/Detail/FwOptionChooser.cs @@ -24,11 +24,12 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail /// /// The ONE compact, filterable select-from-list control every Avalonia option picker uses: /// the chooser's single-select flyout, the reference vector's "+" add flyout, and preview - /// morph-type chooser. It is a small NATIVE composite β€” a filter box - /// stacked over a of options β€” shown INLINE inside the host flyout. The + /// morph-type chooser. It is a small NATIVE composite -- a filter box + /// stacked over a of options -- shown INLINE inside the host flyout. + /// The /// host flyout is therefore the only popup; there is no second floating dropdown (an /// AutoCompleteBox would spawn a separate grey-chromed PART_SuggestionsContainer - /// popup β€” the source of a heavy grey border and focus/arrow-key flakiness). + /// popup -- the source of a heavy grey border and focus/arrow-key flakiness). /// /// Keyboard handling is trivial because focus never leaves the filter box: the options list is /// = false, and Down/Up/Enter/Escape are handled directly @@ -41,18 +42,16 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail /// from committing the highlighted item. /// /// Committing/dismissing is still the HOST field's signal to stage (TrySetOption / - /// TryAddReferenceItem) and hide its flyout β€” the picker itself never stages. The picker keeps + /// TryAddReferenceItem) and hide its flyout -- the picker itself never stages. The picker + /// keeps /// the typed search text visible while arrowing through results; it does not overwrite the query /// with the highlighted row's display text. /// public sealed class FwOptionChooser : Border { - // Diagnostics for the picker's focus/keyboard routing (the historic arrow-key trouble spot). - // OFF by default. Enable either via the "FwOptionPicker" switch (value >= 3) in - // Src/Common/FieldWorks/FieldWorks.Diagnostics.dev.config (or any app .config), or β€” handy - // for the Avalonia Preview Host, whose generated .config has no switches section β€” by setting - // the FW_OPTIONPICKER_TRACE environment variable (e.g. to 3) before launching. Output lands - // in FieldWorks.trace.log. + // OFF by default; besides the "FwOptionPicker" config switch, set FW_OPTIONPICKER_TRACE + // (e.g. to 3) since the Avalonia Preview Host's generated .config has no switches + // section. private static readonly TraceSwitch s_trace = CreateTraceSwitch(); private static TraceSwitch CreateTraceSwitch() @@ -87,20 +86,14 @@ private static TraceSwitch CreateTraceSwitch() // result set) still resolves to its full DetailChoiceOption for the batch. private readonly Dictionary _seenByKey = new Dictionary(StringComparer.Ordinal); - // The last plainly-toggled row, the anchor for shift+click range selection (multi-select only). + // The last plainly-toggled row, the anchor for shift+click range selection (multi-select + // only). private string _anchorKey; private readonly Button _addButton; - // Dropdown (collapsed) presentation mode β€” opt-in, default OFF so every existing consumer - // (chooser single-select flyout, reference-vector "+" multi-select add picker, preview morph - // chooser) is byte-for-byte unchanged: those mount the picker INLINE inside a host flyout and - // want the search box + list always visible. In dropdown mode the picker is instead a compact - // ComboBox-like control: the Border shows a toggle button with the current selection, and the - // existing filter+list panel is hosted in a focus-gated Flyout anchored to the toggle button β€” - // the very Flyout the inline consumers already open through CreateOptionFlyout β€” that opens on - // click and closes on pick, reusing the same filtering + keyboard behavior. A flyout positions - // itself in the trigger's own window, so it stays correct under fractional display scaling; only - // the single-select path supports dropdown mode (the MorphType picker is single-select). + // Opt-in, default OFF so existing inline consumers stay byte-for-byte + // unchanged; dropdown mode reuses the same focus-gated Flyout as + // CreateOptionFlyout; only single-select supports it. private readonly bool _dropdown; private readonly ToggleButton _dropdownButton; private readonly TextBlock _dropdownLabel; @@ -242,7 +235,7 @@ public FwOptionChooser(IReadOnlyList options, _dropdownButton.IsCheckedChanged += OnDropdownButtonCheckedChanged; // The filter+list panel, bordered as in inline mode, becomes the flyout body so the user - // sees the same clean selection panel β€” just floating on top instead of inline. + // sees the same clean selection panel -- just floating on top instead of inline. var popupPanel = new Border { Background = FwAvaloniaDensity.PickerBackgroundBrush, @@ -292,7 +285,8 @@ public FwOptionChooser(IReadOnlyList options, if (_dropdown) { - // Keep the collapsed label in sync with the list selection β€” both the up-front default and + // Keep the collapsed label in sync with the list selection -- both the up-front + // default and // any later external move (the VM's derive-on-type SelectedIndex reselection). _list.SelectionChanged += (s, e) => SyncDropdownLabel(); SyncDropdownLabel(); @@ -300,7 +294,8 @@ public FwOptionChooser(IReadOnlyList options, else { // Inline mode auto-focuses the filter on open (flyout). Dropdown mode is collapsed on - // attach, so it must NOT grab focus β€” focus moves to the filter only when the user opens it. + // attach, so it must NOT grab focus -- focus moves to the filter only when the + // user opens it. AttachedToVisualTree += (s, e) => { Log("AttachedToVisualTree; posting focus (Loaded)."); @@ -319,7 +314,8 @@ private void Log(string message) /// /// Focuses the filter box. Called on attach AND from the host flyout's Opened event, because /// a windowed desktop popup does not synchronously lay out its content, so the flyout's own - /// auto-focus can no-op (GetNext returns null before the template is applied) β€” leaving focus + /// auto-focus can no-op (GetNext returns null before the template is applied) -- leaving + /// focus /// on the launching button, where arrow keys never reach the picker. /// public void FocusFilter() @@ -396,7 +392,8 @@ private void UpdateDropdownLabel() /// /// Builds the host flyout for an option picker with the Fluent FlyoutPresenter's heavy - /// grey decorations (its padding, border, and grey background) stripped to nothing β€” so the + /// grey decorations (its padding, border, and grey background) stripped to nothing -- so + /// the /// picker's own thin border is the ONLY boundary the user sees, instead of the default thick /// grey box wrapping it. Every option picker (chooser, "+" vector add, preview chooser) /// opens through here so the styling stays consistent. @@ -407,11 +404,13 @@ public static Flyout CreateOptionFlyout(FwOptionChooser picker, PlacementMode pl /// /// The single option-flyout construction path: a chromeless that re-requests /// filter focus once open. The inline consumers pass the picker itself as content; dropdown mode - /// passes its filter+list panel and the same picker for focus β€” so both open through one flyout + /// passes its filter+list panel and the same picker for focus -- so both open through one + /// flyout /// implementation instead of a hand-placed popup. /// A windowed desktop popup is shown non-activated (Win32 ShowNoActivate) and the flyout's own /// auto-focus can no-op before the presenter template is applied. Re-request focus once the popup - /// is open, posted at Input priority so it runs AFTER layout/render β€” otherwise focus stays on the + /// is open, posted at Input priority so it runs AFTER layout/render -- otherwise focus + /// stays on the /// launching button and the arrow keys never reach the picker. /// private static Flyout CreateOptionFlyout(object content, FwOptionChooser picker, PlacementMode placement) @@ -466,7 +465,8 @@ public void OpenDropdown() public event Action OptionCommitted; /// - /// Raised when the user commits the CHECKED SET (the "Add" button) in multi-select mode β€” the + /// Raised when the user commits the CHECKED SET (the "Add" button) in multi-select mode + /// -- the /// whole batch in one signal so the host stages it as one undoable step. Never raised in /// single-select mode. Empty checked set does not raise it (the Add button is disabled). /// @@ -677,7 +677,8 @@ private void ApplyFilter() /// Handles navigation at the picker ROOT (the AutoCompleteBox pattern): a single-line /// TextBox does not mark Up/Down as handled, so they bubble from the filter box up to here /// regardless of where exactly focus sits inside the picker. Registered with - /// handledEventsToo so it still fires if some inner control already marked the key handled β€” + /// handledEventsToo so it still fires if some inner control already marked the key + /// handled -- /// far more reliable than a tunnel handler pinned to the TextBox, which only fires when focus /// is exactly on the TextBox. /// @@ -787,7 +788,8 @@ private IDataTemplate OptionTemplate() // Multi-select: a leading checkbox tracking the persisted checked set. The checkbox is // display-only (not hit-test-visible, not focusable) so a single row pointer-release or // Enter toggles it exactly ONCE through ToggleChecked (the row, not the box, owns the - // gesture) β€” matching the legacy multi-check chooser's row-toggle behavior and keeping + // gesture) -- matching the legacy multi-check chooser's row-toggle behavior and + // keeping // focus in the filter box. var check = new CheckBox { diff --git a/Src/Common/FwAvalonia/Detail/FwStructuredTextField.cs b/Src/Common/FwAvalonia/Detail/FwStructuredTextField.cs index 278f0a2395..cfe93b5a25 100644 --- a/Src/Common/FwAvalonia/Detail/FwStructuredTextField.cs +++ b/Src/Common/FwAvalonia/Detail/FwStructuredTextField.cs @@ -18,18 +18,22 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail /// /// FieldWorks-owned editable multi-paragraph structured-text (StText) field. A vertical stack of one /// bordered, dense editor row per paragraph; each row carries a run-aware text editor (the SAME - /// staging the single-WS uses β€” TextChanged replays the untouched + /// staging the single-WS uses -- TextChanged replays the + /// untouched /// runs around the edit and stages through ), /// a per-paragraph named-style picker (the shared ), and add/delete /// paragraph affordances. Enter at a paragraph's end inserts a paragraph after it; Backspace in an /// empty paragraph (when more than one remains) deletes it. /// Commit timing mirrors the reference-vector rule: per-paragraph TEXT edits stage and ride the /// detail view's focus-loss autosave (one undo step per field edit), while STRUCTURAL gestures - /// (add/delete/style) commit immediately through the gestureCompleted callback - /// and the host re-shows β€” the paragraph list is a compose-time snapshot, so without an immediate + /// (add/delete/style) commit immediately through the gestureCompleted + /// callback + /// and the host re-shows -- the paragraph list is a compose-time snapshot, so without an + /// immediate /// commit + re-show the change would not appear. /// An ORC-bearing / lossy paragraph ( - /// false) renders a READ-ONLY box with the embedded-object tooltip and is preserved losslessly β€” full + /// false) renders a READ-ONLY box with the embedded-object tooltip and is preserved + /// losslessly -- full /// editing of such a paragraph stays in the classic view. /// public sealed class FwStructuredTextField : StackPanel, IDisposable @@ -64,9 +68,9 @@ public FwStructuredTextField( gestureCompleted, clipboard, paragraphs[i], i, paragraphs.Count, editable)); } - // An StText always has at least one paragraph in the model; if the composer handed an empty - // list (a not-yet-materialized StText), show a single empty editable row so the user can type - // β€” the first keystroke materializes the StText through the edit-context setter (index 0). + // An StText always has at least one paragraph in the model; if the composer + // handed an empty list, show a single empty editable row: the first keystroke + // materializes it via the edit-context setter. if (paragraphs.Count == 0) { Children.Add(CreateParagraphRow(field, automationId, structuredText, writingSystemFocused, @@ -299,7 +303,8 @@ private Control CreateStyleAffordance(DetailField field, string automationId, Foreground = FwAvaloniaDensity.WsAbbrevBrush, FontSize = FwAvaloniaDensity.WsAbbrevFontSize, VerticalAlignment = VerticalAlignment.Top, - // Keep focus on the editor β€” a focusable trigger blurs the TextBox, Avalonia collapses + // Keep focus on the editor -- a focusable trigger blurs the TextBox, Avalonia + // collapses // the selection to caret on LostFocus, and the style would apply to an empty span (no-op). Focusable = false }; @@ -555,7 +560,8 @@ private static string FirstRunWsTag(DetailRichTextValue rich) return null; } - /// The count of still-attached handler teardowns β€” zero after . + /// The count of still-attached handler teardowns -- zero after . public int AttachedHandlerCount => _teardown.Count; /// diff --git a/Src/Common/FwAvalonia/Detail/HoverReveal.cs b/Src/Common/FwAvalonia/Detail/HoverReveal.cs index d656e10a27..9080afb352 100644 --- a/Src/Common/FwAvalonia/Detail/HoverReveal.cs +++ b/Src/Common/FwAvalonia/Detail/HoverReveal.cs @@ -17,7 +17,8 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail /// /// A field editor whose row decorations include hover-revealed affordances (the chooser's settings /// gear, the reference vector's separator bars and "+" launcher). The detail view reads this - /// to widen the hover surface to the WHOLE row (label + editor) β€” presentation only, no behavior. + /// to widen the hover surface to the WHOLE row (label + editor) -- presentation only, no + /// behavior. /// public interface IHoverAffordanceProvider { @@ -26,12 +27,14 @@ public interface IHoverAffordanceProvider } /// - /// Modern hover-reveal presentation for secondary affordances: the affordances start hidden by - /// OPACITY (they stay in layout β€” rows never reflow β€” and stay in the UIA tree, focusable), + /// Modern hover-reveal presentation for secondary affordances: the affordances start hidden + /// by + /// OPACITY (they stay in layout -- rows never reflow -- and stay in the UIA tree, focusable), /// fade in (~120ms) while the pointer is over any hover source or any affordance (entering /// the gear itself must not flicker it away), and fade out when the pointer leaves them all. /// Keyboard access: an affordance gaining focus (Tab) also reveals; losing focus hides again - /// unless the pointer is over. Pure presentation β€” no flyout, staging, or automation-id changes. + /// unless the pointer is over. Pure presentation -- no flyout, staging, or automation-id + /// changes. /// public static class HoverReveal { @@ -40,7 +43,7 @@ public static class HoverReveal // The reveal registration of an affordance: stamped the first time the affordance is // attached, looked up (and merged into) by every later Attach. The property is the - // idempotence anchor β€” without it each Attach call would stack an independent handler + // idempotence anchor -- without it each Attach call would stack an independent handler // set with its own watched list, and the groups would fight over the opacity. private static readonly AttachedProperty RevealGroupProperty = AvaloniaProperty.RegisterAttached("HoverRevealGroup", typeof(HoverReveal)); @@ -49,8 +52,9 @@ public static class HoverReveal /// Wires to reveal while the pointer is over any of /// (or over an affordance itself) and hide otherwise. /// Idempotent per affordance: attaching again (the view widening the hover surface to the - /// row after the control wired itself) merges into the existing registration β€” one handler - /// set, one watched list β€” instead of stacking a second independent one. + /// row after the control wired itself) merges into the existing registration -- one + /// handler + /// set, one watched list -- instead of stacking a second independent one. /// public static void Attach(IReadOnlyList hoverSources, IReadOnlyList affordances) { @@ -59,9 +63,9 @@ public static void Attach(IReadOnlyList hoverSources, IReadOnlyList()).Where(s => s != null).Distinct().ToList(); - // Resolve the registration this call lands in: the first already-registered target's - // group wins; targets registered in OTHER groups merge into it (an Attach spanning - // previously separate registrations unifies them β€” they reveal together from then on). + // Resolve the registration this call lands in: the first registered + // target's group wins; targets in OTHER groups merge into it (an Attach + // spanning registrations unifies them -- they reveal together). RevealGroup group = null; foreach (var affordance in targets) { @@ -195,16 +199,17 @@ internal static void SetRevealed(IEnumerable affordances, bool revealed /// internal static class DetailChrome { - // A real cog drawn as geometry (circle + teeth + hub hole, even-odd fill), not a text/emoji + // A real cog drawn as geometry (circle + teeth + hub hole, even-odd fill), not a + // text/emoji // glyph: 8 teeth on a 24-unit canvas rendered at ~14px in the muted ws-abbreviation hue. private static readonly Geometry GearGeometry = CreateGearGeometry(); - // A vertical ellipsis ("β‹", the "kebab" field-menu glyph): three stacked dots on the same - // 24-unit canvas, drawn as model EllipseGeometry (no stream context) so it renders in the - // headless unit tests that build these controls with no Avalonia platform loaded. + // A vertical ellipsis ("...", the "kebab" field-menu glyph): three + // stacked dots on the same 24-unit canvas, drawn as model EllipseGeometry + // so it renders in headless tests with no Avalonia platform loaded. private static readonly Geometry KebabGeometry = CreateKebabGeometry(); - /// The "β‹" field-options glyph, in the muted affordance hue. + /// The "..." field-options glyph, in the muted affordance hue. internal static Control CreateKebabIcon() => new Avalonia.Controls.Shapes.Path { @@ -216,7 +221,8 @@ internal static Control CreateKebabIcon() VerticalAlignment = VerticalAlignment.Center }; - /// A flat (transparent, borderless) button carrying the "β‹" glyph as its face. + /// A flat (transparent, borderless) button carrying the "..." glyph as its + /// face. internal static Button CreateKebabButton() => new Button { @@ -265,7 +271,8 @@ internal static Button CreateGearButton() // Built from MODEL segments (PathGeometry/ArcSegment/LineSegment), NOT StreamGeometry.Open: // opening a stream context demands the IPlatformRenderInterface, and xWorks hosts construct - // these controls in plain unit tests with no Avalonia platform loaded β€” model geometry only + // these controls in plain unit tests with no Avalonia platform loaded -- model geometry + // only // touches the platform when actually rendered. private static Geometry CreateGearGeometry() { diff --git a/Src/Common/FwAvalonia/Detail/IDetailEditContext.cs b/Src/Common/FwAvalonia/Detail/IDetailEditContext.cs index f6f0fa942e..3c031d62a0 100644 --- a/Src/Common/FwAvalonia/Detail/IDetailEditContext.cs +++ b/Src/Common/FwAvalonia/Detail/IDetailEditContext.cs @@ -12,7 +12,7 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail /// fenced commit/cancel boundary. The product implementation (xWorks) opens one fenced LCModel /// undo task lazily on the first staged edit, applies writes directly to the domain inside it, /// and ends it on (one step on the single global undo stack shared with - /// the legacy UI) or rolls it back on β€” the model the + /// the legacy UI) or rolls it back on -- the model the /// `avalonia-edit-sessions` and `avalonia-undo-redo` seam specs require. This layer stays /// LCModel-free so the Avalonia view can drive editing without a domain dependency; tests use a /// fake context. @@ -46,14 +46,17 @@ public interface IDetailEditContext : IEditSession /// /// Stages adding an item (by option key) to a - /// row (6.3). Returns false β€” WITHOUT opening the session β€” for keys outside the field's + /// row (6.3). Returns false -- WITHOUT opening the session -- for keys outside the + /// field's /// possibility list, duplicates, or non-vector rows, like the legacy chooser. /// bool TryAddReferenceItem(DetailField field, string optionKey); /// - /// Stages removing an item (by option key) from a - /// row. Returns false β€” without opening the session β€” when the item is not in the vector. + /// Stages removing an item (by option key) from a + /// row. Returns false -- without opening the session -- when the item is not in the + /// vector. /// bool TryRemoveReferenceItem(DetailField field, string optionKey); diff --git a/Src/Common/FwAvalonia/Detail/IStructuredTextEditing.cs b/Src/Common/FwAvalonia/Detail/IStructuredTextEditing.cs index daa6bae38c..cb7dff6dbb 100644 --- a/Src/Common/FwAvalonia/Detail/IStructuredTextEditing.cs +++ b/Src/Common/FwAvalonia/Detail/IStructuredTextEditing.cs @@ -17,7 +17,8 @@ public interface IStructuredTextEditing /// /// Stages a run-aware text edit to ONE paragraph of a /// (StText) field, opening the session on the first - /// edit. Returns false β€” WITHOUT opening the session β€” for a non-StText row, an out-of-range + /// edit. Returns false -- WITHOUT opening the session -- for a non-StText row, an + /// out-of-range /// paragraph index, or an ORC/lossy (read-only) paragraph. Like the run-aware single-WS path, /// the rich payload preserves run metadata so the product ITsString rebuilds without /// flattening. @@ -26,23 +27,29 @@ public interface IStructuredTextEditing /// /// Stages setting (or clearing, when is null/empty) the named - /// paragraph style of ONE paragraph of a field. - /// Returns false β€” without opening the session β€” for a non-StText row or an out-of-range index. + /// paragraph style of ONE paragraph of a + /// field. + /// Returns false -- without opening the session -- for a non-StText row or an + /// out-of-range index. /// bool TrySetParagraphStyle(DetailField field, int paragraphIndex, string styleName); /// - /// Stages inserting a new empty paragraph AFTER in a + /// Stages inserting a new empty paragraph AFTER in + /// a /// field (a negative index inserts at the start). - /// Returns false β€” without opening the session β€” for a non-StText row. The structural gesture + /// Returns false -- without opening the session -- for a non-StText row. The structural + /// gesture /// commits immediately and the host re-shows (the model's paragraph list is a compose snapshot). /// bool TryInsertParagraph(DetailField field, int afterParagraphIndex); /// /// Stages deleting paragraph of a - /// field. Returns false β€” without opening the - /// session β€” for a non-StText row, an out-of-range index, or when it would delete the only + /// field. Returns false -- without opening + /// the + /// session -- for a non-StText row, an out-of-range index, or when it would delete the + /// only /// paragraph (the StText always keeps at least one, like the legacy editor). /// bool TryDeleteParagraph(DetailField field, int paragraphIndex); diff --git a/Src/Common/FwAvalonia/Detail/LexiconFirstSlice.cs b/Src/Common/FwAvalonia/Detail/LexiconFirstSlice.cs index bf9fe8bcd0..b582547709 100644 --- a/Src/Common/FwAvalonia/Detail/LexiconFirstSlice.cs +++ b/Src/Common/FwAvalonia/Detail/LexiconFirstSlice.cs @@ -20,7 +20,8 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail /// - gloss: the LexSense-Detail-GlossAllA part slice (Gloss, multistring) compiled /// from the real parts inventory through a one-line caller layout. The shipped /// LexSense/Normal layout reaches Gloss only through its HeavySummary part ref, - /// which has no part definition in the shipped inventory β€” legacy DataTree walks the class + /// which has no part definition in the shipped inventory -- legacy DataTree walks the + /// class /// hierarchy and then silently omits it (DataTree.ProcessPartRefNode), so the part inventory, not /// that layout, is the live source for the gloss slice's semantics. /// Stable ids therefore derive from the real layout/part paths. Product metadata (automation ids, @@ -35,7 +36,8 @@ public static class LexiconFirstSlice private const string GlossCallerLayout = ""; - /// Subclass β†’ base class chain for part-ref resolution, mirroring the LCModel hierarchy. + /// Subclass -> base class chain for part-ref resolution, mirroring the LCModel + /// hierarchy. private static readonly Dictionary MoFormBaseClassMap = new Dictionary(StringComparer.Ordinal) { { "MoStemAllomorph", "MoForm" }, diff --git a/Src/Common/FwAvalonia/Detail/MorphTypeSwapLogic.cs b/Src/Common/FwAvalonia/Detail/MorphTypeSwapLogic.cs index 34fc49e180..3ba04aedc8 100644 --- a/Src/Common/FwAvalonia/Detail/MorphTypeSwapLogic.cs +++ b/Src/Common/FwAvalonia/Detail/MorphTypeSwapLogic.cs @@ -56,7 +56,7 @@ public static class MorphTypeSwapLogic MorphTypeKind.DiscontiguousPhrase }; - // The ONE GUID β†’ kind table. The seam is + // The ONE GUID -> kind table. The seam is // the cleaner home because it already owns MorphTypeKind and the stem/affix decision, and // both the xWorks composer and any future view can consume it without dragging WinForms // along. This project is deliberately LCModel-free, so the fixed MoMorphTypeTags model GUIDs @@ -100,7 +100,7 @@ public static bool TryClassify(Guid morphTypeGuid, out MorphTypeKind kind) public static bool IsStemType(MorphTypeKind type) => StemTypes.Contains(type); /// - /// True if the morph-type GUID classifies as a stem-type β€” the guid-level twin of the + /// True if the morph-type GUID classifies as a stem-type -- the guid-level twin of the /// legacy MorphTypeAtomicLauncher.IsStemType (an unknown guid is not a stem type, /// exactly like the legacy null/guard behavior). /// diff --git a/Src/Common/FwAvalonia/Detail/SliceFactory.cs b/Src/Common/FwAvalonia/Detail/SliceFactory.cs index 794cf63e86..3a988e61b1 100644 --- a/Src/Common/FwAvalonia/Detail/SliceFactory.cs +++ b/Src/Common/FwAvalonia/Detail/SliceFactory.cs @@ -15,7 +15,7 @@ namespace SIL.FieldWorks.Common.FwAvalonia.Detail { /// /// The small bundle of (all-nullable) collaborators a editor needs, - /// passed to so the SAME fieldβ†’control dispatch serves + /// passed to so the SAME field->control dispatch serves /// every host (today the detail-pane detail view, DataTree.CreateEditor; /// any future in-cell editor passes only the collaborators it has). Every member is optional: a null /// edit context yields read-only display; a null callback simply disables that affordance. @@ -41,19 +41,21 @@ public SliceFactoryContext( ShowWritingSystemAbbreviation = showWritingSystemAbbreviation; } - /// The shared edit-session/staging context; null β†’ read-only display. + /// The shared edit-session/staging context; null -> read-only display. public IDetailEditContext EditContext { get; } - /// Per-WS keyboard activation callback for text fields (null β†’ no keyboard switch). + /// Per-WS keyboard activation callback for text fields (null -> no keyboard + /// switch). public Action WritingSystemFocused { get; } /// Right-click slice/section menu callback (null on hosts without a slice menu). public Action MenuRequested { get; } - /// Hyperlink follow callback for choosers/vectors (null β†’ no link affordance). + /// Hyperlink follow callback for choosers/vectors (null -> no link + /// affordance). public Action LinkRequested { get; } - /// Clipboard seam for text fields (null β†’ framework default). + /// Clipboard seam for text fields (null -> framework default). public IFwClipboard Clipboard { get; } /// @@ -71,11 +73,12 @@ public SliceFactoryContext( } /// - /// The single β†’Avalonia-control dispatch. The detail + /// The single ->Avalonia-control dispatch. The detail /// pane (DataTree.CreateEditor, all 7 kinds) and the browse in-cell editor - /// (EditableCellHost.Activate, a 2-kind Chooser/Text subset) both route here rather than + /// (EditableCellHost.Activate, a 2-kind Chooser/Text subset) both route here rather + /// than /// hand-rolling their own dispatch, so adding a kind (or changing how a kind is built) happens once. - /// The factory is pure (static) β€” all per-host variation arrives through the + /// The factory is pure (static) -- all per-host variation arrives through the /// . /// public static class SliceFactory @@ -92,7 +95,8 @@ public static Control Build(DetailField field, string automationId, return CreateCustom(field, automationId); case DetailFieldKind.ReferenceVector: // Reference add/remove gestures commit immediately (legacy chooser-dialog behavior): the - // staged session would otherwise sit open β€” LCModel broadcasts PropChanged only at + // staged session would otherwise sit open -- LCModel broadcasts PropChanged + // only at // EndUndoTask and the row's Items are a compose-time snapshot, so the user would see no // change. The gesture-completed callback runs the SAME validation-gated save the // focus-loss autosave uses, whose re-show rebuilds the row from domain truth. A host @@ -139,10 +143,10 @@ private static Control CreateLiteral(DetailField field, string automationId) return block; } - // A plugin-claimed custom slice renders its plugin's own Avalonia - // control in the value column, at the slice's real position. Null guard: a missing, null-returning, - // or throwing factory degrades to the explicit unsupported row β€” never a crash, never a silently - // blank row. + // A plugin-claimed slice renders its plugin's Avalonia control in + // the value column. A missing, null-returning, or throwing factory + // degrades to the unsupported row -- never a crash, never silently + // blank. private static Control CreateCustom(DetailField field, string automationId) { if (field.ControlFactory == null) diff --git a/Src/Common/FwAvalonia/DetailHostControl.cs b/Src/Common/FwAvalonia/DetailHostControl.cs index 2bcdeda106..fc7cbdfae5 100644 --- a/Src/Common/FwAvalonia/DetailHostControl.cs +++ b/Src/Common/FwAvalonia/DetailHostControl.cs @@ -16,7 +16,7 @@ namespace SIL.FieldWorks.Common.FwAvalonia public sealed class DetailHostControl : AvaloniaHostControlBase { // The splitter (label/value column) width the user dragged, remembered across re-shows for - // THIS host only β€” deliberately per-instance, never a process-global static. Used only + // THIS host only -- deliberately per-instance, never a process-global static. Used only // as the in-process fallback when the host (RecordEditView) supplies no session-persistence // hooks; the product host routes a PropertyTable LocalSetting through ShowDetail so the width // also survives across SESSIONS, mirroring legacy slice-splitter persistence. @@ -41,7 +41,8 @@ public void ShowDetail(DetailModel detail, IDetailEditContext editContext = null { if (detail == null) throw new ArgumentNullException(nameof(detail)); // Splitter position persists per-HOST across re-shows: this long-lived host owns - // the in-process remembered width, so each window/preview keeps its own β€” no process-global + // the in-process remembered width, so each window/preview keeps its own -- no + // process-global // field. When the product host supplies persistence hooks, the read/write chains // through them too, so a width dragged in one session is restored in the next; otherwise it // falls back to the process-only field (e.g. the preview host / headless tests). diff --git a/Src/Common/FwAvalonia/FilterableDropdownSupport.cs b/Src/Common/FwAvalonia/FilterableDropdownSupport.cs index 44612ea58a..3e3c5ea100 100644 --- a/Src/Common/FwAvalonia/FilterableDropdownSupport.cs +++ b/Src/Common/FwAvalonia/FilterableDropdownSupport.cs @@ -18,8 +18,10 @@ namespace SIL.FieldWorks.Common.FwAvalonia /// The filter-box-over-list machinery the detail pickers share: a case-insensitive contains /// filter that swaps a tree out for a flat result list, keyboard highlight movement, the /// pointer-release-over-own-item guard, and the compact/chromeless themes. Three controls draw - /// on it β€” FwOptionChooser (flat list, optional flyout), FwPosChooser (tree + flyout), - /// and FwFeatureStructureEditor (inline tree) β€” so the parts they truly hold in common live + /// on it -- FwOptionChooser (flat list, optional flyout), FwPosChooser (tree + + /// flyout), + /// and FwFeatureStructureEditor (inline tree) -- so the parts they truly hold in + /// common live /// here once rather than in each. A static helper rather than a base class: the three differ in /// their content shape, popup hosting, and selection model, so shared state is passed in per call. /// diff --git a/Src/Common/FwAvalonia/FinalizerSafeSynchronizationContext.cs b/Src/Common/FwAvalonia/FinalizerSafeSynchronizationContext.cs index ec2c50e7e4..07d66a7a43 100644 --- a/Src/Common/FwAvalonia/FinalizerSafeSynchronizationContext.cs +++ b/Src/Common/FwAvalonia/FinalizerSafeSynchronizationContext.cs @@ -13,16 +13,17 @@ namespace SIL.FieldWorks.Common.FwAvalonia /// Crash guard for hosting Avalonia inside WinForms. Avalonia's MicroCom COM proxies /// capture the ambient at creation /// (MicroComProxyBase._synchronizationContext) and their FINALIZERS post the native Release - /// back through it. When that post lands after the WinForms marshaling window is gone β€” - /// project switch, window teardown, shutdown, or simply an idle-time GC afterwards β€” + /// back through it. When that post lands after the WinForms marshaling window is gone -- + /// project switch, window teardown, shutdown, or simply an idle-time GC afterwards -- /// WindowsFormsSynchronizationContext.Post throws /// on the FINALIZER thread, which terminates the whole process: - /// InvalidOperationException β†’ Control.MarshaledInvoke β†’ BeginInvoke - /// β†’ WindowsFormsSynchronizationContext.Post β†’ MicroCom.Runtime.MicroComProxyBase.Finalize(). + /// InvalidOperationException -> Control.MarshaledInvoke -> BeginInvoke + /// -> WindowsFormsSynchronizationContext.Post -> + /// MicroCom.Runtime.MicroComProxyBase.Finalize(). /// Installed as the UI thread's ambient context BEFORE Avalonia initializes, this wrapper is /// what every proxy captures; it delegates to the real context but swallows POST marshal /// failures whose only victim would be a moot native Release (synchronous Send failures still - /// surface β€” the caller is waiting on the result). WinForms will not displace it β€” + /// surface -- the caller is waiting on the result). WinForms will not displace it -- /// InstallIfNeeded only replaces null/base-type contexts, never custom ones. /// public sealed class FinalizerSafeSynchronizationContext : SynchronizationContext @@ -56,7 +57,7 @@ public override void Post(SendOrPostCallback d, object state) } catch (InvalidOperationException e) { - // Marshaling window gone (ObjectDisposedException is a subtype) β€” for a MicroCom + // Marshaling window gone (ObjectDisposedException is a subtype) -- for a MicroCom // finalizer's native Release the posted work is moot; anything else was collateral. ReportSwallowedPost(d, e); } @@ -68,8 +69,10 @@ public override void Post(SendOrPostCallback d, object state) } /// - /// True when the callback is a MicroCom finalizer post (the crash class this wrapper exists - /// for) β€” identified by the callback's declaring type living in the MicroCom runtime. A pin + /// True when the callback is a MicroCom finalizer post (the crash class this wrapper + /// exists + /// for) -- identified by the callback's declaring type living in the MicroCom runtime. A + /// pin /// test guards this namespace assumption so an Avalonia bump that relocates it fails loudly /// instead of silently reclassifying every finalizer Release as a dropped post. /// @@ -103,7 +106,7 @@ private static void ReportSwallowedPost(SendOrPostCallback d, Exception e) // Send is NOT swallowed: the finalizer rationale above only covers Post (MicroCom proxy // finalizers post their native Release). Send is a synchronous call whose caller is - // waiting on the result β€” silently skipping the callback would corrupt that caller's + // waiting on the result -- silently skipping the callback would corrupt that caller's // state, so marshal failures surface to it. public override void Send(SendOrPostCallback d, object state) => _inner.Send(d, state); diff --git a/Src/Common/FwAvalonia/FwAvalonia.csproj b/Src/Common/FwAvalonia/FwAvalonia.csproj index 6d71c91d42..6e7d9d779a 100644 --- a/Src/Common/FwAvalonia/FwAvalonia.csproj +++ b/Src/Common/FwAvalonia/FwAvalonia.csproj @@ -5,7 +5,7 @@ Owning project for the Avalonia lexical-edit migration: typed view-definition IR, migration seams, detail model, and the first-slice controls. Part of the normal repo - build β€” included in the FieldWorks.proj traversal via the Src glob and listed in + build - included in the FieldWorks.proj traversal via the Src glob and listed in FieldWorks.sln. Targets net48 for in-process hosting beside WinForms. Avalonia 11.3.x is the @@ -16,17 +16,14 @@ --> net48 - + FwAvalonia - - + latest false false @@ -58,9 +55,8 @@ - + diff --git a/Src/Common/FwAvalonia/FwAvaloniaDensity.cs b/Src/Common/FwAvalonia/FwAvaloniaDensity.cs index fe4305e230..4b5a590fc2 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaDensity.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaDensity.cs @@ -41,19 +41,23 @@ public static class FwAvaloniaDensity /// The DETERMINISTIC, GLOBAL checkbox glyph-box size (px), a fixed function of the surface font /// (): the box reads about as tall as a capital /// letter, not the Fluent ~20px box on a 32px-tall layout slot. The single - /// restyles the CheckBox TEMPLATE so the LAYOUT footprint (not just the paint) is this size β€” so a + /// restyles the CheckBox TEMPLATE so the LAYOUT footprint (not just the paint) is this + /// size -- so a /// checkbox never inflates a browse/list/tree/table row past the text-row height /// ( = 18). NOT a RenderTransform scale (that leaves the layout box /// tall, the inflation the user rejected); a concrete size applied to the box + the inner template grid. public const double CheckboxBoxSize = 14d; - /// The gap between a checkbox box and its label text, so the words never butt against the box - /// (the deterministic CheckBox template uses this as the boxβ†’label spacing). ~6px reads as a clear gap + /// The gap between a checkbox box and its label text, so the words never butt + /// against the box + /// (the deterministic CheckBox template uses this as the box->label spacing). ~6px reads + /// as a clear gap /// at the surface font size, matching the breathing room a radio button has. public const double CheckboxLabelGap = 6d; /// The DETERMINISTIC, GLOBAL radio-button outer-circle size (px), the radio counterpart of - /// β€” the same 14px so a radio and a checkbox read at the same density and + /// -- the same 14px so a radio and a checkbox read at the + /// same density and /// neither inflates a row past the text line. The single restyles the /// RadioButton TEMPLATE so the LAYOUT footprint (not just the paint) is this size, exactly as /// does for the checkbox box. @@ -61,11 +65,13 @@ public static class FwAvaloniaDensity /// A small amount of visual distance between adjacent logical control GROUPS (e.g. a radio /// group and the checkbox group that follows it in FilterForDialogView), so the groups read as distinct - /// rather than butting together. ~8px of extra top whitespace, optionally paired with a thin grey 1px + /// rather than butting together. ~8px of extra top whitespace, optionally paired with a + /// thin grey 1px /// separator () for the clearest cases. public const double GroupSeparation = 8d; - /// The selected browse/table row fill β€” the legacy pale blue (XmlBrowseViewBaseVc + /// The selected browse/table row fill -- the legacy pale blue + /// (XmlBrowseViewBaseVc /// kclrBackgroundSelRow 0xFFE6D7 = RGB 215,230,255) rather than the Fluent accent, so the whole /// selected row (including the first column) reads as highlighted like the WinForms browse. public static readonly Avalonia.Media.IBrush SelectedRowBrush = @@ -91,12 +97,15 @@ public static class FwAvaloniaDensity /// The 1px rule between slices (DataTree.PaintLinesBetweenSlices, Color.LightGray). public static readonly Avalonia.Media.IBrush SliceRuleBrush = Avalonia.Media.Brushes.LightGray; - /// The thin grid line between browse rows and columns (the legacy XMLViews table draws - /// faint cell separators); a touch lighter than LightGray so the grid reads as structure, not decoration. + /// The thin grid line between browse rows and columns (the legacy XMLViews table + /// draws + /// faint cell separators); a touch lighter than LightGray so the grid reads as structure, + /// not decoration. public static readonly Avalonia.Media.IBrush BrowseGridLineBrush = new Avalonia.Media.SolidColorBrush(Avalonia.Media.Color.FromRgb(0xDC, 0xDC, 0xDC)); - /// The browse table surface fill β€” plain white like the legacy XMLViews browse, rather + /// The browse table surface fill -- plain white like the legacy XMLViews browse, + /// rather /// than the Fluent panel tint. public static readonly Avalonia.Media.IBrush BrowseBackgroundBrush = Avalonia.Media.Brushes.White; @@ -123,7 +132,8 @@ public static class FwAvaloniaDensity /// The text color for the owned pickers, paired with the concrete /// surface. A single named token (rather than an ad-hoc Brushes.Black at each row/item template) so - /// every owned picker shares one foreground and reads legibly dark-on-light β€” matching the concrete-brush + /// every owned picker shares one foreground and reads legibly dark-on-light -- matching + /// the concrete-brush /// convention the rest of the dialog stack paints its WinForms-density surfaces with, so it renders the same in the /// runtime host and the headless tests regardless of the OS theme variant. public static readonly Avalonia.Media.IBrush PickerForegroundBrush = @@ -136,7 +146,8 @@ public static class FwAvaloniaDensity public static readonly Avalonia.Media.IBrush SectionRuleBrush = Avalonia.Media.Brushes.LightGray; /// The horizontal indent applied per hierarchy level in an indented possibility list / POS - /// tree row (the legacy chooser tree's per-depth inset). One source of truth so the tree picker and + /// tree row (the legacy chooser tree's per-depth inset). One source of truth so the tree + /// picker and /// the option picker's depth-indented rows indent identically. public const double TreeIndentPerLevel = 14d; @@ -144,4 +155,4 @@ public static class FwAvaloniaDensity /// collapsed control reads as a field-sized box rather than shrinking to its current text. public const double DropdownMinWidth = 160d; } -} \ No newline at end of file +} diff --git a/Src/Common/FwAvalonia/FwAvaloniaPlatform.cs b/Src/Common/FwAvalonia/FwAvaloniaPlatform.cs index acc15643d1..deea7940fa 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaPlatform.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaPlatform.cs @@ -19,7 +19,8 @@ namespace SIL.FieldWorks.Common.FwAvalonia /// /// Detection is by reflection on the active IWindowingPlatform's assembly (it is /// Avalonia.Headless under the headless platform). This keeps the production FwAvalonia DLL - /// free of any compile-time dependency on Avalonia.Headless β€” production never loads that assembly, + /// free of any compile-time dependency on Avalonia.Headless -- production never loads that + /// assembly, /// so the probe simply returns false and the real Win32 embed path is unchanged. /// internal static class FwAvaloniaPlatform @@ -28,8 +29,10 @@ internal static class FwAvaloniaPlatform /// /// True when the active Avalonia windowing platform is the headless one. False on the real Win32 - /// platform (production) and false if the runtime is not yet initialized or the platform cannot be - /// resolved β€” i.e. it never claims headless unless it can prove it, so production behavior is safe. + /// platform (production) and false if the runtime is not yet initialized or the platform + /// cannot be + /// resolved -- i.e. it never claims headless unless it can prove it, so production + /// behavior is safe. /// internal static bool IsHeadless { @@ -53,7 +56,8 @@ internal static bool IsHeadless /// /// Makes the WinForms/Avalonia embed (the Win32 HWND reparent in /// WinFormsAvaloniaControlHost.OnHandleCreated) a deliberate no-op when the active platform - /// is HEADLESS, by marking as design-mode β€” the control's own escape hatch: + /// is HEADLESS, by marking as design-mode -- the control's own + /// escape hatch: /// its handle-created path skips creating the embeddable root, getting the (nonexistent) Win32 top /// level handle, and calling SetParent/AddMessageFilter when DesignMode is true. /// The Avalonia content still constructs and lays out when shown (tests assert logic, not pixels). diff --git a/Src/Common/FwAvalonia/FwAvaloniaRuntime.cs b/Src/Common/FwAvalonia/FwAvaloniaRuntime.cs index 37070c9aad..ba8b6b5f12 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaRuntime.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaRuntime.cs @@ -20,13 +20,17 @@ public static class FwAvaloniaRuntime /// /// Test-only hook that lets a test assembly substitute the used by - /// β€” without this production DLL referencing Avalonia.Headless. - /// xWorks integration tests that drive the product UI (RecordEditView/RecordBrowseView/…) + /// -- without this production DLL referencing + /// Avalonia.Headless. + /// xWorks integration tests that drive the product UI + /// (RecordEditView/RecordBrowseView/...) /// otherwise initialize the REAL Win32 Avalonia platform process-wide, so any flyout/dialog/popup /// becomes a real on-screen OS window that flashes and can steal keypresses. A test - /// [SetUpFixture] sets this to a headless builder before any test runs; production leaves it + /// [SetUpFixture] sets this to a headless builder before any test runs; production + /// leaves it /// null and behavior is identical to calling directly. - /// Only honored on the first (winning) call β€” once the runtime is + /// Only honored on the first (winning) call -- once the + /// runtime is /// set up it cannot be re-platformed, so this must be set before the first host is constructed. /// public static Func AppBuilderOverride { get; set; } @@ -39,7 +43,8 @@ public static class FwAvaloniaRuntime /// already-live as "initialized": some test hosts (the /// Avalonia.Headless.NUnit [AvaloniaTestApplication] attribute, e.g. /// FwAvaloniaDialogsTests/FwAvaloniaTests) set up the Avalonia platform themselves, - /// per test session, without ever calling this method β€” calling SetupWithoutStarting again + /// per test session, without ever calling this method -- calling + /// SetupWithoutStarting again /// on top of that would throw (Avalonia only allows one Setup per process). /// public static void EnsureInitialized() diff --git a/Src/Common/FwAvalonia/FwAvaloniaStrings.cs b/Src/Common/FwAvalonia/FwAvaloniaStrings.cs index ce1118ac44..e22c10223c 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaStrings.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaStrings.cs @@ -6,7 +6,8 @@ namespace SIL.FieldWorks.Common.FwAvalonia { /// /// Localized product-facing strings for the FwAvalonia module. Strings - /// resolve through ResourceManager over FwAvaloniaStrings.resx β€” the neutral resx is the English + /// resolve through ResourceManager over FwAvaloniaStrings.resx -- the neutral resx is the + /// English /// source of truth and translations ship as satellite assemblies (the FieldWorks .resx /// localization strategy). Automation ids remain nonlocalized constants in code, never resource /// lookups. @@ -62,13 +63,15 @@ public static class FwAvaloniaStrings public static string RedoEditEntry => Text("FwAvalonia.RedoEditEntry"); /// - /// "Undo change to {0}" β€” field-specific undo label for the fenced lexical-edit session when a + /// "Undo change to {0}" -- field-specific undo label for the fenced lexical-edit session + /// when a /// single field's edit opened it ({0} = the field label). Falls back to /// for the batch/bulk path where no single field applies. /// public static string UndoChangeToFormat => Text("FwAvalonia.UndoChangeToFormat"); - /// "Redo change to {0}" β€” the redo counterpart of . + /// "Redo change to {0}" -- the redo counterpart of . public static string RedoChangeToFormat => Text("FwAvalonia.RedoChangeToFormat"); public static string LexemeFormRequired => Text("FwAvalonia.LexemeFormRequired"); @@ -78,7 +81,8 @@ public static class FwAvaloniaStrings /// /// Warning shown when a pending lexical edit is rolled back on navigate/close because it fails - /// validation (the edit is not silently lost β€” the user is told why). {0} is the validation reason(s). + /// validation (the edit is not silently lost -- the user is told why). {0} is the + /// validation reason(s). /// public static string EditDiscardedInvalidFormat => Text("FwAvalonia.EditDiscardedInvalid"); @@ -100,16 +104,18 @@ public static class FwAvaloniaStrings public static string Copy => Text("FwAvalonia.Copy"); - /// "Remove" β€” reference-vector item context command. + /// "Remove" -- reference-vector item context command. public static string Remove => Text("FwAvalonia.Remove"); - /// "Add item" β€” reference-vector add-slot launcher name. + /// "Add item" -- reference-vector add-slot launcher name. public static string AddItem => Text("FwAvalonia.AddItem"); - /// "Type to search" β€” the search-backed add slot's type-ahead watermark. + /// "Type to search" -- the search-backed add slot's type-ahead + /// watermark. public static string SearchPrompt => Text("FwAvalonia.SearchPrompt"); - /// "Add" β€” confirm button of the multi-select reference-vector add picker; commits the checked set in one undoable step. + /// "Add" -- confirm button of the multi-select reference-vector add picker; + /// commits the checked set in one undoable step. public static string AddSelected => Text("FwAvalonia.AddSelected"); /// Accessible name of the "..." dialog-launcher button. @@ -118,27 +124,31 @@ public static class FwAvaloniaStrings /// Tooltip of a disabled launcher button: no host dialog service. public static string LauncherUnavailable => Text("FwAvalonia.LauncherUnavailable"); - /// "{0} settings" β€” accessible name of a chooser's hover-revealed settings gear. + /// "{0} settings" -- accessible name of a chooser's hover-revealed settings + /// gear. public static string FieldSettingsFormat => Text("FwAvalonia.FieldSettings"); /// - /// "Edit the {0} list" β€” label/tooltip of a configure-gear jump derived from the row's - /// possibility list (the legacy chooser dialog's "Edit the … list" link text). + /// "Edit the {0} list" -- label/tooltip of a configure-gear jump derived from the row's + /// possibility list (the legacy chooser dialog's "Edit the ... list" link text). /// public static string EditListFormat => Text("FwAvalonia.EditListFormat"); - /// "Lexeme Form" β€” first-slice row label (compiled override and authored fallback). + /// "Lexeme Form" -- first-slice row label (compiled override and authored + /// fallback). public static string LexemeFormLabel => Text("FwAvalonia.LexemeFormLabel"); - /// "Morph Type" β€” first-slice row label (authored fallback). + /// "Morph Type" -- first-slice row label (authored fallback). public static string MorphTypeLabel => Text("FwAvalonia.MorphTypeLabel"); - /// "Gloss" β€” first-slice row label (authored fallback). + /// "Gloss" -- first-slice row label (authored fallback). public static string GlossLabel => Text("FwAvalonia.GlossLabel"); /// - /// Accessible name / tooltip of the hover-revealed "β‹" field-options button on each field row - /// (opens the Field Visibility / Move Field / Help menu β€” the affordance that replaced right-click). + /// Accessible name / tooltip of the hover-revealed "..." field-options button on each + /// field row + /// (opens the Field Visibility / Move Field / Help menu -- the affordance that replaced + /// right-click). /// public static string FieldOptionsMenu => Text("FwAvalonia.FieldOptionsMenu"); @@ -162,23 +172,30 @@ public static class FwAvaloniaStrings // ----- Delete tab (destructive Delete Rows mode of the legacy Delete tab) ----- // Seed text matches the canonical legacy wording in XMLViewsStrings (ksDeleteRows label uses - // "{0} (Rows)"; ksDelete; ksConfirmDeleteMulti/ksConfirmDeleteMultiMsg) and BulkEditBar's dual-mode + // "{0} (Rows)"; ksDelete; ksConfirmDeleteMulti/ksConfirmDeleteMultiMsg) and BulkEditBar's + // dual-mode // "Delete what?" combo, so the English fallback is identical to the classic bulk-edit Delete tab and // translation memory carries over. APPEND-ONLY: new accessors at the end of the section. // ----- Part-of-Speech chooser (FwPosChooser) ----- - // Seed text mirrors the legacy WinForms POS picker (POSPopupTreeManager / PopupTreeManager): the - // empty node shows "" by default (or "" when the host opts in via the empty-label + // Seed text mirrors the legacy WinForms POS picker (POSPopupTreeManager / + // PopupTreeManager): the + // empty node shows "" by default (or "" when the host opts in via the + // empty-label // override, as MSAGroupBox does), and the inline create affordance is the tree's "More..." item, // reworded to the clearer "Create a new Part of Speech..." for the new view. APPEND-ONLY. - /// "<Not sure>" β€” the default empty / unspecified Part-of-Speech entry (legacy PopupTreeManager "<Not sure>"). + /// "<Not sure>" -- the default empty / unspecified Part-of-Speech entry + /// (legacy PopupTreeManager "<Not sure>"). public static string PosNotSure => Text("FwAvalonia.Pos.NotSure"); - /// "<Any>" β€” the unspecified Part-of-Speech entry when the host treats unspecified as "any" (legacy MSAGroupBox NotSureIsAny). + /// "<Any>" -- the unspecified Part-of-Speech entry when the host treats + /// unspecified as "any" (legacy MSAGroupBox NotSureIsAny). public static string PosAny => Text("FwAvalonia.Pos.Any"); - /// "Create a new Part of Speech..." β€” the inline create affordance at the bottom of the POS tree (legacy "More..." item that launched MasterCategoryListDlg). + /// "Create a new Part of Speech..." -- the inline create affordance at the + /// bottom of the POS tree (legacy "More..." item that launched + /// MasterCategoryListDlg). public static string PosCreateNew => Text("FwAvalonia.Pos.CreateNew"); /// Accessible name of the collapsed Part-of-Speech chooser dropdown. @@ -224,7 +241,8 @@ public static class FwAvaloniaStrings /// /// Accessible name / tooltip of the delete-embedded-object affordance that removes the embedded - /// object (link, picture, footnote, …) under the selection. Any ORC kind is deletable here even + /// object (link, picture, footnote, ...) under the selection. Any ORC kind is deletable + /// here even /// when its insert/edit path lives elsewhere. /// public static string DeleteEmbeddedObject => Text("FwAvalonia.DeleteEmbeddedObject"); diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/AvaloniaDialogHostTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/AvaloniaDialogHostTests.cs index 42aff7773f..5fc21d3f0c 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/AvaloniaDialogHostTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/AvaloniaDialogHostTests.cs @@ -94,14 +94,16 @@ public void CompactDialogStyles_Apply_NullControl_DoesNotThrow() /// /// The UI-thread guard (the one cheaply-testable slice of the otherwise desktop-only modal path): /// must fail fast with - /// when the owner is a WinForms whose InvokeRequired is true β€” i.e. + /// when the owner is a WinForms whose InvokeRequired + /// is true -- i.e. /// the call is on the wrong thread for the owner's message loop. Modal hosting + Avalonia share the /// single WinForms UI thread during coexistence; touching them off that thread is a re-entrancy / /// cross-thread bug, so the guard runs before any windowing. /// /// To make InvokeRequired true deterministically, the owner control's window handle is created - /// on a dedicated worker thread (kept alive for the duration), then ShowModal is invoked from this test - /// thread β€” a different thread than the one that owns the handle. + /// on a dedicated worker thread (kept alive for the duration), then ShowModal is invoked + /// from this test + /// thread -- a different thread than the one that owns the handle. /// [Test] public void ShowModal_OwnerOnAnotherThread_ThrowsInvalidOperation() @@ -143,9 +145,11 @@ public void ShowModal_OwnerOnAnotherThread_ThrowsInvalidOperation() // --- Sizing / min-size / size-persistence. ShowModal itself spins a real // modal loop (not headless-runnable), so these cover the extracted ApplySizing helper that ShowModal - // delegates to: border style, min-size, and the get-hook that seeds the initial (remembered) size. + // delegates to: border style, min-size, and the get-hook that seeds the initial + // (remembered) size. // SANCTIONED EXCEPTION to the no-WinForms-Forms-in-tests rule: these are bare - // `new Form()` property bags β€” no designer tree, never shown, no message loop β€” and the subject + // `new Form()` property bags -- no designer tree, never shown, no message loop -- and the + // subject // under test IS Form property manipulation (frame delta, FixedDialog min-size semantics), which a // fake would untest. App dialogs/designer Forms remain banned; test presenters for those. --- diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/CanonicalJsonTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/CanonicalJsonTests.cs index 1bf1065700..1b18330748 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/CanonicalJsonTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/CanonicalJsonTests.cs @@ -177,8 +177,8 @@ public void EveryViewNodeProperty_SurvivesRoundTrip() } /// - /// User-override-shaped layout XML β€” label/visibility overrides - /// and a hidden part β€” imports with the overrides surfaced in the typed IR. + /// User-override-shaped layout XML -- label/visibility overrides + /// and a hidden part -- imports with the overrides surfaced in the typed IR. /// [TestFixture] public class OverrideFixtureImportTests diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs index eb8c58807f..77d1823574 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailCustomFieldRenderingTests.cs @@ -21,7 +21,7 @@ namespace FwAvaloniaTests /// A row /// renders its plugin control factory's Avalonia control in-tree in the value column, at the /// slice's real position. The path is guarded: a missing, null-returning, or throwing factory - /// degrades to the explicit unsupported row β€” never a crash, never a silently blank row. + /// degrades to the explicit unsupported row -- never a crash, never a silently blank row. /// [TestFixture] public class DetailCustomFieldRenderingTests diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs index 363a60fd5f..d9699f6d4f 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditingTests.cs @@ -50,7 +50,8 @@ public bool TryRemoveReferenceItem(DetailField field, string optionKey) public int CancelCount; /// The text edits actually CAPTURED by a Commit (those staged since the last commit/cancel - /// boundary) β€” models "commit captures staged, cancel discards" so tests can assert WHICH value was + /// boundary) -- models "commit captures staged, cancel discards" so tests can assert + /// WHICH value was /// committed, not merely that a commit happened. public readonly List<(string Field, string Ws, string Value)> CommittedTextEdits = new List<(string, string, string)>(); @@ -123,7 +124,8 @@ public bool TryDeleteParagraph(DetailField field, int paragraphIndex) public void Commit() { - // Capture everything staged since the last boundary β€” that is what this commit "writes". + // Capture everything staged since the last boundary -- that is what this commit + // "writes". for (var i = _stagedBoundary; i < TextEdits.Count; i++) CommittedTextEdits.Add(TextEdits[i]); _stagedBoundary = TextEdits.Count; @@ -132,15 +134,16 @@ public void Commit() public void Cancel() { - // Discard everything staged since the last boundary β€” a cancelled session writes nothing. + // Discard everything staged since the last boundary -- a cancelled session writes + // nothing. _stagedBoundary = TextEdits.Count; CancelCount++; } } /// - /// The detail view drives editing through the edit-context seam β€” staging on - /// text/option change, validation-gated Save, Cancel rollback β€” with stable automation ids. + /// The detail view drives editing through the edit-context seam -- staging on + /// text/option change, validation-gated Save, Cancel rollback -- with stable automation ids. /// [TestFixture] public class DetailEditingViewTests @@ -284,7 +287,8 @@ public void RichTextChange_StagesThroughTheRichEditContext_AndPreservesRunMetada // DATA-SAFETY: a value flagged lossy (a run carries a // TsString property the model does not round-trip) renders a READ-ONLY editor with the - // not-editable-here tooltip, even though an edit context is supplied β€” so a keystroke can + // not-editable-here tooltip, even though an edit context is supplied -- so a keystroke + // can // never silently drop the property. The matching model/composer assertions live in xWorks's // DetailEditContextEditingTests.Compose_RunWithUnsupportedProperty_ComposesReadOnly_*. [AvaloniaTest] @@ -318,7 +322,7 @@ public void RichTextCopy_UsesTheSharedClipboardPayload() var flyout = box.ContextFlyout as MenuFlyout; // The menu also carries the rich-text operations (Link / delete embedded object), - // so Copy is not the sole item β€” pick it out by header. + // so Copy is not the sole item -- pick it out by header. var copyItem = flyout?.Items.OfType() .FirstOrDefault(i => (string)i.Header == FwAvaloniaStrings.Copy); Assert.That(copyItem, Is.Not.Null); @@ -513,17 +517,17 @@ public void TextField_StagesEditsByWsTag_FallingBackToAbbrevWithoutOne() "tag-less rows keep the abbreviation alias"); // The per-row automation id (DetailFocusMemory's focus-restore key) must - // be unique too, so it uses the same tag-preferred key as edits β€” abbreviations collide. + // be unique too, so it uses the same tag-preferred key as edits -- abbreviations + // collide. Assert.That(AutomationProperties.GetAutomationId(boxes[0]), Is.EqualTo("TagField.qaa-x-one"), "a tagged row's automation id keys on the unique IETF tag, not the collidable abbreviation"); Assert.That(AutomationProperties.GetAutomationId(boxes[1]), Is.EqualTo("TagField.du"), "tag-less rows keep the abbreviation-suffixed id"); } - // Voice/sound writing systems: a voice/audio alternative renders as READ-ONLY text (the audio - // filename) with no in-pane player. There are no play/record - // affordances, so the recording can never be corrupted by an edit β€” full audio editing stays - // in the classic view. + // Voice/sound writing systems: a voice/audio alternative renders as READ-ONLY + // text with no in-pane player, so recording can never be corrupted by an edit + // -- full audio editing stays in the classic view. [AvaloniaTest] public void AudioValue_RendersReadOnlyText_WithNoPlayerAndNoStagedEdit() { @@ -708,7 +712,8 @@ public void VectorItemText_HasATransparentBackground_SoTheWholeItemTakesTheRight } // Bug "removing Publish In items not working": a successful remove stage completes the - // gesture β€” the callback (which the view wires to its commit/re-show) fires exactly once. + // gesture -- the callback (which the view wires to its commit/re-show) fires exactly + // once. [AvaloniaTest] public void ReferenceRemove_Success_StagesAndFiresTheGestureCallbackOnce() { @@ -1282,7 +1287,7 @@ public void TryParse_RejectsNonKeys(string input) /// /// GEAR = CONFIGURE: a chooser or reference-vector row whose supporting list /// resolved a list-editor target (a goto ) draws the gear, and - /// clicking it DIRECTLY raises the host's β€” no flyout, no + /// clicking it DIRECTLY raises the host's -- no flyout, no /// context menu. Option flyouts (single-select chooser click, vector "+") are OPTIONS ONLY: /// they contain zero link items. Rows without a resolvable list editor draw no gear; text /// rows NEVER draw one (the Lexeme Form slice menu is right-click only). @@ -1400,7 +1405,7 @@ public void RowsWithoutAResolvableListEditor_HaveNoGear() new FakeDetailEditContext()); Assert.That(noCallback.HoverAffordances, Is.Empty, "no host bridge, no gear"); - // A vector without links: bars + "+" only β€” no Settings button at all. + // A vector without links: bars + "+" only -- no Settings button at all. var vectorField = new DetailField("LexEntry/x/#1", "Publish Entry In", "PublishIn", null, DetailFieldKind.ReferenceVector, EditorClassification.Known, "PlainVector", null, HostRouting.Inherit, null, @@ -1414,8 +1419,8 @@ public void RowsWithoutAResolvableListEditor_HaveNoGear() } // Gears never open context menus: the Lexeme Form text row draws NO gear; its - // slice menu (menu="mnuDataTree-LexemeForm") stays on right-click only β€” the label path in - // the detail view (DetailMenuTests) and the in-string path below are unchanged. + // slice menu (menu="mnuDataTree-LexemeForm") stays on right-click only -- the + // label and in-string paths below are unchanged. [AvaloniaTest] public void TextRows_NeverDrawAGear_TheSliceMenuStaysOnRightClickOnly() { diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditorParityTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditorParityTests.cs index 0fb0ddb6d6..dc808747da 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditorParityTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailEditorParityTests.cs @@ -21,7 +21,7 @@ namespace FwAvaloniaTests /// Editor-type parity for the lexical detail view: /// the importer carries an enumComboBox's stringList ids/group onto the node (the metadata /// survives even though the detail view does not render a closed enum combo); - /// FwReferenceVectorField.Dispose detaches every handler it wired (count >0 β†’ 0). + /// FwReferenceVectorField.Dispose detaches every handler it wired (count >0 -> 0). /// [TestFixture] public class DetailEditorParityTests @@ -103,7 +103,8 @@ public void ReferenceVector_Dispose_DetachesEveryHandler() [AvaloniaTest] public void ReferenceVector_ReadOnly_HasNothingToDetach() { - // A read-only vector (no edit context) wires no edit handlers, so its teardown is empty β€” + // A read-only vector (no edit context) wires no edit handlers, so its teardown is + // empty -- // Dispose is a safe no-op. var vector = new FwReferenceVectorField(VectorFieldWithItems(), "PublishIn", editContext: null); Assert.That(vector.AttachedHandlerCount, Is.EqualTo(0)); diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailFocusMemoryTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailFocusMemoryTests.cs index 492a5a2f9e..67763d52f3 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailFocusMemoryTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailFocusMemoryTests.cs @@ -18,7 +18,7 @@ namespace FwAvaloniaTests /// /// Focus continuity across detail-view re-shows (14.4 usability): the host replaces the entire view /// after every committed edit, so the focused editor (identified by its stable automation id) - /// and caret must carry over to the rebuilt view β€” otherwise tabbing out of a field would + /// and caret must carry over to the rebuilt view -- otherwise tabbing out of a field would /// destroy the editor the user just moved into. /// [TestFixture] @@ -182,9 +182,9 @@ public void CaptureAndRestore_CarryScrollOffset_AcrossAViewRebuild() "rebuilding the detail view should keep the user at the same scroll position instead of jumping back to the top"); } - // A single-text-field view whose editor's stable automation id is exactly - // + ".vern" (null AutomationId falls back to StableId; the WS suffix is the WsTag). This lets the - // test reproduce the ghost id ("…@ownerHvo/ghost.vern") and its real successor ("…@newHvo.vern"). + // A single-text-field view's editor automation id is exactly + // + ".vern", reproducing the ghost id + // ("...@ownerHvo/ghost.vern") and real successor ("...@newHvo.vern"). private static DataTree ViewWithEditorId(string stableId) { var field = new DetailField(stableId, "Lexeme Form", "Form", "vernacular", diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailMenuTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailMenuTests.cs index 9f5dd5d7f9..51acaa70bc 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/DetailMenuTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/DetailMenuTests.cs @@ -155,9 +155,9 @@ private static T FindOrNull(Visual view, string automationId) where T : Visua => view.GetVisualDescendants().OfType() .FirstOrDefault(c => AutomationProperties.GetAutomationId(c) == automationId); - // The field-options "β‹" affordance opens the menu on a click OR keyboard activation; both arrive - // as Button.Click, so raising it exercises the same path the icon does (no hit-test dependence on - // the hover-reveal opacity state). + // The field-options "..." affordance opens the menu on a click OR + // keyboard activation; both arrive as Button.Click, so raising it exercises + // the icon's own path, without depending on hover-reveal opacity. private static void ClickKebab(Button kebab) { kebab.RaiseEvent(new RoutedEventArgs { RoutedEvent = Button.ClickEvent }); @@ -169,7 +169,7 @@ public void FieldMenuButton_OnLabelRow_RaisesTheSliceMenuRequest_WithTheLegacyMe { var (view, requests) = Show(Field("Gloss", DetailFieldKind.Text, menuId: "mnuDataTree-Help")); - // The "β‹" field-options button (which replaced right-click) opens the slice menu. + // The "..." field-options button (which replaced right-click) opens the slice menu. ClickKebab(Find [TestFixture] public class FinalizerSafeSynchronizationContextTests @@ -266,7 +266,8 @@ public void Analyze_SameSide_DoesNotPrompt() /// FwAvaloniaPlatform.IsHeadless resolves Avalonia internals BY STRING NAME (AvaloniaLocator in /// Avalonia.Base; IWindowingPlatform in Avalonia.Controls; AvaloniaLocator.Current + its GetService). /// Unlike a public API, a version bump can relocate these without a compile break, which would leave - /// the reflection returning null forever β€” silently reporting "not headless" and disabling the + /// the reflection returning null forever -- silently reporting "not headless" and disabling + /// the /// headless-embed no-op for the WHOLE suite. This pins each target against the referenced Avalonia, /// failing loudly (mirroring the MicroCom pin above) so a bump forces an FwAvaloniaPlatform update. /// diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/SliceFactoryTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/SliceFactoryTests.cs index b0ad71f9af..44de538ece 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/SliceFactoryTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/SliceFactoryTests.cs @@ -12,12 +12,15 @@ namespace FwAvaloniaTests { /// - /// The shared β†’control dispatch both the detail-pane detail view + /// The shared ->control dispatch both the detail-pane detail + /// view /// and the browse in-cell editor route through. These pin that one switch produces the right control - /// per surviving kind (Text / Chooser / ReferenceVector / Literal / Custom / Unsupported), and that - /// the all-nullable serves both hosts β€” the browse cell - /// passes null menu/link callbacks and suppresses the WS-abbreviation gutter while the detail pane - /// passes the full set β€” without either host hand-rolling its own dispatch. + /// per surviving kind (Text / Chooser / ReferenceVector / Literal / Custom / Unsupported), + /// and that + /// the all-nullable serves both hosts -- the browse cell + /// passes null menu/link callbacks and suppresses the WS-abbreviation gutter while the detail + /// pane + /// passes the full set -- without either host hand-rolling its own dispatch. /// [TestFixture] public class SliceFactoryTests @@ -51,7 +54,7 @@ public void UnsupportedKind_BuildsUnsupportedTextBlock() => Assert.That(SliceFactory.Build(Field(DetailFieldKind.Unsupported), "Auto.Id", null), Is.InstanceOf()); - // Literal: a static text renderer (legacy MessageSlice) β€” the label/message text is the + // Literal: a static text renderer (legacy MessageSlice) -- the label/message text is the // content, no editable value column. [AvaloniaTest] public void LiteralKind_BuildsStaticTextBlock_ShowingTheLabel() @@ -84,8 +87,10 @@ public void CustomKind_FactoryControl_IsReturned() [AvaloniaTest] public void BrowseStyleContext_TextField_SuppressesWritingSystemAbbreviation() { - // The dense browse cell context (null callbacks, no abbreviation gutter) must still build a - // usable text field β€” the same control the detail pane gets, just configured for the cell. + // The dense browse cell context (null callbacks, no abbreviation gutter) must still + // build a + // usable text field -- the same control the detail pane gets, just configured for the + // cell. var browseContext = new SliceFactoryContext( editContext: null, writingSystemFocused: _ => { }, showWritingSystemAbbreviation: false); Assert.That(SliceFactory.Build(Field(DetailFieldKind.Text), "Auto.Id", browseContext), diff --git a/Src/Common/FwAvalonia/FwAvaloniaTests/StructuredTextEdgeCaseTests.cs b/Src/Common/FwAvalonia/FwAvaloniaTests/StructuredTextEdgeCaseTests.cs index 94b8b4a59d..80c8334a67 100644 --- a/Src/Common/FwAvalonia/FwAvaloniaTests/StructuredTextEdgeCaseTests.cs +++ b/Src/Common/FwAvalonia/FwAvaloniaTests/StructuredTextEdgeCaseTests.cs @@ -24,7 +24,8 @@ namespace FwAvaloniaTests /// Edge cases for the owned multi-paragraph structured-text (StText) editor /// (): an empty StText (zero / one paragraph), the /// only-paragraph-cannot-delete invariant, RTL + complex-script (Khmer) content round-tripping, - /// rapid interleaved insert/delete, ORC/lossy interleaving, and clear-style β†’ Normal mapping. + /// rapid interleaved insert/delete, ORC/lossy interleaving, and clear-style -> Normal + /// mapping. /// These pin the corners the happy-path unit tests don't reach. The view side stays LCModel-free /// (a recording fake context); the matching real-LCModel round-trip assertions live in /// StructuredTextAdapterTests / StructuredTextWorkflowTests. @@ -108,7 +109,8 @@ public void EmptyStText_FirstKeystrokeMaterializesParagraph_StagesAtIndex0() var context = new FakeDetailEditContext(); var (control, _) = Show(field, context, gestureCompleted: () => { }); - // Typing into the lone empty row stages a text edit against paragraph index 0 β€” the seam the + // Typing into the lone empty row stages a text edit against paragraph index 0 -- the + // seam the // composer's text setter turns into "create paragraphs up to the index" against a null StText. Boxes(control)[0].Text = "first words"; Dispatcher.UIThread.RunJobs(); @@ -183,9 +185,10 @@ public void RtlAndComplexScript_ParagraphStagesAndRoundTrips_LosslessRuns() [AvaloniaTest] public void RapidInterleavedInsertDelete_DoNotCrashOrOrphanUndo() { - // Each structural gesture completes immediately (the gestureCompleted callback the host wires - // to its one validation-gated commit + re-show). Interleaving them rapidly must remain - // one-completed-gesture-per-action β€” no missed or doubled completion (which would orphan undo). + // Each structural gesture completes via gestureCompleted (its + // validation-gated commit + re-show). Interleaving must stay + // one-completed-gesture-per-action -- missing or doubled completion + // orphans undo. var field = Field(new List { Para("Alpha."), Para("Beta."), Para("Gamma.") }); var context = new FakeDetailEditContext(); var gestures = 0; @@ -194,7 +197,8 @@ public void RapidInterleavedInsertDelete_DoNotCrashOrOrphanUndo() Button AddButton(int i) => control.GetVisualDescendants().OfType