diff --git a/.github/commit-guidelines.md b/.github/commit-guidelines.md index 41ba41b86e..3a988ce1b3 100644 --- a/.github/commit-guidelines.md +++ b/.github/commit-guidelines.md @@ -15,6 +15,7 @@ These align with the gitlint rules run in CI. - Wrap lines at 80 characters. - Explain what and why over how; link issues like "Fixes #1234" when applicable. - No hard tabs, no trailing whitespace. +- The `Agent-Logs-Url:` footer generated by agent tooling is ignored by gitlint. ## Helpful commands (Windows PowerShell) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..5cc5108cd5 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,42 @@ +# FieldWorks Copilot Code Review Instructions + +## Purpose + +Review FieldWorks pull requests for the project-specific risks most likely to +cause regressions in this Windows/x64, .NET Framework 4.8, native/managed +desktop application. + +## Highest-priority checks + +- Verify changes preserve the native-before-managed build ordering enforced by + `build.ps1`, `FieldWorks.proj`, and related targets. +- Flag changes that introduce global COM registration or registry hacks; + FieldWorks uses registration-free COM. +- Check all native, COM, file-system, installer, and process-boundary inputs for + validation, encoding assumptions, buffer lengths, and ownership rules. +- Verify user-visible UI strings are placed in `.resx` resources and remain + compatible with Crowdin localization. +- For C# code, flag C# 8+ syntax and APIs that are not compatible with .NET + Framework 4.8 / C# 7.3. +- For legacy `.csproj` changes, verify new source files are explicitly included + and test sources are explicitly excluded where production and test trees mix. +- Require meaningful test or validation evidence for bug fixes, regressions, + parser changes, interlinear display changes, installer changes, and build + workflow changes. + +## Review focus from recent history + +- Recent churn is concentrated in `Src/Utilities`, parser utilities, Allomorph + Generator, Interlinear display, build scripts, and installer workflows. +- Recent bugs include click-handler crashes, stale UI state, layout/display + regressions, media-line rework, localization gaps, installer/build fixes, and + dependency-version alignment. +- When those areas change, review null checks, collection bounds, selection + state, refresh/invalidation behavior, layout anchoring, localization, and + regression-test coverage before style-only concerns. + +## Review style + +- Prefer concise, actionable comments that explain the FieldWorks-specific risk. +- Do not request unrelated refactors. +- Do not rely on external links; cite the relevant rule or file path directly. diff --git a/.github/instructions/fieldworks-build-installer-review.instructions.md b/.github/instructions/fieldworks-build-installer-review.instructions.md new file mode 100644 index 0000000000..9e862e909b --- /dev/null +++ b/.github/instructions/fieldworks-build-installer-review.instructions.md @@ -0,0 +1,45 @@ +--- +applyTo: ".github/workflows/**,Build/**,scripts/**,*.ps1,*.proj,*.targets,*.props,Directory.Packages.props,FLExInstaller/**" +name: "fieldworks-build-installer-review" +description: "Copilot code review checks for FieldWorks build, CI, dependency, installer, and PowerShell changes" +--- + +# Build, CI, Dependency, and Installer Review Checks + +## Purpose + +Use these checks for build scripts, CI workflows, dependency versions, MSBuild +targets, installer inputs, and PowerShell helper changes. + +## Build and CI integrity + +- Preserve native-before-managed ordering and do not bypass `build.ps1` or + `test.ps1` for normal validation. +- Verify build/test process cleanup remains scoped to the current worktree and + does not terminate processes from other worktrees. +- For workflow changes, verify tests run before installer packaging when that + ordering affects release confidence. +- Flag hardcoded absolute paths, machine-specific assumptions, and hidden + dependency downloads that are not represented in repo scripts. + +## Dependency alignment + +- Version changes in `Directory.Packages.props`, `Build/SilVersions.props`, + app configs, binding redirects, and local-library scripts should agree. +- Dependency updates should explain compatibility with .NET Framework 4.8, + native x64 requirements, and existing installer/runtime packaging. + +## PowerShell and script safety + +- Prefer repo wrapper scripts over ad-hoc command pipelines. +- Check error handling, exit-code propagation, quoting of paths with spaces, and + deterministic evidence output. +- Avoid name-based process termination; use specific process IDs when stopping a + process is required. + +## Installer evidence + +- WiX and installer changes should include validation evidence for bundle/MSI + build, install/upgrade/uninstall behavior, logs, and expected footprint. +- Preserve both WiX 3 and WiX 6 flows unless the change explicitly scopes one + toolset. diff --git a/.github/instructions/fieldworks-interop-review.instructions.md b/.github/instructions/fieldworks-interop-review.instructions.md new file mode 100644 index 0000000000..96c26bfac7 --- /dev/null +++ b/.github/instructions/fieldworks-interop-review.instructions.md @@ -0,0 +1,39 @@ +--- +applyTo: "Src/Common/ViewsInterfaces/**,Src/views/**,Src/Generic/**,Src/Kernel/**,**/*.{cpp,h,hpp,ixx,def}" +name: "fieldworks-interop-review" +description: "Copilot code review checks for native, C++/CLI, COM, and managed/native boundary changes" +--- + +# Interop and Native Boundary Review Checks + +## Purpose + +Use these checks for native code, C++/CLI-adjacent code, COM interfaces, +ViewsInterfaces, Generic, Kernel, and managed/native boundary changes. + +## Boundary safety + +- Validate untrusted input before it crosses native, COM, file-system, or process + boundaries. +- Check string encoding conversions, buffer lengths, array counts, pointer + ownership, lifetime, and nullability on every changed boundary. +- Avoid throwing exceptions across managed/native boundaries; translate failures + into the existing error-reporting pattern. +- Verify SAL annotations, checked APIs, RAII, and deterministic cleanup are used + where applicable. + +## COM and ABI stability + +- Flag COM GUID, interface layout, vtable order, manifest, or registration-free + COM changes for explicit compatibility review. +- Do not introduce global COM registration, registry hacks, or assumptions that + require elevated machine-wide state. +- Managed wrapper changes should stay consistent with the native contract and + include tests or validation that exercise the boundary. + +## Build and diagnostics + +- Native changes must not hide compiler or MSBuild warnings; fix warnings rather + than suppressing them casually. +- Boundary failures should produce actionable diagnostics without leaking + sensitive data or overwhelming normal logs. diff --git a/.github/instructions/fieldworks-parser-utilities-review.instructions.md b/.github/instructions/fieldworks-parser-utilities-review.instructions.md new file mode 100644 index 0000000000..29099cd4f6 --- /dev/null +++ b/.github/instructions/fieldworks-parser-utilities-review.instructions.md @@ -0,0 +1,40 @@ +--- +applyTo: "Src/Utilities/pcpatrflex/**,Src/Utilities/AlloVarGen/**,Src/LexText/ParserCore/**,Src/LexText/Morphology/**,Src/LexText/Interlinear/**" +name: "fieldworks-parser-utilities-review" +description: "Copilot code review checks for parser, morphology, interlinear, PCPATR, and Allomorph Generator changes" +--- + +# Parser and Linguistic Utilities Review Checks + +## Purpose + +Use these checks for parser utilities, morphology, interlinear analysis, PCPATR, +TonePars, Allomorph Generator, and related test data. + +## Correctness and regression coverage + +- Parser, morphology, grammar, or transform changes should include acceptance + examples or regression tests that show the intended linguistic behavior. +- For media-line, interlinear display, stale filtering, and preview-pane changes, + verify the changed model state triggers display refresh and persistence updates. +- When large `.fwdata`, grammar, XML, or expected-output files change, verify the + diff is intentional and tied to a specific parser or fixture behavior. +- Flag changes that alter matching, regex, grammar, or word-analysis behavior + without edge-case tests for empty values, multiple writing systems, and + ambiguous analyses. + +## Allomorph and variant generator safety + +- Dialog and chooser changes must handle empty collections, deleted items, + missing writing systems, and null cast results. +- Selection state should be restored by stable identifiers or names when object + instances can be recreated. +- New UI text or documentation for these utilities should use resources and + remain localizable. + +## Test data discipline + +- Keep fixture updates scoped: do not refresh broad expected-output files unless + the changed behavior requires it. +- Review paired before/after fixture files together and verify they remain + consistent. diff --git a/.github/instructions/fieldworks-ui-review.instructions.md b/.github/instructions/fieldworks-ui-review.instructions.md new file mode 100644 index 0000000000..71a5dfedc7 --- /dev/null +++ b/.github/instructions/fieldworks-ui-review.instructions.md @@ -0,0 +1,45 @@ +--- +applyTo: "Src/**/*.{cs,resx}" +name: "fieldworks-ui-review" +description: "Copilot code review checks for FieldWorks WinForms, xWorks, interlinear display, and localization changes" +--- + +# FieldWorks UI Review Checks + +## Purpose + +Use these checks for managed UI, WinForms dialogs, xWorks, interlinear display, +and resource changes. + +## Crash prevention + +- In event handlers and dialog callbacks, verify `sender`, selected items, + selected indices, model objects, and cast results are checked before use. +- Flag `Items[index]`, `SelectedItems[0]`, `SelectedIndex`, or `as Type` casts + when bounds or null checks are missing. +- Verify deleted or replaced model objects cannot remain in chooser lists, + cached selections, mediator state, or reference collections. +- Check that UI updates from background work marshal back to the UI thread. + +## Display and layout regressions + +- For interlinear, preview pane, dictionary, media-line, and tree/list display + changes, verify refresh/invalidation and persistence behavior are covered. +- Review `TableLayoutPanel`, `FlowLayoutPanel`, anchoring, docking, minimum size, + tab order, and resizable dialog behavior when layout files change. +- Flag display logic changes without corresponding tests or explicit manual + validation notes for the affected user workflow. + +## Localization and resources + +- User-visible strings belong in `.resx` resources, not hardcoded in C#. +- Resource keys should be stable, descriptive, and reused consistently between + code, designer files, and localized resources. +- When `.resx` files change, verify designer/resource accessors stay in sync. + +## Designer safety + +- Keep generated initialization in designer files and application logic in the + non-designer partial class. +- Flag manual designer edits that remove disposal, component initialization, or + resource manager usage. diff --git a/.gitlint b/.gitlint new file mode 100644 index 0000000000..228fc2a76b --- /dev/null +++ b/.gitlint @@ -0,0 +1,6 @@ +[general] +regex-style-search=true + +[ignore-by-body] +regex=^Agent-Logs-Url: +ignore=B1 diff --git a/Build/Agent/check-whitespace.ps1 b/Build/Agent/check-whitespace.ps1 index dcfd38ea5d..1e7f2b2c14 100644 --- a/Build/Agent/check-whitespace.ps1 +++ b/Build/Agent/check-whitespace.ps1 @@ -83,7 +83,7 @@ Get-Content check-results.log | ForEach-Object { } if ($problems.Count -gt 0) { - Write-Host "`u26A0`uFE0F Please review the output for further information." + Write-Host '[WARN] Please review the output for further information.' Write-Host '### A whitespace issue was found in one or more of the commits.' Write-Host 'This check validates commit history from origin/main..HEAD, not just the current working tree.' Write-Host 'If the report names an older commit, fix the file and then amend, squash, or rebase so that commit no longer appears in the branch history.' diff --git a/Build/Agent/fix-whitespace.ps1 b/Build/Agent/fix-whitespace.ps1 index 324a254a2f..1933489889 100644 --- a/Build/Agent/fix-whitespace.ps1 +++ b/Build/Agent/fix-whitespace.ps1 @@ -78,7 +78,7 @@ if (Test-Path -LiteralPath 'check-results.log') { if (-not $fixFiles -or $fixFiles.Count -eq 0) { $base = Get-BaseRef Write-Host "Fixing whitespace for files changed since $base..HEAD" - $fixFiles = git diff --name-only "$base"..HEAD + $fixFiles = git diff --name-only "$base..HEAD" } $files = $fixFiles | Where-Object { $_ -and (Test-Path $_) }