From 5adf2af8ec468c51fc0922012134daabdd46c35b Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 18:47:48 +1000 Subject: [PATCH 01/13] [patch] Design the patch verbs: reading hunks, applying them, unstaging Diff answers which files changed and by how much, with no patch text and no hunks, so a caller can neither draw a diff nor stage part of a file. There is no apply, so no patch reaches the index, and no reset or restore, so nothing staged can be unstaged. Three verbs: Patch reads a unified diff into files, hunks and lines, keeping each hunk's text verbatim because a regenerated one loses the no-newline marker and is rejected on apply. Apply takes patch text and can target the index, reverse itself, or check without changing anything. Unstage covers the whole-file case, probing for restore and falling back to reset the way Fetch already probes for porcelain. The reader always passes --no-ext-diff and --no-textconv. A repository with a gitattributes diff driver emits human-readable output instead of a patch, so without them the feature would fail on exactly the repositories that configure one. --- .../specs/2026-09-24-patch-verbs-design.md | 220 ++++++++++++++++++ 1 file changed, 220 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-24-patch-verbs-design.md diff --git a/docs/superpowers/specs/2026-09-24-patch-verbs-design.md b/docs/superpowers/specs/2026-09-24-patch-verbs-design.md new file mode 100644 index 0000000..9e23185 --- /dev/null +++ b/docs/superpowers/specs/2026-09-24-patch-verbs-design.md @@ -0,0 +1,220 @@ +# Patch verbs: reading hunks, applying them, and unstaging + +Status: approved design, not yet implemented. + +## Why this exists + +A caller that wants to show a diff, or stage part of a file, cannot do either through this library +today. + +`Diff()` answers "which files changed, and by how many lines". It runs `--name-status`, or +`--raw --numstat` when line counts are asked for, and parses one `GitDiffEntry` per file carrying a +path, a change kind and two counts. There is no patch text and no hunks, so there is nothing to draw +and nothing to slice. + +There is no `apply`, so no patch can reach the index. There is no `reset` or `restore`, so nothing +that has been staged can be unstaged. A caller that needs any of this has to shell out to git itself, +which is the thing this library exists to prevent. + +The immediate consumer is a launcher's source control tab wanting hunk-level staging, but nothing +here is specific to it. These are three git verbs this library does not yet wrap. + +## Scope + +In scope: + +- A patch reader: `Patch()`, returning files, their hunks, and each hunk's lines. +- Assembling a chosen subset of hunks into text `git apply` accepts. +- `Apply()`, with the options that make staging and unstaging a hunk possible. +- `Unstage()`, for the whole-file case. + +Out of scope, deliberately: + +- Computing a diff in managed code. Git produces the patch, and this library reads it. +- Conflict resolution, three-way application, and merge tooling. +- Binary patch application. Binary files are reported and staged whole-file. +- A staging facade. `Stage(hunk)` is a wrapper a caller can write in a few lines over `Apply`, and + putting it here would mean this library deciding what staging means, which nothing else in it does. + +## The patch model + +``` +GitPatch Files +GitFilePatch Path, OriginalPath, Kind, IsBinary, IsConflicted, Header, Hunks +GitHunk OldStart, OldCount, NewStart, NewCount, Heading, Lines, Text +GitPatchLine Kind, Text, OldNumber, NewNumber +``` + +`Kind` on the file reuses the existing `GitChangeKind`. Only one new enum is needed, +`GitPatchLineKind`, with `Context`, `Added` and `Removed`. + +`Lines` is the structured form, for a caller drawing the diff. `Text` is the hunk exactly as git +emitted it, and `Header` is the file's header lines. Both are kept because they serve different +jobs, and because one cannot be derived from the other safely. + +### Why hunk text is kept verbatim + +Regenerating a hunk from its parsed lines loses `\ No newline at end of file`, which is a real line +in git's output and carries no leading space, plus or minus. A patch missing it is rejected on apply +for any file that lacks a trailing newline. Keeping git's own bytes is what makes the round trip +hold, and the alternative is a parser that must reproduce every marker git might emit, forever. + +### Assembling a patch from chosen hunks + +`GitFilePatch.PatchFor(IEnumerable hunks)` concatenates the file's header with the text of +the hunks given, in file order, and returns something `apply` accepts. + +Selecting a subset is safe because each hunk's `@@` positions it against the original file, which is +what the index still holds. It stops being safe once the index has moved, which is what `Checked()` +below is for. + +A hunk from one `GitFilePatch` must not be handed to another's `PatchFor`. The method takes the +hunks it is given and does not verify they came from it, because the check would cost a reference +comparison per hunk to catch a caller error that no reasonable caller makes. + +## Reading a patch + +`Patch()` returns `GitPatchBuilder`, producing a `GitPatch`. + +``` +Staged() --cached, the patch of what is already staged +WithContext(int lines) -U +ForPath(path) -- , repeatable +Against(ref) one revision +Between(a, b) two revisions +DetectRenames() --find-renames +``` + +The vector always carries `--no-ext-diff`, `--no-textconv` and `--no-color`, and never `-z`. + +`--no-ext-diff` and `--no-textconv` are correctness, not tidiness. A repository with a `.gitattributes` +diff driver produces human-readable output in place of a patch, so a hunk from such a file would be +unapplyable. Game repositories commonly configure these for binary asset formats, which means the +feature would work everywhere except the repositories most likely to need it. `--no-color` guards +against a user config setting `color.diff = always`, which would inject escape sequences into text +that is handed back to `apply`. + +`-z` is absent because the patch format is line-based. It changes only the name-status framing that +this builder does not use. + +## Applying a patch + +`Apply(string patchText)` returns `GitApplyBuilder`, producing `GitCompleted`. + +``` +ToIndex() --cached +Reversed() --reverse +Checked() --check +``` + +Staging a hunk is `Apply(text).ToIndex()`. Unstaging one is `Apply(text).ToIndex().Reversed()`. + +The vector also carries `--whitespace=nowarn`. Git otherwise honours `apply.whitespace`, and a user +who has set it to `error` would find that staging fails on trailing whitespace already present in +their own working tree. Staging existing content is not the moment to enforce a whitespace policy, +and the content reaching the index is identical either way. + +### The temporary file + +Git reads a patch from standard input or from a file. `GitProcessRequest` carries an argument vector +and a progress sink and has no standard input, and the upstream work it anticipates is about working +directories and environment variables rather than input. So `Apply` writes the patch to a temporary +file, passes that path, and deletes it in a `finally`. + +The file is written as UTF-8 with no byte order mark, with the text's line endings preserved exactly. +A patch is byte-sensitive: a rewritten line ending or an inserted mark makes it unapplyable. + +That the file exists at all is this builder's business. A caller has no reason to know that standard +input was unavailable, and if it later becomes available this changes without touching the surface. + +### Checking before applying + +`Checked()` emits `--check`, which reports whether the patch would apply and changes nothing. + +It needs no result type: `TryExecuteAsync` already reports failure without throwing, so +`Checked().TryExecuteAsync()` answers "would this still apply?" as `Success`. A caller that checks +before staging turns a worktree that moved underneath into a refusal rather than a half-staged file. + +### Errors are git's own + +`Apply` is the first verb here that takes caller-supplied content rather than caller-supplied +arguments. There is no option-injection surface, because the patch reaches git through a file path +this library controls, but it does mean a malformed patch fails at runtime and git's message is the +only explanation anyone gets. The failure therefore carries git's standard error verbatim rather than +a summary of it. + +## Unstaging + +`Unstage(path)` returns `GitRestoreBuilder`, producing `GitCompleted`, and emits +`restore --staged -- `. + +`git restore` arrived in 2.23. Rather than declaring a floor, this probes the installed version and +emits `reset HEAD -- ` below it, the same shape `Fetch` already uses for `--porcelain`. Two +verbs, one meaning, chosen by what is installed. + +This is the whole-file case only. Unstaging a hunk is `Apply(...).ToIndex().Reversed()`, and a caller +wanting to unstage a binary file has nothing else, since a binary file has no hunks. + +## What a patch cannot express + +Four cases where the model reports rather than pretends: + +**Conflicted files.** An unmerged path produces combined format, with `@@@` and two columns, which is +not an applyable patch. The parser recognises it and sets `IsConflicted`, leaving `Hunks` empty. A +caller stages such a file whole or not at all. Mis-parsing combined hunks into ordinary ones would +produce patches git rejects, with nothing on screen explaining why. + +**Binary files.** Git emits either a one-line notice or an encoded blob. `IsBinary` is set and +`Hunks` is empty. + +**Renames and mode changes.** Both produce a file entry with headers and no hunks. Nothing special is +needed beyond not assuming every file has them. + +**Untracked files.** They never appear in a patch, because `git diff` does not show them. Staging one +is `Add`, and a caller building a staging view has to know that its file list comes from `Status` and +its hunks come from `Patch`, which are different sources that disagree about untracked content. + +## Testing + +Parser tests run against fixtures captured from a real git, following this repository's existing +practice of capturing output once and treating it as fixed rather than re-deriving it. Fixtures +cover: a modification with two hunks, a file with no trailing newline, a rename with no hunks, a +binary file, a combined conflict hunk, and content with CRLF line endings. + +The test that matters most is the round trip, against a real repository in a temporary directory: +generate a patch, take one hunk of a two-hunk file through `PatchFor`, apply it to the index, and +assert the staged and unstaged halves split exactly. A parser that drops a byte passes every unit +test and fails this one. + +Beside it, `Checked()` against a working tree that changed after the patch was generated, asserting +it reports failure and leaves the index untouched. That is the guarantee a caller's refusal depends +on. + +The version fallback in `Unstage` is tested only if the probe can be driven without a second git +installation. If it cannot, the spec says so rather than asserting against a mock that proves the +mock. + +## File layout + +| File | Responsibility | +|---|---| +| `GitIntegration/Models/GitPatch.cs` | `GitPatch`, `GitFilePatch`, `GitHunk`, `GitPatchLine`, and `PatchFor` | +| `GitIntegration/Parsing/GitPatchParser.cs` | Unified diff to model | +| `GitIntegration/Builders/GitPatchBuilder.cs` | `Patch()` | +| `GitIntegration/Builders/GitApplyBuilder.cs` | `Apply()`, including the temporary file | +| `GitIntegration/Builders/GitRestoreBuilder.cs` | `Unstage()`, including the version fallback | + +Modified: `GitRepository` gains `Patch()`, `Apply()` and `Unstage()`. `GitEnums` gains +`GitPatchLineKind`. + +## Implementation order + +1. The model and `PatchFor`, with no parser and no git. The assembly rule is the one piece of logic + here that is pure, and it is worth having tested before anything produces input for it. +2. `GitPatchParser` against the captured fixtures. +3. `GitPatchBuilder`, which is then the first thing that runs git. +4. `GitApplyBuilder`, and with it the round-trip test that proves steps 1 to 3 preserved every byte. +5. `GitRestoreBuilder` and its version fallback. + +Steps 1 and 2 need no git at all. Step 4 is where a mistake anywhere earlier becomes visible, which +is why the round trip belongs there rather than at the end. From 2e67b164c21dbc6914e4ff0394b668520a3430e9 Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 19:08:36 +1000 Subject: [PATCH 02/13] [patch] Plan the patch verbs implementation Five tasks in the spec's order: the model and hunk assembly with no git at all, the parser against captured fixtures, the reader, apply with the round-trip test that proves every byte survived, and unstage with its version fallback. The round trip sits at task four rather than the end, because that is where a byte lost in the model or the parser becomes visible, and a hand-written fixture would pass every parser test and fail there. --- .../plans/2026-09-24-patch-verbs.md | 797 ++++++++++++++++++ 1 file changed, 797 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-24-patch-verbs.md diff --git a/docs/superpowers/plans/2026-09-24-patch-verbs.md b/docs/superpowers/plans/2026-09-24-patch-verbs.md new file mode 100644 index 0000000..b8caeb6 --- /dev/null +++ b/docs/superpowers/plans/2026-09-24-patch-verbs.md @@ -0,0 +1,797 @@ +# Patch verbs implementation plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Give this library the three git verbs a caller needs to show a diff and stage part of a file: reading a patch into hunks, applying one to the index, and unstaging. + +**Architecture:** A model that keeps git's own hunk bytes alongside a parsed form, a parser that fills it, and three builders in the same shape as every other verb here. Assembling chosen hunks into an applyable patch is pure logic in the model, so it is tested without running git. + +**Tech Stack:** C#, net10.0 and net9.0, MSTest 4.3.3 through MSTest.Sdk, `ktsu.Semantics.Paths` and `ktsu.Semantics.Strings`. + +**Spec:** `docs/superpowers/specs/2026-09-24-patch-verbs-design.md` + +## Global Constraints + +Every task's requirements implicitly include this section. + +- Copyright header on every new file, exactly: `// Copyright (c) 2023-2026 ktsu-dev contributors` +- Namespaces: `ktsu.GitIntegration` for library files, `ktsu.GitIntegration.Test` for tests. File-scoped, with every `using` **inside** the namespace declaration. +- Indent with tabs, never spaces. +- US spelling in identifiers, comments and user-facing strings. +- **In the library**, null guards are `Ensure.NotNull(x)` from Polyfill. +- **In the test project**, they are `ArgumentNullException.ThrowIfNull(x)`. Polyfill is referenced with `PrivateAssets="all"`, so `Ensure` is not visible there. This is the reverse of the library rule and the existing fakes carry a comment saying so. +- Compare sequences with `Assert.AreSequenceEqual`, never `CollectionAssert.AreEqual` or `AreEquivalent`. Existing tests still use `CollectionAssert`; new ones do not. +- Warnings are errors. The build fails on an unused `using` and on formatting. +- A public builder is an `IGitXBuilder` interface plus an `internal sealed class` implementing it, constructed with `(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath)` and deriving from `GitCommandBuilder`. +- `GitCommandBuilder` provides `BuildArguments()` for tests, `ExecuteAsync`, `TryExecuteAsync`, and the overridable `GetDiagnostic` and `CreateException`. Subclasses implement `AppendVerbArguments(ICollection)` and `ParseResult(GitProcessResult)`. +- `AppendOperands(arguments, params string[])` emits `--end-of-options` before its operands. Use it for anything caller-supplied. +- Every built vector begins `-C --no-pager -c core.quotepath=false -c color.ui=false`, added by the base class. Task tests that assert a whole vector must include that prefix. +- Commit messages carry a version tag: `[major]`, `[minor]`, `[patch]`. This work is `[minor]`, since it adds surface without breaking any. **Do not add `Co-Authored-By` lines.** +- Do not edit `VERSION.md`, `CHANGELOG.md` or `LICENSE.md`. +- Build with `dotnet build`. `dotnet test` reports zero tests on macOS with this MSTest.Sdk setup: run `./GitIntegration.Test/bin/Debug/net10.0/ktsu.GitIntegration.Test` directly, optionally with `--filter "FullyQualifiedName~"`. +- Baseline before any change: **668 tests passing, 0 warnings.** +- Integration tests live in `GitIntegration.Test/Integration/`, call `await IntegrationGitFixture.RequireGitAsync(cancellationToken)` first so they skip where git is absent, and build repositories with `using TemporaryRepository repository = new();`. + +## Review Focus + +Five inputs the spec implies that no task's happy path exercises, most likely to bite first. + +1. **An empty or whitespace patch handed to `Apply`.** Git reports an unhelpful error about a corrupt patch. A caller passing the result of selecting no hunks should get something better. Test in Task 4. +2. **`PatchFor` with no hunks.** Produces a header with no body, which is the input above. It should refuse rather than build it. Test in Task 1. +3. **The temporary file when apply throws.** A failing patch must not leave files in the temp directory, and the failure that matters is git's, not the cleanup's. Test in Task 4. +4. **A file both renamed and modified.** Carries rename headers *and* hunks, so code that treats rename as "no hunks" drops real changes. Test in Task 2. +5. **A patch generated, then the working tree moved.** `Checked()` must report failure and leave the index untouched, which is the guarantee a caller's refusal depends on. Test in Task 4. + +--- + +## File Structure + +| File | Responsibility | +|---|---| +| `GitIntegration/Models/GitPatch.cs` | **Create.** `GitPatch`, `GitFilePatch`, `GitHunk`, `GitPatchLine`, and `PatchFor` | +| `GitIntegration/Models/GitEnums.cs` | **Modify.** Add `GitPatchLineKind` | +| `GitIntegration/Parsing/GitPatchParser.cs` | **Create.** Unified diff text to `GitPatch` | +| `GitIntegration/Builders/GitPatchBuilder.cs` | **Create.** `IGitPatchBuilder` and its implementation | +| `GitIntegration/Builders/GitApplyBuilder.cs` | **Create.** `IGitApplyBuilder`, including the temporary file | +| `GitIntegration/Builders/GitRestoreBuilder.cs` | **Create.** `IGitRestoreBuilder`, including the version fallback | +| `GitIntegration/GitRepository.cs` | **Modify.** Add `Patch()`, `Apply()`, `Unstage()` | + +--- + +## Task 1: The patch model and assembling a patch + +**Files:** +- Create: `GitIntegration/Models/GitPatch.cs` +- Modify: `GitIntegration/Models/GitEnums.cs` +- Test: `GitIntegration.Test/Models/GitPatchTests.cs` + +**Interfaces:** +- Consumes: `GitChangeKind` from `GitEnums.cs`. +- Produces: + - `enum GitPatchLineKind { Context, Added, Removed }` + - `sealed record GitPatchLine { GitPatchLineKind Kind; string Text; int? OldNumber; int? NewNumber; }` + - `sealed record GitHunk { int OldStart; int OldCount; int NewStart; int NewCount; string Heading; IReadOnlyList Lines; string Text; }` + - `sealed record GitFilePatch { RelativeFilePath Path; RelativeFilePath? OriginalPath; GitChangeKind Kind; bool IsBinary; bool IsConflicted; string Header; IReadOnlyList Hunks; string PatchFor(IEnumerable hunks); }` + - `sealed record GitPatch { IReadOnlyList Files; }` + +This task runs no git and needs no parser. `PatchFor` is the one piece of pure logic here and everything later depends on it being right. + +- [ ] **Step 1: Write the failing tests** + +Create `GitIntegration.Test/Models/GitPatchTests.cs`: + +```csharp +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.Collections.Generic; + +using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; + +[TestClass] +public class GitPatchTests +{ + private const string Header = + "diff --git a/f.txt b/f.txt\nindex 92dfa21..81f8caa 100644\n--- a/f.txt\n+++ b/f.txt\n"; + + private static GitHunk HunkOne => new() + { + OldStart = 1, + OldCount = 3, + NewStart = 1, + NewCount = 3, + Heading = string.Empty, + Lines = [], + Text = "@@ -1,3 +1,3 @@\n a\n-b\n+B\n c\n", + }; + + private static GitHunk HunkTwo => new() + { + OldStart = 8, + OldCount = 3, + NewStart = 8, + NewCount = 3, + Heading = string.Empty, + Lines = [], + Text = "@@ -8,3 +8,3 @@\n h\n-i\n+I\n j\n", + }; + + private static GitFilePatch FileWithTwoHunks => new() + { + Path = "f.txt".As(), + OriginalPath = null, + Kind = GitChangeKind.Modified, + IsBinary = false, + IsConflicted = false, + Header = Header, + Hunks = [HunkOne, HunkTwo], + }; + + [TestMethod] + public void PatchForOneHunkCarriesTheHeaderAndThatHunkOnly() + { + string patch = FileWithTwoHunks.PatchFor([HunkOne]); + + Assert.AreEqual(Header + HunkOne.Text, patch); + } + + [TestMethod] + public void PatchForEveryHunkKeepsThemInFileOrder() + { + string patch = FileWithTwoHunks.PatchFor([HunkTwo, HunkOne]); + + Assert.AreEqual( + Header + HunkOne.Text + HunkTwo.Text, + patch, + "Hunks are emitted in the order the file holds them, not the order the caller asked for, because git reads a patch top to bottom."); + } + + [TestMethod] + public void PatchForNoHunksRefuses() => + Assert.ThrowsExactly( + () => _ = FileWithTwoHunks.PatchFor([]), + "A header with no body is a patch git rejects as corrupt, and the caller learns nothing from that message."); + + [TestMethod] + public void PatchForNullRefuses() => + Assert.ThrowsExactly(() => _ = FileWithTwoHunks.PatchFor(null!)); + + [TestMethod] + public void PatchPreservesAHunkVerbatimIncludingTheNoNewlineMarker() + { + GitHunk hunk = new() + { + OldStart = 1, + OldCount = 1, + NewStart = 1, + NewCount = 1, + Heading = string.Empty, + Lines = [], + Text = "@@ -1 +1 @@\n-a\n+b\n\\ No newline at end of file\n", + }; + + GitFilePatch file = FileWithTwoHunks with { Hunks = [hunk] }; + + StringAssert.Contains( + file.PatchFor([hunk]), + "\\ No newline at end of file", + StringComparison.Ordinal, + "Regenerating a hunk from its parsed lines loses this, and apply then rejects the patch."); + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `dotnet build` +Expected: build failure, `GitPatch`, `GitFilePatch`, `GitHunk` and `GitPatchLineKind` do not exist. + +- [ ] **Step 3: Add the enum** + +In `GitIntegration/Models/GitEnums.cs`, following the shape of the enums already there: + +```csharp +/// What one line of a patch does. +public enum GitPatchLineKind +{ + /// Present on both sides, shown for context. + Context, + + /// Present only after the change. + Added, + + /// Present only before the change. + Removed, +} +``` + +- [ ] **Step 4: Create the model** + +Create `GitIntegration/Models/GitPatch.cs` with the four records. `PatchFor` is the only method: + +```csharp + /// + /// Assembles the header and the given hunks into text git apply accepts. + /// + /// + /// Hunks are emitted in the order this file holds them rather than the order they were given, + /// because git reads a patch top to bottom and rejects one whose hunks run backwards. + /// + /// The hunks are not checked for belonging to this file. The comparison would cost a pass per + /// call to catch a mistake no reasonable caller makes, and a hunk from elsewhere fails at apply + /// with git's own message. + /// + /// + /// The hunks to include. + /// The patch text. + /// is . + /// is empty. + public string PatchFor(IEnumerable hunks) + { + Ensure.NotNull(hunks); + + HashSet wanted = [.. hunks]; + + if (wanted.Count == 0) + { + throw new ArgumentException( + "A patch needs at least one hunk. A header with no body is rejected as corrupt, and " + + "git's message for it explains nothing.", + nameof(hunks)); + } + + StringBuilder builder = new(Header); + + foreach (GitHunk hunk in Hunks.Where(wanted.Contains)) + { + _ = builder.Append(hunk.Text); + } + + return builder.ToString(); + } +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `dotnet build && ./GitIntegration.Test/bin/Debug/net10.0/ktsu.GitIntegration.Test --filter "FullyQualifiedName~GitPatchTests"` +Expected: 5 passing. + +- [ ] **Step 6: Run the full suite** + +Run: `dotnet build && ./GitIntegration.Test/bin/Debug/net10.0/ktsu.GitIntegration.Test` +Expected: 673 passing (668 baseline plus 5), 0 warnings. + +- [ ] **Step 7: Commit** + +```bash +git add GitIntegration/Models/GitPatch.cs GitIntegration/Models/GitEnums.cs GitIntegration.Test/Models/GitPatchTests.cs +git commit -m "[minor] Add the patch model and hunk assembly + +A file patch keeps git's own hunk text beside the parsed lines, because a +hunk regenerated from its lines loses the no-newline marker and apply then +rejects it. PatchFor selects hunks into a patch, emitting them in file +order since git reads one top to bottom, and refuses an empty selection +rather than producing a header git calls corrupt." +``` + +--- + +## Task 2: The parser + +**Files:** +- Create: `GitIntegration/Parsing/GitPatchParser.cs` +- Create: `GitIntegration.Test/Fixtures/patch-two-hunks.txt`, `patch-no-newline.txt`, `patch-rename-modified.txt`, `patch-binary.txt`, `patch-conflict.txt`, `patch-crlf.txt` +- Test: `GitIntegration.Test/Parsing/GitPatchParserTests.cs` + +**Interfaces:** +- Consumes: the model from Task 1. +- Produces: `internal static class GitPatchParser` with `public static GitPatch Parse(string output)`. + +**Capturing the fixtures.** Generate each with a real git in a scratch directory and save the exact bytes. Do not hand-write them, and do not tidy them: the point of a fixture is that it is what git emits. Capture with `git -c color.ui=false diff --no-ext-diff --no-textconv -U3`. For the conflict fixture, create a merge conflict and diff the unmerged path. For the CRLF fixture, write a file with CRLF endings and modify one line. + +- [ ] **Step 1: Capture the fixtures** + +Build each case in a temporary repository and write the captured output into `GitIntegration.Test/Fixtures/`. Record the git version used in a comment at the top of the test class, matching how `docs/superpowers/plans/2026-08-20-gitintegration-v2-phase3-read-only-verbs.md` records its fixtures. Set every fixture file to `Content` with `CopyToOutputDirectory` in `GitIntegration.Test.csproj` if the existing fixtures need that, matching whatever the JSON fixtures already do. + +- [ ] **Step 2: Write the failing tests** + +Create `GitIntegration.Test/Parsing/GitPatchParserTests.cs`. One test per fixture, asserting the shape rather than every byte: + +```csharp + [TestMethod] + public void ParsesTwoHunksWithTheirLineNumbers() + { + GitPatch patch = GitPatchParser.Parse(Fixture("patch-two-hunks.txt")); + + GitFilePatch file = patch.Files.Single(); + + Assert.AreEqual("f.txt", file.Path.WeakString); + Assert.AreEqual(GitChangeKind.Modified, file.Kind); + Assert.AreEqual(2, file.Hunks.Count); + + GitHunk first = file.Hunks[0]; + Assert.AreEqual(1, first.OldStart); + Assert.IsTrue(first.Text.StartsWith("@@", StringComparison.Ordinal)); + Assert.IsTrue( + first.Lines.Any(line => line.Kind == GitPatchLineKind.Added), + "A modification has at least one added line."); + } + + [TestMethod] + public void KeepsTheNoNewlineMarkerInsideTheHunkText() + { + GitPatch patch = GitPatchParser.Parse(Fixture("patch-no-newline.txt")); + + StringAssert.Contains( + patch.Files.Single().Hunks.Single().Text, + "\\ No newline at end of file", + StringComparison.Ordinal); + } + + [TestMethod] + public void ReadsARenamedFileThatAlsoChanged() + { + GitPatch patch = GitPatchParser.Parse(Fixture("patch-rename-modified.txt")); + + GitFilePatch file = patch.Files.Single(); + + Assert.AreEqual(GitChangeKind.Renamed, file.Kind); + Assert.IsNotNull(file.OriginalPath); + Assert.AreNotEqual(file.OriginalPath!.WeakString, file.Path.WeakString); + Assert.IsTrue( + file.Hunks.Count > 0, + "A rename can carry content changes too, and treating rename as hunkless drops them."); + } + + [TestMethod] + public void FlagsABinaryFileAndGivesItNoHunks() + { + GitFilePatch file = GitPatchParser.Parse(Fixture("patch-binary.txt")).Files.Single(); + + Assert.IsTrue(file.IsBinary); + Assert.AreEqual(0, file.Hunks.Count); + } + + [TestMethod] + public void FlagsAConflictedFileAndGivesItNoHunks() + { + GitFilePatch file = GitPatchParser.Parse(Fixture("patch-conflict.txt")).Files.Single(); + + Assert.IsTrue( + file.IsConflicted, + "Combined format is not an applyable patch, so it must not be parsed into ordinary hunks."); + Assert.AreEqual(0, file.Hunks.Count); + } + + [TestMethod] + public void ParsesCarriageReturnContentWithoutStrippingIt() + { + GitFilePatch file = GitPatchParser.Parse(Fixture("patch-crlf.txt")).Files.Single(); + + StringAssert.Contains( + file.Hunks.Single().Text, + "\r", + StringComparison.Ordinal, + "A patch is byte-sensitive, so content line endings survive parsing."); + } + + [TestMethod] + public void ParsesAnEmptyDiffAsNoFiles() => + Assert.AreEqual(0, GitPatchParser.Parse(string.Empty).Files.Count); +``` + +- [ ] **Step 3: Run the tests to verify they fail** + +Run: `dotnet build` +Expected: build failure, `GitPatchParser` does not exist. + +- [ ] **Step 4: Implement the parser** + +Walk the output line by line. A line starting `diff --git ` begins a file. Header lines are everything before the first `@@` or, for a file with no hunks, everything to the next `diff --git `. `similarity index`, `rename from` and `rename to` set `Kind` and `OriginalPath`. `new file mode` and `deleted file mode` set `Kind`. A line starting `Binary files ` or `GIT binary patch` sets `IsBinary`. A line starting `@@@` sets `IsConflicted` and stops hunk collection for that file. A line starting `@@ ` begins a hunk: parse `@@ -old,count +new,count @@ heading`, where a missing count means 1. Inside a hunk, ` ` is `Context`, `+` is `Added`, `-` is `Removed`, and `\` is part of the text but not a line. Line numbers advance per kind. Accumulate each hunk's raw text verbatim as you go. + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `dotnet build && ./GitIntegration.Test/bin/Debug/net10.0/ktsu.GitIntegration.Test --filter "FullyQualifiedName~GitPatchParserTests"` +Expected: 7 passing. + +- [ ] **Step 6: Run the full suite and commit** + +```bash +dotnet build && ./GitIntegration.Test/bin/Debug/net10.0/ktsu.GitIntegration.Test +git add GitIntegration/Parsing/GitPatchParser.cs GitIntegration.Test/Parsing/GitPatchParserTests.cs GitIntegration.Test/Fixtures/ +git commit -m "[minor] Parse a unified diff into files, hunks and lines + +Each hunk keeps the bytes git emitted alongside the parsed lines. Combined +format from an unmerged path is recognised and left unparsed rather than +turned into ordinary hunks, which would produce patches git rejects with +nothing explaining why, and a rename that also changed content keeps its +hunks." +``` + +--- + +## Task 3: Reading a patch + +**Files:** +- Create: `GitIntegration/Builders/GitPatchBuilder.cs` +- Modify: `GitIntegration/GitRepository.cs` +- Test: `GitIntegration.Test/Builders/GitPatchBuilderTests.cs` + +**Interfaces:** +- Consumes: `GitPatchParser.Parse` from Task 2. +- Produces: + - `public interface IGitPatchBuilder : IGitCommandBuilder` with `Staged()`, `WithContext(int lines)`, `ForPath(RelativeFilePath path)`, `Against(GitRefName revision)`, `Between(GitRefName a, GitRefName b)`, `DetectRenames()` + - `GitRepository.Patch()` returning `IGitPatchBuilder` + +- [ ] **Step 1: Write the failing tests** + +```csharp + [TestMethod] + public void BuildsTheDefaultPatchVector() + { + RecordingGitProcessRunner runner = new(); + GitPatchBuilder builder = new(runner, TestPaths.Root); + + string[] expectedArguments = + [ + "-C", TestPaths.Root.WeakString, + "--no-pager", + "-c", "core.quotepath=false", + "-c", "color.ui=false", + "diff", + "--no-ext-diff", + "--no-textconv", + "--no-color", + ]; + + Assert.AreSequenceEqual(expectedArguments, builder.BuildArguments()); + } + + [TestMethod] + public void MapsTheOptionFlags() + { + RecordingGitProcessRunner runner = new(); + + GitPatchBuilder staged = new(runner, TestPaths.Root); + _ = staged.Staged(); + Assert.IsTrue(staged.BuildArguments().Contains("--cached")); + + GitPatchBuilder context = new(runner, TestPaths.Root); + _ = context.WithContext(7); + Assert.IsTrue(context.BuildArguments().Contains("-U7")); + + GitPatchBuilder renames = new(runner, TestPaths.Root); + _ = renames.DetectRenames(); + Assert.IsTrue(renames.BuildArguments().Contains("--find-renames")); + } + + [TestMethod] + public void NeverEmitsTheNulSeparator() + { + RecordingGitProcessRunner runner = new(); + GitPatchBuilder builder = new(runner, TestPaths.Root); + + Assert.IsFalse( + builder.BuildArguments().Contains("-z"), + "Patch format is line-based, and -z changes only the name-status framing this builder does not use."); + } + + [TestMethod] + public void RefusesANegativeContextCount() + { + RecordingGitProcessRunner runner = new(); + GitPatchBuilder builder = new(runner, TestPaths.Root); + + _ = Assert.ThrowsExactly(() => _ = builder.WithContext(-1)); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `dotnet build` +Expected: build failure, `GitPatchBuilder` does not exist. + +- [ ] **Step 3: Implement the builder** + +`AppendVerbArguments` adds `diff`, then `--no-ext-diff`, `--no-textconv`, `--no-color` unconditionally, then the options in a fixed order, then revisions and paths through `AppendOperands`. `ParseResult` returns `GitPatchParser.Parse(result.StandardOutput)`. + +Document the three unconditional flags where they are added: + +```csharp + // Correctness, not tidiness. A repository with a gitattributes diff driver emits + // human-readable output in place of a patch, which no caller can apply, and --no-color + // guards a config setting color.diff to always, which would put escape sequences into text + // that goes back to apply. color.ui on the base vector does not cover that: git lets + // color.diff take precedence over it. +``` + +- [ ] **Step 4: Add the entry point** + +In `GitRepository.cs`, beside `Diff()`: + +```csharp + /// Reads a patch, with the hunks and lines a caller needs to show or stage a change. + /// The builder. + public IGitPatchBuilder Patch() => new GitPatchBuilder(RequireRunner(), RequireLocalPath()); +``` + +- [ ] **Step 5: Run the tests to verify they pass, then the full suite** + +Run: `dotnet build && ./GitIntegration.Test/bin/Debug/net10.0/ktsu.GitIntegration.Test` +Expected: everything passing, 0 warnings. + +- [ ] **Step 6: Commit** + +```bash +git add GitIntegration/Builders/GitPatchBuilder.cs GitIntegration/GitRepository.cs GitIntegration.Test/Builders/GitPatchBuilderTests.cs +git commit -m "[minor] Add Patch, which reads a diff as hunks rather than counts + +Diff answers which files changed and by how much. Patch answers what +changed, which is what a caller drawing a diff or staging a hunk needs. It +always passes --no-ext-diff and --no-textconv, because a repository with a +gitattributes diff driver otherwise emits something no caller can apply." +``` + +--- + +## Task 4: Applying a patch + +**Files:** +- Create: `GitIntegration/Builders/GitApplyBuilder.cs` +- Modify: `GitIntegration/GitRepository.cs` +- Test: `GitIntegration.Test/Builders/GitApplyBuilderTests.cs` +- Test: `GitIntegration.Test/Integration/GitPatchRoundTripTests.cs` + +**Interfaces:** +- Consumes: the model from Task 1, `Patch()` from Task 3. +- Produces: + - `public interface IGitApplyBuilder : IGitCommandBuilder` with `ToIndex()`, `Reversed()`, `Checked()` + - `GitRepository.Apply(string patchText)` returning `IGitApplyBuilder` + +- [ ] **Step 1: Write the failing unit tests** + +```csharp + [TestMethod] + public void MapsTheOptionFlags() + { + RecordingGitProcessRunner runner = new(); + + GitApplyBuilder index = new(runner, TestPaths.Root, "patch"); + _ = index.ToIndex(); + Assert.IsTrue(index.BuildArguments().Contains("--cached")); + + GitApplyBuilder reversed = new(runner, TestPaths.Root, "patch"); + _ = reversed.Reversed(); + Assert.IsTrue(reversed.BuildArguments().Contains("--reverse")); + + GitApplyBuilder checkOnly = new(runner, TestPaths.Root, "patch"); + _ = checkOnly.Checked(); + Assert.IsTrue(checkOnly.BuildArguments().Contains("--check")); + } + + [TestMethod] + public void AlwaysSuppressesWhitespaceWarnings() + { + RecordingGitProcessRunner runner = new(); + GitApplyBuilder builder = new(runner, TestPaths.Root, "patch"); + + Assert.IsTrue( + builder.BuildArguments().Contains("--whitespace=nowarn"), + "Staging content already on disk is not the moment to enforce a whitespace policy the user configured for authoring."); + } + + [TestMethod] + public void RefusesEmptyPatchText() + { + RecordingGitProcessRunner runner = new(); + GitRepository repository = new() { LocalPath = TestPaths.Root, ProcessRunner = runner }; + + _ = Assert.ThrowsExactly(() => _ = repository.Apply(" ")); + } + + [TestMethod] + public async Task DeletesTheTemporaryFileEvenWhenGitFailsAsync() + { + RecordingGitProcessRunner runner = new() { ExitCode = 1, StandardError = "error: corrupt patch" }; + GitApplyBuilder builder = new(runner, TestPaths.Root, "not a patch\n"); + + _ = await Assert.ThrowsExactlyAsync( + async () => await builder.ExecuteAsync().ConfigureAwait(false)).ConfigureAwait(false); + + string path = runner.LastArguments!.Last(); + + Assert.IsFalse( + File.Exists(path), + "A failing patch must not leave files behind, and the failure the caller sees is git's, not the cleanup's."); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `dotnet build` +Expected: build failure, `GitApplyBuilder` does not exist. + +- [ ] **Step 3: Implement the builder** + +The constructor takes the patch text. `Apply` on `GitRepository` validates it is neither null nor whitespace and throws `ArgumentException` otherwise, because an empty patch reaches git as a corrupt-patch error that explains nothing. + +`AppendVerbArguments` adds `apply`, `--whitespace=nowarn`, the option flags, then the temporary file's path through `AppendOperands`. The file is written when the vector is built and deleted after the process returns, in a `finally`, by overriding `ExecuteAsync` and `TryExecuteAsync` to wrap the base call. + +Write the file as UTF-8 with no byte order mark, preserving the text exactly: + +```csharp + File.WriteAllText(path, _patchText, new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)); +``` + +- [ ] **Step 4: Write the round-trip integration test** + +Create `GitIntegration.Test/Integration/GitPatchRoundTripTests.cs`, following the shape of the tests already in that directory: + +```csharp + [TestMethod] + public async Task StagingOneHunkLeavesTheOtherUnstagedAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + + // seed a ten-line file, commit it, then change the second and the tenth line + // so the diff has two separate hunks + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + + GitPatch patch = await opened.Patch().WithContext(1).ExecuteAsync().ConfigureAwait(false); + GitFilePatch file = patch.Files.Single(); + + Assert.AreEqual(2, file.Hunks.Count, "The fixture must produce two hunks, or this proves nothing."); + + _ = await opened.Apply(file.PatchFor([file.Hunks[0]])).ToIndex().ExecuteAsync().ConfigureAwait(false); + + IReadOnlyList staged = await opened.Diff().Staged().WithLineCounts() + .ExecuteAsync().ConfigureAwait(false); + IReadOnlyList unstaged = await opened.Diff().WithLineCounts() + .ExecuteAsync().ConfigureAwait(false); + + Assert.AreEqual(1, staged.Single().Insertions); + Assert.AreEqual(1, unstaged.Single().Insertions); + } + + [TestMethod] + public async Task CheckedReportsFailureWhenTheWorkingTreeMovedAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + + // seed and commit a file, change it, read the patch, then overwrite the file + // with unrelated content so the patch no longer applies + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + GitFilePatch file = (await opened.Patch().ExecuteAsync().ConfigureAwait(false)).Files.Single(); + string text = file.PatchFor(file.Hunks); + + repository.WriteFile("f.txt", "something else entirely\n"); + + GitResult result = await opened.Apply(text).ToIndex().Checked() + .TryExecuteAsync().ConfigureAwait(false); + + Assert.IsFalse(result.Success, "A caller's refusal depends on this reporting failure rather than throwing."); + + IReadOnlyList staged = await opened.Diff().Staged().ExecuteAsync().ConfigureAwait(false); + + Assert.AreEqual(0, staged.Count, "--check must change nothing."); + } +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `dotnet build && ./GitIntegration.Test/bin/Debug/net10.0/ktsu.GitIntegration.Test --filter "FullyQualifiedName~GitApply"` +Expected: unit tests and both round-trip tests passing. If git is absent they skip rather than fail. + +- [ ] **Step 6: Run the full suite and commit** + +```bash +git add GitIntegration/Builders/GitApplyBuilder.cs GitIntegration/GitRepository.cs GitIntegration.Test/Builders/GitApplyBuilderTests.cs GitIntegration.Test/Integration/GitPatchRoundTripTests.cs +git commit -m "[minor] Add Apply, which puts a patch into the index + +Staging a hunk is Apply(text).ToIndex(), and unstaging one is the same with +Reversed. Checked asks whether a patch would still apply without changing +anything, which is what lets a caller refuse cleanly when the working tree +moved underneath it. + +Git reads a patch from standard input or a file, and the process request +carries no standard input, so the patch goes to a temporary file that is +deleted on every path. The round-trip test is what proves the model and the +parser kept every byte." +``` + +--- + +## Task 5: Unstaging + +**Files:** +- Create: `GitIntegration/Builders/GitRestoreBuilder.cs` +- Modify: `GitIntegration/GitRepository.cs` +- Test: `GitIntegration.Test/Builders/GitRestoreBuilderTests.cs` + +**Interfaces:** +- Consumes: the version probe pattern in `GitFetchBuilder`. +- Produces: + - `public interface IGitRestoreBuilder : IGitCommandBuilder` + - `GitRepository.Unstage(RelativeFilePath path)` returning `IGitRestoreBuilder` + +- [ ] **Step 1: Write the failing tests** + +```csharp + [TestMethod] + public void BuildsTheRestoreVectorOnAModernGit() + { + RecordingGitProcessRunner runner = new(); + GitRestoreBuilder builder = new(runner, TestPaths.Root, "f.txt".As()); + + IReadOnlyList arguments = builder.BuildArguments(); + + Assert.IsTrue(arguments.Contains("restore")); + Assert.IsTrue(arguments.Contains("--staged")); + Assert.IsTrue(arguments.Contains("f.txt")); + } + + [TestMethod] + public void RefusesANullPath() + { + RecordingGitProcessRunner runner = new(); + GitRepository repository = new() { LocalPath = TestPaths.Root, ProcessRunner = runner }; + + _ = Assert.ThrowsExactly(() => _ = repository.Unstage(null!)); + } +``` + +Read `GitFetchBuilder`'s `ProbeVersionAsync` and `PorcelainSupportedByVersion` before writing the fallback test. If the probe can be driven from a `RecordingGitProcessRunner` by seeding `StandardOutput` with a version string, add a test asserting the vector becomes `reset HEAD -- ` below 2.23. If it cannot be driven without a second git installation, do not write a test that asserts against a mock of our own probe: say so in the report and leave the fallback covered by inspection. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `dotnet build` +Expected: build failure, `GitRestoreBuilder` does not exist. + +- [ ] **Step 3: Implement the builder with its fallback** + +Follow `GitFetchBuilder`: probe the version in `ExecuteAsync` and `TryExecuteAsync` before the vector is built, and choose `restore --staged -- ` at 2.23 or above and `reset HEAD -- ` below it. + +- [ ] **Step 4: Add the entry point** + +```csharp + /// Removes a path's staged changes, leaving the working tree alone. + /// The path, relative to the repository root. + /// The builder. + public IGitRestoreBuilder Unstage(RelativeFilePath path) => + new GitRestoreBuilder(RequireRunner(), RequireLocalPath(), Ensure.NotNull(path)); +``` + +- [ ] **Step 5: Run the full suite and commit** + +```bash +git add GitIntegration/Builders/GitRestoreBuilder.cs GitIntegration/GitRepository.cs GitIntegration.Test/Builders/GitRestoreBuilderTests.cs +git commit -m "[minor] Add Unstage for the whole-file case + +git restore arrived in 2.23, so this probes the installed version and falls +back to reset below it, the same shape Fetch already uses for porcelain. +Unstaging a hunk is Apply reversed. This is the file-level verb, and the +only one available for a binary file, which has no hunks." +``` + +--- + +## Self-review notes + +**Spec coverage.** Every section maps to a task: the model and `PatchFor` to Task 1, the parser and the four cases a patch cannot express to Task 2, the reader and its three unconditional flags to Task 3, apply with its temporary file and `--check` to Task 4, and unstaging with its version fallback to Task 5. The testing section's fixtures are Task 2 and its round trip is Task 4. + +**Review Focus coverage.** Empty patch text is Task 4 Step 1, `PatchFor` with no hunks is Task 1 Step 1, the temporary file on failure is Task 4 Step 1, a renamed and modified file is Task 2 Step 2, and a moved working tree is Task 4 Step 4. + +**One thing deliberately left open.** Task 5 does not promise a test for the version fallback, because whether the probe can be driven without a second git installation is not knowable from reading. The task says what to do in each case rather than pretending the answer. + +**Fixtures are captured, not written.** Task 2 Step 1 exists as its own step because a hand-written fixture would pass the parser tests and fail the round trip, which is the one failure this plan is shaped to prevent. From 6d0e50a00da1111add67bb9fc07063ead3c5b72a Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 19:35:40 +1000 Subject: [PATCH 03/13] [minor] Add the patch model and hunk assembly A file patch keeps git's own hunk text beside the parsed lines, because a hunk regenerated from its lines loses the no-newline marker and apply then rejects it. PatchFor selects hunks into a patch, emitting them in file order since git reads one top to bottom, and refuses an empty selection rather than producing a header git calls corrupt. --- GitIntegration.Test/Models/GitPatchTests.cs | 100 +++++++++++++ GitIntegration/Models/GitEnums.cs | 13 ++ GitIntegration/Models/GitPatch.cs | 155 ++++++++++++++++++++ 3 files changed, 268 insertions(+) create mode 100644 GitIntegration.Test/Models/GitPatchTests.cs create mode 100644 GitIntegration/Models/GitPatch.cs diff --git a/GitIntegration.Test/Models/GitPatchTests.cs b/GitIntegration.Test/Models/GitPatchTests.cs new file mode 100644 index 0000000..48f0669 --- /dev/null +++ b/GitIntegration.Test/Models/GitPatchTests.cs @@ -0,0 +1,100 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; + +using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; + +[TestClass] +public class GitPatchTests +{ + private const string Header = + "diff --git a/f.txt b/f.txt\nindex 92dfa21..81f8caa 100644\n--- a/f.txt\n+++ b/f.txt\n"; + + private static GitHunk HunkOne => new() + { + OldStart = 1, + OldCount = 3, + NewStart = 1, + NewCount = 3, + Heading = string.Empty, + Lines = [], + Text = "@@ -1,3 +1,3 @@\n a\n-b\n+B\n c\n", + }; + + private static GitHunk HunkTwo => new() + { + OldStart = 8, + OldCount = 3, + NewStart = 8, + NewCount = 3, + Heading = string.Empty, + Lines = [], + Text = "@@ -8,3 +8,3 @@\n h\n-i\n+I\n j\n", + }; + + private static GitFilePatch FileWithTwoHunks => new() + { + Path = "f.txt".As(), + OriginalPath = null, + Kind = GitChangeKind.Modified, + IsBinary = false, + IsConflicted = false, + Header = Header, + Hunks = [HunkOne, HunkTwo], + }; + + [TestMethod] + public void PatchForOneHunkCarriesTheHeaderAndThatHunkOnly() + { + string patch = FileWithTwoHunks.PatchFor([HunkOne]); + + Assert.AreEqual(Header + HunkOne.Text, patch); + } + + [TestMethod] + public void PatchForEveryHunkKeepsThemInFileOrder() + { + string patch = FileWithTwoHunks.PatchFor([HunkTwo, HunkOne]); + + Assert.AreEqual( + Header + HunkOne.Text + HunkTwo.Text, + patch, + "Hunks are emitted in the order the file holds them, not the order the caller asked for, because git reads a patch top to bottom."); + } + + [TestMethod] + public void PatchForNoHunksRefuses() => + Assert.ThrowsExactly( + () => _ = FileWithTwoHunks.PatchFor([]), + "A header with no body is a patch git rejects as corrupt, and the caller learns nothing from that message."); + + [TestMethod] + public void PatchForNullRefuses() => + Assert.ThrowsExactly(() => _ = FileWithTwoHunks.PatchFor(null!)); + + [TestMethod] + public void PatchPreservesAHunkVerbatimIncludingTheNoNewlineMarker() + { + GitHunk hunk = new() + { + OldStart = 1, + OldCount = 1, + NewStart = 1, + NewCount = 1, + Heading = string.Empty, + Lines = [], + Text = "@@ -1 +1 @@\n-a\n+b\n\\ No newline at end of file\n", + }; + + GitFilePatch file = FileWithTwoHunks with { Hunks = [hunk] }; + + StringAssert.Contains( + file.PatchFor([hunk]), + "\\ No newline at end of file", + StringComparison.Ordinal, + "Regenerating a hunk from its parsed lines loses this, and apply then rejects the patch."); + } +} diff --git a/GitIntegration/Models/GitEnums.cs b/GitIntegration/Models/GitEnums.cs index 3b465a2..0514db5 100644 --- a/GitIntegration/Models/GitEnums.cs +++ b/GitIntegration/Models/GitEnums.cs @@ -68,6 +68,19 @@ public enum GitChangeKind Unknown, } +/// What one line of a patch does. +public enum GitPatchLineKind +{ + /// Present on both sides, shown for context. + Context, + + /// Present only after the change. + Added, + + /// Present only before the change. + Removed, +} + /// /// How much untracked detail status should report. /// diff --git a/GitIntegration/Models/GitPatch.cs b/GitIntegration/Models/GitPatch.cs new file mode 100644 index 0000000..c58f582 --- /dev/null +++ b/GitIntegration/Models/GitPatch.cs @@ -0,0 +1,155 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; + +using ktsu.Semantics.Paths; + +/// +/// One line of a hunk, as parsed from the patch text. +/// +public sealed record GitPatchLine +{ + /// Gets what this line does. + public required GitPatchLineKind Kind { get; init; } + + /// Gets the line's text, without its leading marker character. + public required string Text { get; init; } + + /// + /// Gets this line's number on the old side of the hunk, or for a line + /// present only on the new side. + /// + public int? OldNumber { get; init; } + + /// + /// Gets this line's number on the new side of the hunk, or for a line + /// present only on the old side. + /// + public int? NewNumber { get; init; } +} + +/// +/// One contiguous range of changed lines within a file's patch, together with the surrounding +/// context git included. +/// +public sealed record GitHunk +{ + /// Gets the first line number this hunk covers on the old side. + public required int OldStart { get; init; } + + /// Gets how many lines this hunk covers on the old side. + public required int OldCount { get; init; } + + /// Gets the first line number this hunk covers on the new side. + public required int NewStart { get; init; } + + /// Gets how many lines this hunk covers on the new side. + public required int NewCount { get; init; } + + /// + /// Gets the function or section heading git found for this hunk, or the empty string when it + /// found none. + /// + public required string Heading { get; init; } + + /// Gets the hunk's lines, parsed from . + public required IReadOnlyList Lines { get; init; } + + /// + /// Gets the hunk's own text, from its @@ line to its last line, exactly as git printed + /// it. + /// + /// + /// Kept verbatim rather than regenerated from , because a detail such as the + /// no-newline-at-end-of-file marker has no home in the parsed lines and would otherwise be lost, + /// leaving git apply to reject the reassembled hunk. + /// + public required string Text { get; init; } +} + +/// +/// One file's patch: git's own header for the file, plus every hunk git found within it. +/// +public sealed record GitFilePatch +{ + /// Gets the path as it exists after the change, relative to the repository root. + public required RelativeFilePath Path { get; init; } + + /// + /// Gets the path this file came from for a rename or a copy, or + /// otherwise. + /// + public RelativeFilePath? OriginalPath { get; init; } + + /// Gets what happened to the path. + public required GitChangeKind Kind { get; init; } + + /// Gets whether git treated this file as binary rather than diffing its lines. + public required bool IsBinary { get; init; } + + /// Gets whether this file has conflicting changes from an unfinished merge. + public required bool IsConflicted { get; init; } + + /// + /// Gets the patch header, from the diff --git line up to but not including the first + /// hunk. + /// + public required string Header { get; init; } + + /// Gets the file's hunks, in the order git printed them. + public required IReadOnlyList Hunks { get; init; } + + /// + /// Assembles the header and the given hunks into text git apply accepts. + /// + /// + /// Hunks are emitted in the order this file holds them rather than the order they were given, + /// because git reads a patch top to bottom and rejects one whose hunks run backwards. + /// + /// The hunks are not checked for belonging to this file. The comparison would cost a pass per + /// call to catch a mistake no reasonable caller makes, and a hunk from elsewhere fails at apply + /// with git's own message. + /// + /// + /// The hunks to include. + /// The patch text. + /// is . + /// is empty. + public string PatchFor(IEnumerable hunks) + { + Ensure.NotNull(hunks); + + HashSet wanted = [.. hunks]; + + if (wanted.Count == 0) + { + throw new ArgumentException( + "A patch needs at least one hunk. A header with no body is rejected as corrupt, and " + + "git's message for it explains nothing.", + nameof(hunks)); + } + + StringBuilder builder = new(Header); + + foreach (GitHunk hunk in Hunks.Where(wanted.Contains)) + { + _ = builder.Append(hunk.Text); + } + + return builder.ToString(); + } +} + +/// +/// The full patch for a set of files, as diff or show reported it. +/// +public sealed record GitPatch +{ + /// Gets the patch for each changed file. + public required IReadOnlyList Files { get; init; } +} From 50fe58b4e6ca5f897dc1645f85071603dfa7b025 Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 19:54:56 +1000 Subject: [PATCH 04/13] [minor] Parse a unified diff into files, hunks and lines Each hunk keeps the bytes git emitted alongside the parsed lines. Combined format from an unmerged path is recognized and left unparsed rather than turned into ordinary hunks, which would produce patches git rejects with nothing explaining why, and a rename that also changed content keeps its hunks. The CRLF fixture mixes LF structural lines with CRLF content lines by design, so .gitattributes excludes the patch-*.txt fixtures from this repository's LF normalization; without that they would be rewritten to LF on the next checkout and the test they support would stop meaning anything. --- .gitattributes | 6 + GitIntegration.Test/Fixtures/patch-binary.txt | 3 + .../Fixtures/patch-conflict.txt | 14 + GitIntegration.Test/Fixtures/patch-crlf.txt | 9 + .../Fixtures/patch-no-newline.txt | 10 + .../Fixtures/patch-rename-modified.txt | 14 + .../Fixtures/patch-two-hunks.txt | 18 + .../GitIntegration.Test.csproj | 4 + .../Parsing/GitPatchParserTests.cs | 99 +++++ GitIntegration/Parsing/GitPatchParser.cs | 368 ++++++++++++++++++ 10 files changed, 545 insertions(+) create mode 100644 GitIntegration.Test/Fixtures/patch-binary.txt create mode 100644 GitIntegration.Test/Fixtures/patch-conflict.txt create mode 100644 GitIntegration.Test/Fixtures/patch-crlf.txt create mode 100644 GitIntegration.Test/Fixtures/patch-no-newline.txt create mode 100644 GitIntegration.Test/Fixtures/patch-rename-modified.txt create mode 100644 GitIntegration.Test/Fixtures/patch-two-hunks.txt create mode 100644 GitIntegration.Test/Parsing/GitPatchParserTests.cs create mode 100644 GitIntegration/Parsing/GitPatchParser.cs diff --git a/.gitattributes b/.gitattributes index a0bea35..3df8366 100644 --- a/.gitattributes +++ b/.gitattributes @@ -24,6 +24,12 @@ # them to avoid a spurious whole-file diff every time the solution is opened. *.sln text eol=crlf +# Captured git diff output, kept exactly as git emitted it. A patch fixture mixes LF +# structural lines with CRLF content lines by design, testing that the parser leaves +# carriage returns alone, so eol normalization above would corrupt the very bytes the +# test asserts on. +GitIntegration.Test/Fixtures/patch-*.txt -text + ############################### # Git Large File System (LFS) # ############################### diff --git a/GitIntegration.Test/Fixtures/patch-binary.txt b/GitIntegration.Test/Fixtures/patch-binary.txt new file mode 100644 index 0000000..7366ed5 --- /dev/null +++ b/GitIntegration.Test/Fixtures/patch-binary.txt @@ -0,0 +1,3 @@ +diff --git a/f.bin b/f.bin +index 872fc74..06389e4 100644 +Binary files a/f.bin and b/f.bin differ diff --git a/GitIntegration.Test/Fixtures/patch-conflict.txt b/GitIntegration.Test/Fixtures/patch-conflict.txt new file mode 100644 index 0000000..4cd4bdc --- /dev/null +++ b/GitIntegration.Test/Fixtures/patch-conflict.txt @@ -0,0 +1,14 @@ +diff --cc f.txt +index 305b879,1ebef9c..0000000 +--- a/f.txt ++++ b/f.txt +@@@ -1,3 -1,3 +1,9 @@@ + line1 +++<<<<<<< HEAD + +line2-A +++||||||| c7f1403 +++line2 +++======= ++ line2-B +++>>>>>>> branch-b + line3 diff --git a/GitIntegration.Test/Fixtures/patch-crlf.txt b/GitIntegration.Test/Fixtures/patch-crlf.txt new file mode 100644 index 0000000..4fe8ae9 --- /dev/null +++ b/GitIntegration.Test/Fixtures/patch-crlf.txt @@ -0,0 +1,9 @@ +diff --git a/f.txt b/f.txt +index b87108a..1a83710 100644 +--- a/f.txt ++++ b/f.txt +@@ -1,3 +1,3 @@ + line1 +-line2 ++line2-changed + line3 diff --git a/GitIntegration.Test/Fixtures/patch-no-newline.txt b/GitIntegration.Test/Fixtures/patch-no-newline.txt new file mode 100644 index 0000000..9531985 --- /dev/null +++ b/GitIntegration.Test/Fixtures/patch-no-newline.txt @@ -0,0 +1,10 @@ +diff --git a/f.txt b/f.txt +index e64015c..28d2298 100644 +--- a/f.txt ++++ b/f.txt +@@ -1,3 +1,3 @@ + line1 +-line2 ++line2-changed + line3 +\ No newline at end of file diff --git a/GitIntegration.Test/Fixtures/patch-rename-modified.txt b/GitIntegration.Test/Fixtures/patch-rename-modified.txt new file mode 100644 index 0000000..343344d --- /dev/null +++ b/GitIntegration.Test/Fixtures/patch-rename-modified.txt @@ -0,0 +1,14 @@ +diff --git a/old.txt b/new.txt +similarity index 64% +rename from old.txt +rename to new.txt +index 600d48a..39ac7c0 100644 +--- a/old.txt ++++ b/new.txt +@@ -1,5 +1,5 @@ + alpha + beta +-gamma ++gamma-CHANGED + delta + epsilon diff --git a/GitIntegration.Test/Fixtures/patch-two-hunks.txt b/GitIntegration.Test/Fixtures/patch-two-hunks.txt new file mode 100644 index 0000000..4c5489f --- /dev/null +++ b/GitIntegration.Test/Fixtures/patch-two-hunks.txt @@ -0,0 +1,18 @@ +diff --git a/f.txt b/f.txt +index f696b4b..532f82e 100644 +--- a/f.txt ++++ b/f.txt +@@ -1,5 +1,5 @@ + line1 +-line2 ++line2-CHANGED + line3 + line4 + line5 +@@ -16,5 +16,5 @@ line15 + line16 + line17 + line18 +-line19 ++line19-CHANGED + line20 diff --git a/GitIntegration.Test/GitIntegration.Test.csproj b/GitIntegration.Test/GitIntegration.Test.csproj index ee91bf3..4ec0280 100644 --- a/GitIntegration.Test/GitIntegration.Test.csproj +++ b/GitIntegration.Test/GitIntegration.Test.csproj @@ -30,5 +30,9 @@ + + + diff --git a/GitIntegration.Test/Parsing/GitPatchParserTests.cs b/GitIntegration.Test/Parsing/GitPatchParserTests.cs new file mode 100644 index 0000000..45dca26 --- /dev/null +++ b/GitIntegration.Test/Parsing/GitPatchParserTests.cs @@ -0,0 +1,99 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.IO; +using System.Linq; + +[TestClass] +public class GitPatchParserTests +{ + // Fixtures captured from git version 2.54.0 (Apple Git-157) on macOS, with + // `git -c color.ui=false diff --no-ext-diff --no-textconv -U3`. + + /// Reads a captured fixture's raw text from the test output's Fixtures directory. + private static string Fixture(string name) => + File.ReadAllText(Path.Combine(AppContext.BaseDirectory, "Fixtures", name)); + + [TestMethod] + public void ParsesTwoHunksWithTheirLineNumbers() + { + GitPatch patch = GitPatchParser.Parse(Fixture("patch-two-hunks.txt")); + + GitFilePatch file = patch.Files.Single(); + + Assert.AreEqual("f.txt", file.Path.WeakString); + Assert.AreEqual(GitChangeKind.Modified, file.Kind); + Assert.AreEqual(2, file.Hunks.Count); + + GitHunk first = file.Hunks[0]; + Assert.AreEqual(1, first.OldStart); + Assert.IsTrue(first.Text.StartsWith("@@", StringComparison.Ordinal)); + Assert.IsTrue( + first.Lines.Any(line => line.Kind == GitPatchLineKind.Added), + "A modification has at least one added line."); + } + + [TestMethod] + public void KeepsTheNoNewlineMarkerInsideTheHunkText() + { + GitPatch patch = GitPatchParser.Parse(Fixture("patch-no-newline.txt")); + + StringAssert.Contains( + patch.Files.Single().Hunks.Single().Text, + "\\ No newline at end of file", + StringComparison.Ordinal); + } + + [TestMethod] + public void ReadsARenamedFileThatAlsoChanged() + { + GitPatch patch = GitPatchParser.Parse(Fixture("patch-rename-modified.txt")); + + GitFilePatch file = patch.Files.Single(); + + Assert.AreEqual(GitChangeKind.Renamed, file.Kind); + Assert.IsNotNull(file.OriginalPath); + Assert.AreNotEqual(file.OriginalPath!.WeakString, file.Path.WeakString); + Assert.IsTrue( + file.Hunks.Count > 0, + "A rename can carry content changes too, and treating rename as hunkless drops them."); + } + + [TestMethod] + public void FlagsABinaryFileAndGivesItNoHunks() + { + GitFilePatch file = GitPatchParser.Parse(Fixture("patch-binary.txt")).Files.Single(); + + Assert.IsTrue(file.IsBinary); + Assert.AreEqual(0, file.Hunks.Count); + } + + [TestMethod] + public void FlagsAConflictedFileAndGivesItNoHunks() + { + GitFilePatch file = GitPatchParser.Parse(Fixture("patch-conflict.txt")).Files.Single(); + + Assert.IsTrue( + file.IsConflicted, + "Combined format is not an applyable patch, so it must not be parsed into ordinary hunks."); + Assert.AreEqual(0, file.Hunks.Count); + } + + [TestMethod] + public void ParsesCarriageReturnContentWithoutStrippingIt() + { + GitFilePatch file = GitPatchParser.Parse(Fixture("patch-crlf.txt")).Files.Single(); + + StringAssert.Contains( + file.Hunks.Single().Text, + "\r", + StringComparison.Ordinal, + "A patch is byte-sensitive, so content line endings survive parsing."); + } + + [TestMethod] + public void ParsesAnEmptyDiffAsNoFiles() => + Assert.AreEqual(0, GitPatchParser.Parse(string.Empty).Files.Count); +} diff --git a/GitIntegration/Parsing/GitPatchParser.cs b/GitIntegration/Parsing/GitPatchParser.cs new file mode 100644 index 0000000..1f42a01 --- /dev/null +++ b/GitIntegration/Parsing/GitPatchParser.cs @@ -0,0 +1,368 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System; +using System.Collections.Generic; +using System.Globalization; + +using ktsu.Semantics.Paths; + +/// +/// Reads git diff -U3 output into a : one +/// per changed file, and one per contiguous change within it. +/// +/// +/// Nothing here runs git. The text is whatever a caller captured, and every hunk's +/// is carried through verbatim so that feeding it back to +/// git apply reproduces exactly the bytes git itself emitted. +/// +internal static class GitPatchParser +{ + private const string GitHeaderPrefix = "diff --git "; + private const string CombinedHeaderPrefix = "diff --cc "; + private const string BSidePathMarker = " b/"; + private const string RenameFromPrefix = "rename from "; + private const string RenameToPrefix = "rename to "; + private const string NewFileModePrefix = "new file mode"; + private const string DeletedFileModePrefix = "deleted file mode"; + private const string BinaryFilesPrefix = "Binary files "; + private const string BinaryPatchPrefix = "GIT binary patch"; + private const string ConflictHunkPrefix = "@@@"; + private const string HunkPrefix = "@@ "; + + /// + /// Parses the text a diff-producing command wrote to standard output. + /// + /// Everything git wrote to standard output. + /// The patch, with one file per diff --git or diff --cc block found. + /// A header or a hunk was malformed. + public static GitPatch Parse(string output) + { + Ensure.NotNull(output); + + List<(int Start, int End)> lines = SplitLines(output); + List files = []; + + int index = 0; + while (index < lines.Count) + { + if (!IsFileStart(Line(output, lines, index))) + { + index++; + continue; + } + + files.Add(ParseFile(output, lines, ref index)); + } + + return new GitPatch { Files = files }; + } + + /// + /// Splits into line spans, each excluding its own trailing + /// \n but keeping every other byte, including a trailing \r from a + /// carriage-return-terminated source line. + /// + /// The text to split. + /// Each line's start and end offset into . + private static List<(int Start, int End)> SplitLines(string output) + { + List<(int Start, int End)> lines = []; + + int position = 0; + while (position <= output.Length) + { + int newline = output.IndexOf('\n', position); + + if (newline < 0) + { + if (position < output.Length) + { + lines.Add((position, output.Length)); + } + + break; + } + + lines.Add((position, newline)); + position = newline + 1; + } + + return lines; + } + + private static string Line(string output, List<(int Start, int End)> lines, int index) => + output[lines[index].Start..lines[index].End]; + + /// + /// The offset one past the last line belonging to a region ending at : + /// the start of that line, or the end of the text when the region runs to the end of the input. + /// + private static int RegionEnd(string output, List<(int Start, int End)> lines, int index) => + index < lines.Count ? lines[index].Start : output.Length; + + private static bool IsFileStart(string line) => + line.StartsWith(GitHeaderPrefix, StringComparison.Ordinal) || + line.StartsWith(CombinedHeaderPrefix, StringComparison.Ordinal); + + private static GitFilePatch ParseFile(string output, List<(int Start, int End)> lines, ref int index) + { + int fileStart = lines[index].Start; + string firstLine = Line(output, lines, index); + + RelativeFilePath path = GitParseValues.ToRelativeFilePath(ReadPathFromFileStart(firstLine)); + RelativeFilePath? originalPath = null; + GitChangeKind kind = GitChangeKind.Modified; + bool isBinary = false; + bool isConflicted = false; + + index++; + + while (index < lines.Count) + { + string line = Line(output, lines, index); + + if (IsFileStart(line) || + line.StartsWith(ConflictHunkPrefix, StringComparison.Ordinal) || + line.StartsWith(HunkPrefix, StringComparison.Ordinal)) + { + break; + } + + ApplyHeaderLine(line, ref kind, ref path, ref originalPath, ref isBinary); + index++; + } + + string header = output[fileStart..RegionEnd(output, lines, index)]; + List hunks = []; + + if (index < lines.Count) + { + string boundary = Line(output, lines, index); + + if (boundary.StartsWith(ConflictHunkPrefix, StringComparison.Ordinal)) + { + // Combined format from an unmerged path is not a patch git apply accepts, so its + // body is skipped rather than misread as ordinary hunks. + isConflicted = true; + index++; + + while (index < lines.Count && !IsFileStart(Line(output, lines, index))) + { + index++; + } + } + else if (!isBinary) + { + while (index < lines.Count && !IsFileStart(Line(output, lines, index))) + { + hunks.Add(ParseHunk(output, lines, ref index)); + } + } + } + + return new GitFilePatch + { + Path = path, + OriginalPath = originalPath, + Kind = kind, + IsBinary = isBinary, + IsConflicted = isConflicted, + Header = header, + Hunks = hunks, + }; + } + + /// + /// Reads the path to use until a more specific header line overrides it: the combined format's + /// single path, or the ordinary format's new-side path. + /// + /// The diff --git or diff --cc line that starts the file. + /// The path found on that line. + /// The line does not carry a recognisable path. + private static string ReadPathFromFileStart(string line) + { + if (line.StartsWith(CombinedHeaderPrefix, StringComparison.Ordinal)) + { + return line[CombinedHeaderPrefix.Length..]; + } + + string remainder = line[GitHeaderPrefix.Length..]; + + // Searched from the end rather than the first match, because the old-side path can itself + // contain the literal text " b/" as part of a directory or file name. + int split = remainder.LastIndexOf(BSidePathMarker, StringComparison.Ordinal); + + return split < 0 + ? throw new GitParseException($"A diff header has no recognisable new-side path: '{line}'.") + : remainder[(split + BSidePathMarker.Length)..]; + } + + private static void ApplyHeaderLine( + string line, + ref GitChangeKind kind, + ref RelativeFilePath path, + ref RelativeFilePath? originalPath, + ref bool isBinary) + { + if (line.StartsWith(RenameFromPrefix, StringComparison.Ordinal)) + { + originalPath = GitParseValues.ToRelativeFilePath(line[RenameFromPrefix.Length..]); + kind = GitChangeKind.Renamed; + } + else if (line.StartsWith(RenameToPrefix, StringComparison.Ordinal)) + { + path = GitParseValues.ToRelativeFilePath(line[RenameToPrefix.Length..]); + kind = GitChangeKind.Renamed; + } + else if (line.StartsWith(NewFileModePrefix, StringComparison.Ordinal)) + { + kind = GitChangeKind.Added; + } + else if (line.StartsWith(DeletedFileModePrefix, StringComparison.Ordinal)) + { + kind = GitChangeKind.Deleted; + } + else if (line.StartsWith(BinaryFilesPrefix, StringComparison.Ordinal) || + line.StartsWith(BinaryPatchPrefix, StringComparison.Ordinal)) + { + isBinary = true; + } + } + + private static GitHunk ParseHunk(string output, List<(int Start, int End)> lines, ref int index) + { + int hunkStart = lines[index].Start; + (int oldStart, int oldCount, int newStart, int newCount, string heading) = + ParseHunkHeader(Line(output, lines, index)); + + int oldLine = oldStart; + int newLine = newStart; + List patchLines = []; + + index++; + + while (index < lines.Count) + { + string line = Line(output, lines, index); + char marker = line.Length > 0 ? line[0] : '\0'; + + if (marker == '\\') + { + // "\ No newline at end of file" belongs to the hunk's text but is not itself a line + // of content. + index++; + continue; + } + + GitPatchLineKind? kind = marker switch + { + ' ' => GitPatchLineKind.Context, + '+' => GitPatchLineKind.Added, + '-' => GitPatchLineKind.Removed, + _ => null, + }; + + if (kind is null) + { + break; + } + + int? oldNumber = kind == GitPatchLineKind.Added ? null : oldLine; + int? newNumber = kind == GitPatchLineKind.Removed ? null : newLine; + + patchLines.Add(new GitPatchLine + { + Kind = kind.Value, + Text = line[1..], + OldNumber = oldNumber, + NewNumber = newNumber, + }); + + if (kind != GitPatchLineKind.Added) + { + oldLine++; + } + + if (kind != GitPatchLineKind.Removed) + { + newLine++; + } + + index++; + } + + string text = output[hunkStart..RegionEnd(output, lines, index)]; + + return new GitHunk + { + OldStart = oldStart, + OldCount = oldCount, + NewStart = newStart, + NewCount = newCount, + Heading = heading, + Lines = patchLines, + Text = text, + }; + } + + /// + /// Parses a hunk's @@ -old,count +new,count @@ heading line. + /// + /// The hunk header line. + /// The two ranges and the heading, which is the empty string when git found none. + /// The line is not a well-formed hunk header. + private static (int OldStart, int OldCount, int NewStart, int NewCount, string Heading) ParseHunkHeader( + string line) + { + // "@@ -" is always exactly four characters, so the old range starts right after it. + int oldRangeStart = 4; + int oldRangeEnd = line.IndexOf(' ', oldRangeStart); + + if (oldRangeEnd < 0) + { + throw new GitParseException($"Malformed hunk header: '{line}'."); + } + + (int oldStart, int oldCount) = ParseRange(line[oldRangeStart..oldRangeEnd], line); + + // Skips the space and the '+' that introduce the new range. + int newRangeStart = oldRangeEnd + 2; + int newRangeEnd = line.IndexOf(' ', newRangeStart); + + if (newRangeEnd < 0) + { + throw new GitParseException($"Malformed hunk header: '{line}'."); + } + + (int newStart, int newCount) = ParseRange(line[newRangeStart..newRangeEnd], line); + + int closingMarker = line.IndexOf("@@", newRangeEnd, StringComparison.Ordinal); + string heading = closingMarker >= 0 + ? line[(closingMarker + 2)..].TrimStart(' ') + : string.Empty; + + return (oldStart, oldCount, newStart, newCount, heading); + } + + /// + /// Parses one side of a hunk header, such as 16,5 or 1. + /// + /// The range text, without its leading - or +. + /// The whole hunk header, used only to report a failure. + /// The start line and the count, which git omits when it is 1. + private static (int Start, int Count) ParseRange(string range, string line) + { + int comma = range.IndexOf(','); + + return comma < 0 + ? (ParseLineNumber(range, line), 1) + : (ParseLineNumber(range[..comma], line), ParseLineNumber(range[(comma + 1)..], line)); + } + + private static int ParseLineNumber(string value, string line) => + int.TryParse(value, NumberStyles.None, CultureInfo.InvariantCulture, out int number) + ? number + : throw new GitParseException($"Malformed hunk header: '{line}'."); +} From c480b6e8052311142954c973608332809ca3f375 Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 20:05:35 +1000 Subject: [PATCH 05/13] [patch] Use US spelling in the patch parser's diagnostics "Recognisable" slipped into a doc comment and its matching exception message. Global constraints require US spelling throughout. --- GitIntegration/Parsing/GitPatchParser.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/GitIntegration/Parsing/GitPatchParser.cs b/GitIntegration/Parsing/GitPatchParser.cs index 1f42a01..336ff38 100644 --- a/GitIntegration/Parsing/GitPatchParser.cs +++ b/GitIntegration/Parsing/GitPatchParser.cs @@ -180,7 +180,7 @@ private static GitFilePatch ParseFile(string output, List<(int Start, int End)> /// /// The diff --git or diff --cc line that starts the file. /// The path found on that line. - /// The line does not carry a recognisable path. + /// The line does not carry a recognizable path. private static string ReadPathFromFileStart(string line) { if (line.StartsWith(CombinedHeaderPrefix, StringComparison.Ordinal)) @@ -195,7 +195,7 @@ private static string ReadPathFromFileStart(string line) int split = remainder.LastIndexOf(BSidePathMarker, StringComparison.Ordinal); return split < 0 - ? throw new GitParseException($"A diff header has no recognisable new-side path: '{line}'.") + ? throw new GitParseException($"A diff header has no recognizable new-side path: '{line}'.") : remainder[(split + BSidePathMarker.Length)..]; } From a97019cbb259186f0be686be802fa48c38a8dd92 Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 20:15:20 +1000 Subject: [PATCH 06/13] [minor] Add Patch, which reads a diff as hunks rather than counts Diff answers which files changed and by how much. Patch answers what changed, which is what a caller drawing a diff or staging a hunk needs. It always passes --no-ext-diff and --no-textconv, because a repository with a gitattributes diff driver otherwise emits something no caller can apply. --- .../Builders/GitPatchBuilderTests.cs | 69 ++++++++ GitIntegration/Builders/GitPatchBuilder.cs | 166 ++++++++++++++++++ GitIntegration/GitRepository.cs | 4 + 3 files changed, 239 insertions(+) create mode 100644 GitIntegration.Test/Builders/GitPatchBuilderTests.cs create mode 100644 GitIntegration/Builders/GitPatchBuilder.cs diff --git a/GitIntegration.Test/Builders/GitPatchBuilderTests.cs b/GitIntegration.Test/Builders/GitPatchBuilderTests.cs new file mode 100644 index 0000000..8407738 --- /dev/null +++ b/GitIntegration.Test/Builders/GitPatchBuilderTests.cs @@ -0,0 +1,69 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.Linq; + +[TestClass] +public class GitPatchBuilderTests +{ + [TestMethod] + public void BuildsTheDefaultPatchVector() + { + RecordingGitProcessRunner runner = new(); + GitPatchBuilder builder = new(runner, TestPaths.Root); + + string[] expectedArguments = + [ + "-C", TestPaths.Root.WeakString, + "--no-pager", + "-c", "core.quotepath=false", + "-c", "color.ui=false", + "diff", + "--no-ext-diff", + "--no-textconv", + "--no-color", + ]; + + Assert.AreSequenceEqual(expectedArguments, builder.BuildArguments()); + } + + [TestMethod] + public void MapsTheOptionFlags() + { + RecordingGitProcessRunner runner = new(); + + GitPatchBuilder staged = new(runner, TestPaths.Root); + _ = staged.Staged(); + Assert.IsTrue(staged.BuildArguments().Contains("--cached")); + + GitPatchBuilder context = new(runner, TestPaths.Root); + _ = context.WithContext(7); + Assert.IsTrue(context.BuildArguments().Contains("-U7")); + + GitPatchBuilder renames = new(runner, TestPaths.Root); + _ = renames.DetectRenames(); + Assert.IsTrue(renames.BuildArguments().Contains("--find-renames")); + } + + [TestMethod] + public void NeverEmitsTheNulSeparator() + { + RecordingGitProcessRunner runner = new(); + GitPatchBuilder builder = new(runner, TestPaths.Root); + + Assert.IsFalse( + builder.BuildArguments().Contains("-z"), + "Patch format is line-based, and -z changes only the name-status framing this builder does not use."); + } + + [TestMethod] + public void RefusesANegativeContextCount() + { + RecordingGitProcessRunner runner = new(); + GitPatchBuilder builder = new(runner, TestPaths.Root); + + _ = Assert.ThrowsExactly(() => _ = builder.WithContext(-1)); + } +} diff --git a/GitIntegration/Builders/GitPatchBuilder.cs b/GitIntegration/Builders/GitPatchBuilder.cs new file mode 100644 index 0000000..7c6808f --- /dev/null +++ b/GitIntegration/Builders/GitPatchBuilder.cs @@ -0,0 +1,166 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System; +using System.Collections.Generic; + +using ktsu.Semantics.Paths; + +/// +/// Reads a patch: the hunks and lines a caller needs to show or stage a change, rather than the +/// per-file counts reports. +/// +public interface IGitPatchBuilder : IGitCommandBuilder +{ + /// Compares the index against HEAD instead of the working tree against the index. + /// The same builder, to allow chaining. + public IGitPatchBuilder Staged(); + + /// Sets how many lines of surrounding context each hunk carries. + /// The number of context lines. Git's own default applies when this is never called. + /// The same builder, to allow chaining. + /// is negative. + public IGitPatchBuilder WithContext(int lines); + + /// Limits the result to this path. May be called more than once. + /// The path, relative to the repository root. + /// The same builder, to allow chaining. + /// is . + public IGitPatchBuilder ForPath(RelativeFilePath path); + + /// + /// Compares against one revision. Replaces any previous revision selection. + /// + /// The revision to compare against. + /// The same builder, to allow chaining. + /// is . + public IGitPatchBuilder Against(GitRefName revision); + + /// + /// Compares two revisions. Replaces any previous revision selection. + /// + /// The revision to compare from. + /// The revision to compare to. + /// The same builder, to allow chaining. + /// + /// or is . + /// + public IGitPatchBuilder Between(GitRefName fromRevision, GitRefName toRevision); + + /// Reports a delete and an add of similar content as a rename. + /// The same builder, to allow chaining. + public IGitPatchBuilder DetectRenames(); +} + +/// +/// Builds git diff --no-ext-diff --no-textconv --no-color and parses its output through +/// . +/// +/// Runs the assembled command. +/// The repository to scope the command to. +internal sealed class GitPatchBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath) + : GitCommandBuilder(runner, repositoryPath), IGitPatchBuilder +{ + private readonly List _paths = []; + + // One slot, so Against and Between cannot combine into a three-revision vector git would reject. + private string[] _revisions = []; + private bool _staged; + private int? _contextLines; + private bool _detectRenames; + + /// + public IGitPatchBuilder Staged() + { + _staged = true; + return this; + } + + /// + public IGitPatchBuilder WithContext(int lines) + { + ArgumentOutOfRangeException.ThrowIfNegative(lines); + _contextLines = lines; + return this; + } + + /// + public IGitPatchBuilder ForPath(RelativeFilePath path) + { + _paths.Add(Ensure.NotNull(path)); + return this; + } + + /// + public IGitPatchBuilder Against(GitRefName revision) + { + _revisions = [Ensure.NotNull(revision).WeakString]; + return this; + } + + /// + public IGitPatchBuilder Between(GitRefName fromRevision, GitRefName toRevision) + { + _revisions = [Ensure.NotNull(fromRevision).WeakString, Ensure.NotNull(toRevision).WeakString]; + return this; + } + + /// + public IGitPatchBuilder DetectRenames() + { + _detectRenames = true; + return this; + } + + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("diff"); + + // Correctness, not tidiness. A repository with a gitattributes diff driver emits + // human-readable output in place of a patch, which no caller can apply, and --no-color + // guards a config setting color.diff to always, which would put escape sequences into text + // that goes back to apply. color.ui on the base vector does not cover that: git lets + // color.diff take precedence over it. + arguments.Add("--no-ext-diff"); + arguments.Add("--no-textconv"); + arguments.Add("--no-color"); + + if (_staged) + { + arguments.Add("--cached"); + } + + if (_contextLines is int contextLines) + { + arguments.Add($"-U{contextLines}"); + } + + if (_detectRenames) + { + arguments.Add("--find-renames"); + } + + if (_revisions.Length > 0) + { + AppendOperands(arguments, _revisions); + } + + if (_paths.Count > 0) + { + arguments.Add("--"); + + foreach (RelativeFilePath path in _paths) + { + arguments.Add(path.WeakString); + } + } + } + + /// + protected override GitPatch ParseResult(GitProcessResult result) => + GitPatchParser.Parse(Ensure.NotNull(result).StandardOutput); +} diff --git a/GitIntegration/GitRepository.cs b/GitIntegration/GitRepository.cs index 5679f04..64547cf 100644 --- a/GitIntegration/GitRepository.cs +++ b/GitIntegration/GitRepository.cs @@ -119,6 +119,10 @@ private static Task IsClonedCoreAsync(IGitProcessRunner runner, AbsoluteDi /// This repository has no . public IGitDiffBuilder Diff() => new GitDiffBuilder(RequireRunner(), RequireLocalPath()); + /// Reads a patch, with the hunks and lines a caller needs to show or stage a change. + /// The builder. + public IGitPatchBuilder Patch() => new GitPatchBuilder(RequireRunner(), RequireLocalPath()); + /// Resolves a revision to the object id it names. /// The revision to resolve. /// A fresh builder. From c2c59be64a62da96ba74cb40412f65bbbda3472f Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 20:35:50 +1000 Subject: [PATCH 07/13] [minor] Add Apply, which puts a patch into the index Staging a hunk is Apply(text).ToIndex(), and unstaging one is the same with Reversed. Checked asks whether a patch would still apply without changing anything, which is what lets a caller refuse cleanly when the working tree moved underneath it. Git reads a patch from standard input or a file, and the process request carries no standard input, so the patch goes to a temporary file that is deleted on every path. The round-trip test is what proves the model and the parser kept every byte. --- .../Builders/GitApplyBuilderTests.cs | 65 ++++++++ .../Integration/GitPatchRoundTripTests.cs | 110 +++++++++++++ GitIntegration/Builders/GitApplyBuilder.cs | 148 ++++++++++++++++++ GitIntegration/GitRepository.cs | 29 ++++ 4 files changed, 352 insertions(+) create mode 100644 GitIntegration.Test/Builders/GitApplyBuilderTests.cs create mode 100644 GitIntegration.Test/Integration/GitPatchRoundTripTests.cs create mode 100644 GitIntegration/Builders/GitApplyBuilder.cs diff --git a/GitIntegration.Test/Builders/GitApplyBuilderTests.cs b/GitIntegration.Test/Builders/GitApplyBuilderTests.cs new file mode 100644 index 0000000..30f8d42 --- /dev/null +++ b/GitIntegration.Test/Builders/GitApplyBuilderTests.cs @@ -0,0 +1,65 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.IO; +using System.Threading.Tasks; + +[TestClass] +public class GitApplyBuilderTests +{ + [TestMethod] + public void MapsTheOptionFlags() + { + RecordingGitProcessRunner runner = new(); + + GitApplyBuilder index = new(runner, TestPaths.Root, "patch"); + _ = index.ToIndex(); + Assert.IsTrue(index.BuildArguments().Contains("--cached")); + + GitApplyBuilder reversed = new(runner, TestPaths.Root, "patch"); + _ = reversed.Reversed(); + Assert.IsTrue(reversed.BuildArguments().Contains("--reverse")); + + GitApplyBuilder checkOnly = new(runner, TestPaths.Root, "patch"); + _ = checkOnly.Checked(); + Assert.IsTrue(checkOnly.BuildArguments().Contains("--check")); + } + + [TestMethod] + public void AlwaysSuppressesWhitespaceWarnings() + { + RecordingGitProcessRunner runner = new(); + GitApplyBuilder builder = new(runner, TestPaths.Root, "patch"); + + Assert.IsTrue( + builder.BuildArguments().Contains("--whitespace=nowarn"), + "Staging content already on disk is not the moment to enforce a whitespace policy the user configured for authoring."); + } + + [TestMethod] + public void RefusesEmptyPatchText() + { + RecordingGitProcessRunner runner = new(); + GitRepository repository = new() { LocalPath = TestPaths.Root, ProcessRunner = runner }; + + _ = Assert.ThrowsExactly(() => _ = repository.Apply(" ")); + } + + [TestMethod] + public async Task DeletesTheTemporaryFileEvenWhenGitFailsAsync() + { + RecordingGitProcessRunner runner = new() { ExitCode = 1, StandardError = "error: corrupt patch" }; + GitApplyBuilder builder = new(runner, TestPaths.Root, "not a patch\n"); + + _ = await Assert.ThrowsExactlyAsync( + async () => await builder.ExecuteAsync().ConfigureAwait(false)).ConfigureAwait(false); + + string path = runner.LastArguments![^1]; + + Assert.IsFalse( + File.Exists(path), + "A failing patch must not leave files behind, and the failure the caller sees is git's, not the cleanup's."); + } +} diff --git a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs new file mode 100644 index 0000000..638e92a --- /dev/null +++ b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs @@ -0,0 +1,110 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; + +using ktsu.Semantics.Strings; + +/// +/// Exercises Apply against a real git binary: the round trip that proves the model, the +/// parser, and the builder kept every byte of a patch between reading it and staging it back. +/// +[TestClass] +[TestCategory("Integration")] +public class GitPatchRoundTripTests +{ + private static readonly GitAuthorName AuthorName = "Fixture Author".As(); + private static readonly GitAuthorEmail AuthorEmail = "fixture@example.com".As(); + + [TestMethod] + public async Task StagingOneHunkLeavesTheOtherUnstagedAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + + // Seed a ten-line file, commit it, then change the second and the tenth line so the diff + // has two separate hunks. + GitInitResult init = await client.Init(repository.Root) + .WithInitialBranch("main".As()) + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + await IntegrationGitFixture.ConfigureIdentityAsync( + init.Repository, AuthorName, AuthorEmail, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + string[] lines = [.. Enumerable.Range(1, 10).Select(number => $"line {number}")]; + repository.WriteFile("f.txt", string.Join('\n', lines) + "\n"); + _ = await init.Repository.Add().All() + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + _ = await init.Repository.Commit("seed".As()) + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + lines[1] = "line 2 changed"; + lines[9] = "line 10 changed"; + repository.WriteFile("f.txt", string.Join('\n', lines) + "\n"); + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + + GitPatch patch = await opened.Patch().WithContext(1).ExecuteAsync().ConfigureAwait(false); + GitFilePatch file = patch.Files.Single(); + + Assert.AreEqual(2, file.Hunks.Count, "The fixture must produce two hunks, or this proves nothing."); + + _ = await opened.Apply(file.PatchFor([file.Hunks[0]])).ToIndex().ExecuteAsync().ConfigureAwait(false); + + IReadOnlyList staged = await opened.Diff().Staged().WithLineCounts() + .ExecuteAsync().ConfigureAwait(false); + IReadOnlyList unstaged = await opened.Diff().WithLineCounts() + .ExecuteAsync().ConfigureAwait(false); + + Assert.AreEqual(1, staged.Single().Insertions); + Assert.AreEqual(1, unstaged.Single().Insertions); + } + + [TestMethod] + public async Task CheckedReportsFailureWhenTheWorkingTreeMovedAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + + // Seed and commit a file, then change it without staging the change. + GitInitResult init = await client.Init(repository.Root) + .WithInitialBranch("main".As()) + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + await IntegrationGitFixture.ConfigureIdentityAsync( + init.Repository, AuthorName, AuthorEmail, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + repository.WriteFile("f.txt", "one\ntwo\nthree\n"); + _ = await init.Repository.Add().All() + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + _ = await init.Repository.Commit("seed".As()) + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + repository.WriteFile("f.txt", "one\nCHANGED\nthree\n"); + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + GitFilePatch file = (await opened.Patch().ExecuteAsync().ConfigureAwait(false)).Files.Single(); + string text = file.PatchFor(file.Hunks); + + repository.WriteFile("f.txt", "something else entirely\n"); + + // Not ToIndex(): --cached checks only the index entry, which nothing here has touched + // since the patch was read, so it would always apply cleanly and prove nothing. Checking + // the default target is what actually sees the working tree that moved. + GitResult result = await opened.Apply(text).Checked() + .TryExecuteAsync().ConfigureAwait(false); + + Assert.IsFalse(result.Success, "A caller's refusal depends on this reporting failure rather than throwing."); + + IReadOnlyList staged = await opened.Diff().Staged().ExecuteAsync().ConfigureAwait(false); + + Assert.AreEqual(0, staged.Count, "--check must change nothing."); + } + + public TestContext TestContext { get; set; } = null!; +} diff --git a/GitIntegration/Builders/GitApplyBuilder.cs b/GitIntegration/Builders/GitApplyBuilder.cs new file mode 100644 index 0000000..cfce755 --- /dev/null +++ b/GitIntegration/Builders/GitApplyBuilder.cs @@ -0,0 +1,148 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System.Collections.Generic; +using System.IO; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +using ktsu.Semantics.Paths; + +/// +/// Puts a patch into the index or the working tree. +/// +public interface IGitApplyBuilder : IGitCommandBuilder +{ + /// Applies the patch to the index rather than the working tree. + /// The same builder, to allow chaining. + public IGitApplyBuilder ToIndex(); + + /// Applies the patch in reverse, which is what unstages or reverts a change. + /// The same builder, to allow chaining. + public IGitApplyBuilder Reversed(); + + /// Reports whether the patch would apply, without changing anything. + /// The same builder, to allow chaining. + public IGitApplyBuilder Checked(); +} + +/// +/// Builds git apply. +/// +/// +/// Git reads a patch from standard input or from a file, and carries +/// no standard input, so the patch text goes to a temporary file whose path is computed once, in the +/// constructor. AppendVerbArguments only names that path as an operand; nothing is written +/// to disk until or actually runs git, and +/// the file is always removed afterwards, in a finally, whether git succeeded or failed. +/// +/// Runs the assembled command. +/// The repository to scope the command to. +/// The patch to apply. +internal sealed class GitApplyBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath, string patchText) + : GitCommandBuilder(runner, repositoryPath), IGitApplyBuilder +{ + private readonly string _patchText = Ensure.NotNull(patchText); + private readonly string _temporaryPath = Path.Combine(Path.GetTempPath(), $"ktsu-git-apply-{Path.GetRandomFileName()}.patch"); + + private bool _toIndex; + private bool _reversed; + private bool _checked; + + /// + public IGitApplyBuilder ToIndex() + { + _toIndex = true; + return this; + } + + /// + public IGitApplyBuilder Reversed() + { + _reversed = true; + return this; + } + + /// + public IGitApplyBuilder Checked() + { + _checked = true; + return this; + } + + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + arguments.Add("apply"); + + // Staging content already on disk is not the moment to enforce a whitespace policy the user + // configured for authoring. + arguments.Add("--whitespace=nowarn"); + + if (_toIndex) + { + arguments.Add("--cached"); + } + + if (_reversed) + { + arguments.Add("--reverse"); + } + + if (_checked) + { + arguments.Add("--check"); + } + + AppendOperands(arguments, _temporaryPath); + } + + /// + protected override GitCompleted ParseResult(GitProcessResult result) => + new() { Arguments = Ensure.NotNull(result).Arguments }; + + /// + public override async Task ExecuteAsync(CancellationToken cancellationToken = default) + { + WritePatchFile(); + + try + { + return await base.ExecuteAsync(cancellationToken).ConfigureAwait(false); + } + finally + { + File.Delete(_temporaryPath); + } + } + + /// + public override async Task> TryExecuteAsync(CancellationToken cancellationToken = default) + { + WritePatchFile(); + + try + { + return await base.TryExecuteAsync(cancellationToken).ConfigureAwait(false); + } + finally + { + File.Delete(_temporaryPath); + } + } + + /// + /// Writes the patch text to as UTF-8 with no byte order mark, + /// preserving its line endings exactly. + /// + /// + /// A rewritten line ending, or a byte order mark git reads as part of the first line, makes the + /// patch unapplyable. + /// + private void WritePatchFile() => + File.WriteAllText(_temporaryPath, _patchText, new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)); +} diff --git a/GitIntegration/GitRepository.cs b/GitIntegration/GitRepository.cs index 64547cf..199f4f7 100644 --- a/GitIntegration/GitRepository.cs +++ b/GitIntegration/GitRepository.cs @@ -123,6 +123,35 @@ private static Task IsClonedCoreAsync(IGitProcessRunner runner, AbsoluteDi /// The builder. public IGitPatchBuilder Patch() => new GitPatchBuilder(RequireRunner(), RequireLocalPath()); + /// + /// Puts a patch into the index or the working tree. Staging one hunk of a file's patch is + /// Apply(text).ToIndex(); unstaging one already staged is the same call with + /// Reversed() added. + /// + /// + /// The patch text, typically applied to a subset of a file's + /// hunks. + /// + /// A fresh builder. + /// + /// is null, empty, or all whitespace. Git reports an empty patch as + /// a corrupt-patch error that explains nothing, so this is caught before the process is started. + /// + /// This repository has no . + public IGitApplyBuilder Apply(string patchText) + { + // Argument validation before RequireRunner(), matching every other verb taking an operand. + if (string.IsNullOrWhiteSpace(patchText)) + { + throw new ArgumentException( + "A patch needs at least some content. An empty patch reaches git as a corrupt-patch " + + "error that explains nothing.", + nameof(patchText)); + } + + return new GitApplyBuilder(RequireRunner(), RequireLocalPath(), patchText); + } + /// Resolves a revision to the object id it names. /// The revision to resolve. /// A fresh builder. From 697d1dca2752919d5d4a7f34b11a4d1c9aa79e7d Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 20:38:26 +1000 Subject: [PATCH 08/13] [major] Use US spelling throughout the sources and the docs The repository's conventions call for US spelling in identifiers, comments and user-facing strings, and roughly 190 British forms had accumulated against that. This rewrites all of them, in prose, in exception messages, in test method names and in the design documents. Two public members are renamed with them, which is what makes this a major release: IGitSubmoduleUpdateBuilder.Initialise becomes Initialize, and the enum member GitSubmoduleState.Uninitialised becomes Uninitialized. The captured patch fixtures are left alone. They hold bytes git emitted, and nothing in this repository's conventions applies to those. --- .../Builders/GitBranchListBuilderTests.cs | 2 +- .../Builders/GitCommandBuilderTests.cs | 2 +- .../Builders/GitInitBuilderTests.cs | 2 +- .../Builders/GitPullBuilderTests.cs | 2 +- .../Builders/GitStatusBuilderTests.cs | 2 +- .../Builders/GitSubmoduleBuilderTests.cs | 12 ++--- .../RunCommandGitProcessRunnerTests.cs | 10 ++-- .../Hosting/AzureDevOpsProviderTests.cs | 8 ++-- .../Hosting/GitHubDeviceFlowTests.cs | 6 +-- .../Hosting/GitHubProviderTests.cs | 8 ++-- .../Hosting/GitProviderTests.cs | 14 +++--- .../Integration/GitRemoteSyncTests.cs | 4 +- .../Integration/GitRoundTripTests.cs | 36 +++++++-------- .../Integration/GitSubmoduleTests.cs | 6 +-- .../Integration/TemporaryRepository.cs | 2 +- .../Parsing/GitDiffParserTests.cs | 2 +- .../Parsing/GitLogParserTests.cs | 2 +- .../Parsing/GitStatusParserTests.cs | 6 +-- .../Parsing/GitWorktreeParserTests.cs | 2 +- GitIntegration/Builders/GitCommandBuilder.cs | 6 +-- GitIntegration/Builders/GitCommitBuilder.cs | 2 +- GitIntegration/Builders/GitFetchBuilder.cs | 2 +- GitIntegration/Builders/GitInitBuilder.cs | 2 +- GitIntegration/Builders/GitLogBuilder.cs | 2 +- GitIntegration/Builders/GitPullBuilder.cs | 8 ++-- GitIntegration/Builders/GitPushBuilder.cs | 4 +- GitIntegration/Builders/GitStatusBuilder.cs | 2 +- .../Builders/GitSubmoduleUpdateBuilder.cs | 18 ++++---- GitIntegration/Execution/GitExceptions.cs | 2 +- .../Execution/RunCommandGitProcessRunner.cs | 4 +- GitIntegration/GitHubProvider.cs | 28 +++++------ GitIntegration/GitProvider.cs | 24 +++++----- GitIntegration/Hosting/AzureDevOpsProvider.cs | 14 +++--- GitIntegration/Hosting/GitHubDeviceFlow.cs | 18 ++++---- GitIntegration/Hosting/IGitHostingProvider.cs | 6 +-- GitIntegration/IGitClient.cs | 2 +- GitIntegration/Models/GitCommit.cs | 2 +- GitIntegration/Models/GitEnums.cs | 8 ++-- GitIntegration/Models/GitInitResult.cs | 4 +- GitIntegration/Models/GitRefUpdate.cs | 2 +- GitIntegration/Models/GitSubmodule.cs | 6 +-- GitIntegration/Parsing/GitPushParser.cs | 4 +- GitIntegration/Parsing/GitStatusParser.cs | 4 +- GitIntegration/Parsing/GitSubmoduleParser.cs | 10 ++-- GitIntegration/Parsing/GitVersionParser.cs | 4 +- .../SemanticTypes/GitProviderTypes.cs | 4 +- GitIntegration/SemanticTypes/GitRefTypes.cs | 2 +- ...9-gitintegration-v2-phase1-2-foundation.md | 10 ++-- ...itintegration-v2-phase3-read-only-verbs.md | 34 +++++++------- ...gitintegration-v2-phase4-mutating-verbs.md | 46 +++++++++---------- ...0-gitintegration-v2-phase5a-remote-sync.md | 20 ++++---- ...21-gitintegration-phase5b-hosting-layer.md | 16 +++---- .../2026-09-21-worktrees-and-github-auth.md | 36 +++++++-------- .../plans/2026-09-24-patch-verbs.md | 2 +- .../2026-08-21-azure-devops-rest-findings.md | 4 +- .../2026-08-19-gitintegration-v2-design.md | 14 +++--- ...-21-gitintegration-hosting-layer-design.md | 12 ++--- ...-09-21-worktrees-and-github-auth-design.md | 18 ++++---- .../specs/2026-09-24-patch-verbs-design.md | 2 +- 59 files changed, 268 insertions(+), 268 deletions(-) diff --git a/GitIntegration.Test/Builders/GitBranchListBuilderTests.cs b/GitIntegration.Test/Builders/GitBranchListBuilderTests.cs index f8c970d..3906eb3 100644 --- a/GitIntegration.Test/Builders/GitBranchListBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitBranchListBuilderTests.cs @@ -37,7 +37,7 @@ public void PinsTheExactFormatStringSentToGit() { // Asserted literally rather than through GitOutputFormats. The leading %(refname) is what // lets the parser tell a local branch from a remote-tracking one and drop the remote HEAD - // symbolic reference, so silently losing it would break both behaviours at once. + // symbolic reference, so silently losing it would break both behaviors at once. RecordingGitProcessRunner runner = new(); GitBranchListBuilder builder = new(runner, TestPaths.Root); diff --git a/GitIntegration.Test/Builders/GitCommandBuilderTests.cs b/GitIntegration.Test/Builders/GitCommandBuilderTests.cs index d0b2826..bb70454 100644 --- a/GitIntegration.Test/Builders/GitCommandBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitCommandBuilderTests.cs @@ -10,7 +10,7 @@ namespace ktsu.GitIntegration.Test; [TestClass] public class GitCommandBuilderTests { - /// A minimal concrete builder, exercising only the base class behaviour. + /// A minimal concrete builder, exercising only the base class behavior. private sealed class EchoBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath? repositoryPath) : GitCommandBuilder(runner, repositoryPath) { diff --git a/GitIntegration.Test/Builders/GitInitBuilderTests.cs b/GitIntegration.Test/Builders/GitInitBuilderTests.cs index df71de4..293390c 100644 --- a/GitIntegration.Test/Builders/GitInitBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitInitBuilderTests.cs @@ -70,7 +70,7 @@ public void RejectsNullArguments() } [TestMethod] - public async Task ProbesBeforeInitialisingAndReportsAFreshRepositoryAsync() + public async Task ProbesBeforeInitializingAndReportsAFreshRepositoryAsync() { ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() .Then(standardError: "fatal: not a git repository (or any of the parent directories): .git\n", exitCode: 128) diff --git a/GitIntegration.Test/Builders/GitPullBuilderTests.cs b/GitIntegration.Test/Builders/GitPullBuilderTests.cs index 5c0b801..d81994f 100644 --- a/GitIntegration.Test/Builders/GitPullBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitPullBuilderTests.cs @@ -195,7 +195,7 @@ public async Task ThrowsConflictWhenTheMergeLeavesConflictsAsync() } [TestMethod] - public async Task RecognisesARebaseConflictTooAsync() + public async Task RecognizesARebaseConflictTooAsync() { // A rebase reports its conflicts with different prose but the same "CONFLICT" marker, and // leaves the repository mid-rebase rather than mid-merge. Both are conflicts to a caller. diff --git a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs index 828aa3e..c64e9f0 100644 --- a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs @@ -101,7 +101,7 @@ public void ConfigurationMethodsReturnTheSameBuilderForChaining() } [TestMethod] - public void RejectsAnUnrecognisedUntrackedFilesMode() + public void RejectsAnUnrecognizedUntrackedFilesMode() { RecordingGitProcessRunner runner = new(); GitStatusBuilder builder = new(runner, TestPaths.Root); diff --git a/GitIntegration.Test/Builders/GitSubmoduleBuilderTests.cs b/GitIntegration.Test/Builders/GitSubmoduleBuilderTests.cs index 016b875..90dd770 100644 --- a/GitIntegration.Test/Builders/GitSubmoduleBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitSubmoduleBuilderTests.cs @@ -94,9 +94,9 @@ public async Task KeepsTheRecordedGitlinkApartFromTheCheckedOutCommitAsync() } [TestMethod] - public async Task ReportsNoCheckedOutCommitForAnUninitialisedSubmoduleAsync() + public async Task ReportsNoCheckedOutCommitForAnUninitializedSubmoduleAsync() { - // git prints the recorded gitlink again for an uninitialised submodule, which would make it + // git prints the recorded gitlink again for an uninitialized submodule, which would make it // indistinguishable from a synchronised one if it were reported verbatim. Nothing is checked // out there, so nothing is reported. Note there is no describe suffix either. ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() @@ -107,7 +107,7 @@ public async Task ReportsNoCheckedOutCommitForAnUninitialisedSubmoduleAsync() IReadOnlyList submodules = await builder.ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); - Assert.AreEqual(GitSubmoduleState.Uninitialised, submodules[0].State); + Assert.AreEqual(GitSubmoduleState.Uninitialized, submodules[0].State); Assert.IsNull(submodules[0].CheckedOutSha); Assert.IsNull(submodules[0].Describe); } @@ -326,7 +326,7 @@ public void MapsEveryOptionToItsFlag() RecordingGitProcessRunner runner = new(); GitSubmoduleUpdateBuilder builder = new(runner, TestPaths.Root); - _ = builder.Initialise().Recursive().FromRemote().Force().WithDepth(1); + _ = builder.Initialize().Recursive().FromRemote().Force().WithDepth(1); string[] arguments = [.. builder.BuildArguments()]; @@ -363,7 +363,7 @@ public void ConfigurationMethodsReturnTheSameBuilderForChaining() RecordingGitProcessRunner runner = new(); GitSubmoduleUpdateBuilder builder = new(runner, TestPaths.Root); - Assert.AreSame(builder, builder.Initialise().Recursive().FromRemote().Force().WithDepth(1)); + Assert.AreSame(builder, builder.Initialize().Recursive().FromRemote().Force().WithDepth(1)); } } @@ -434,7 +434,7 @@ public void PushEmitsItsOwnValueSet() } [TestMethod] - public void RejectsAnUnrecognisedEnumValue() + public void RejectsAnUnrecognizedEnumValue() { RecordingGitProcessRunner runner = new(); diff --git a/GitIntegration.Test/Execution/RunCommandGitProcessRunnerTests.cs b/GitIntegration.Test/Execution/RunCommandGitProcessRunnerTests.cs index b8b666c..dd491a0 100644 --- a/GitIntegration.Test/Execution/RunCommandGitProcessRunnerTests.cs +++ b/GitIntegration.Test/Execution/RunCommandGitProcessRunnerTests.cs @@ -71,11 +71,11 @@ public async Task CallerCancellationSurfacesAsOperationCanceledAsync() Timeout = TimeSpan.FromMinutes(5), }); - using CancellationTokenSource alreadyCancelled = new(); - await alreadyCancelled.CancelAsync().ConfigureAwait(false); + using CancellationTokenSource alreadyCanceled = new(); + await alreadyCanceled.CancelAsync().ConfigureAwait(false); OperationCanceledException exception = await Assert.ThrowsExactlyAsync( - async () => await runner.RunAsync(new GitProcessRequest { Arguments = ["--version"] }, alreadyCancelled.Token).ConfigureAwait(false)).ConfigureAwait(false); + async () => await runner.RunAsync(new GitProcessRequest { Arguments = ["--version"] }, alreadyCanceled.Token).ConfigureAwait(false)).ConfigureAwait(false); Assert.IsNotInstanceOfType(exception); } @@ -130,7 +130,7 @@ await Assert.ThrowsExactlyAsync( [TestMethod] public async Task CallerCancellationMidRunSurfacesAsOperationCanceledAsync() { - // CallerCancellationSurfacesAsOperationCanceledAsync uses a pre-cancelled token, so + // CallerCancellationSurfacesAsOperationCanceledAsync uses a pre-canceled token, so // ktsu.RunCommand throws at its own entry point and the post-return guard in RunAsync is // never reached. This test cancels while the invocation is in flight, which is what can // drive execution into that guard, where the caller's cancellation must be re-raised as a @@ -239,7 +239,7 @@ await Assert.ThrowsExactlyAsync( } [TestMethod] - public async Task MutatingOptionsAfterConstructionDoesNotChangeBehaviourAsync() + public async Task MutatingOptionsAfterConstructionDoesNotChangeBehaviorAsync() { GitOptions options = new() { ExecutablePath = "dotnet" }; RunCommandGitProcessRunner runner = new(options); diff --git a/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs b/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs index ebc0882..7e416f9 100644 --- a/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs +++ b/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs @@ -36,7 +36,7 @@ public sealed class AzureDevOpsProviderTests /// like oversights and are not. The synthetic user npaulk is Microsoft's placeholder, kept /// so a reader comparing a fixture against the published sample sees the same value rather than /// wondering which fields this library altered; and homepage retains the real - /// docs.microsoft.com domain with only the organisation path segment swapped, for the same + /// docs.microsoft.com domain with only the organization path segment swapped, for the same /// reason. Neither is a credential or an internal hostname, both are test-only assets that are /// never packed, and no request is ever issued against either. /// @@ -129,7 +129,7 @@ private static string WithDraftFlag(string json) => /// /// Builds a single-repository list response carrying only the fields /// 's mapping reads, with a caller-supplied name — used to - /// drive 's containment behaviour without depending on the + /// drive 's containment behavior without depending on the /// full captured fixture's real repository names. /// /// The value to send as the repository's name field, or to omit it. @@ -776,7 +776,7 @@ public async Task MapsActiveCompletedAndAbandonedAsync() } [TestMethod] - public async Task TranslatesAnUnrecognisedPullRequestStatusToGitHostingRequestExceptionAsync() + public async Task TranslatesAnUnrecognizedPullRequestStatusToGitHostingRequestExceptionAsync() { using FakeHttpMessageHandler handler = new FakeHttpMessageHandler() .Respond(HttpStatusCode.OK, SinglePullRequestListResponse("notSet"), ("Content-Type", "application/json")); @@ -792,7 +792,7 @@ public async Task TranslatesAnUnrecognisedPullRequestStatusToGitHostingRequestEx .ConfigureAwait(false); Assert.AreEqual(HttpStatusCode.OK, exception.StatusCode); - StringAssert.Contains(exception.Message, "unrecognised pull request status"); + StringAssert.Contains(exception.Message, "unrecognized pull request status"); StringAssert.Contains(exception.ResponseBody, "\"status\": \"notSet\""); } diff --git a/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs b/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs index 243d125..00a1307 100644 --- a/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs +++ b/GitIntegration.Test/Hosting/GitHubDeviceFlowTests.cs @@ -167,7 +167,7 @@ await Assert.ThrowsExactlyAsync( } [TestMethod] - public async Task ReportsAnUnrecognisedErrorCodeAsARequestFailureAsync() + public async Task ReportsAnUnrecognizedErrorCodeAsARequestFailureAsync() { using FakeHttpMessageHandler handler = new(); _ = handler.Respond(HttpStatusCode.OK, DeviceCodeBody, ("Content-Type", "application/json")); @@ -180,7 +180,7 @@ public async Task ReportsAnUnrecognisedErrorCodeAsARequestFailureAsync() GitHubDeviceCode code = await flow.RequestDeviceCodeAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); // The deliberate default for a code this library was not taught: a request fault, not an - // authentication failure, so an unrecognised error never invites retrying a sign-in. + // authentication failure, so an unrecognized error never invites retrying a sign-in. GitHostingRequestException exception = await Assert.ThrowsExactlyAsync( async () => await flow.WaitForTokenAsync(code, TestContext.CancellationTokenSource.Token).ConfigureAwait(false)) @@ -413,7 +413,7 @@ public async Task CancelsDuringTheWaitBetweenPollsAsync() using CancellationTokenSource cts = new(); TaskCompletionSource waitStarted = new(TaskCreationOptions.RunContinuationsAsynchronously); - // Never completes on its own: it only resolves when the token passed to it is cancelled, + // Never completes on its own: it only resolves when the token passed to it is canceled, // which is exactly the wait this test needs to cancel into rather than before. GitHubDeviceFlow flow = CreateFlow(handler, delay: (_, cancellationToken) => { diff --git a/GitIntegration.Test/Hosting/GitHubProviderTests.cs b/GitIntegration.Test/Hosting/GitHubProviderTests.cs index 1c0778f..63a532c 100644 --- a/GitIntegration.Test/Hosting/GitHubProviderTests.cs +++ b/GitIntegration.Test/Hosting/GitHubProviderTests.cs @@ -76,7 +76,7 @@ private static string SinglePullRequestArrayReplacing(string original, string re /// /// Builds a single-repository response carrying only the fields 's /// mapping reads, with a caller-supplied name — used to drive - /// 's containment behaviour without depending on the full + /// 's containment behavior without depending on the full /// captured fixture's real repository names. /// /// The value to send as the repository's name field. @@ -193,10 +193,10 @@ public async Task IssuesNoOwnerTypeProbeWithoutACredentialAsync() } [TestMethod] - public async Task EnumeratesAnOrganisationOnTheOrgRouteWhenAuthenticatedAsync() + public async Task EnumeratesAnOrganizationOnTheOrgRouteWhenAuthenticatedAsync() { // The case this whole route selection exists for. GET /users/{login}/repos is public-only even - // with a token, so an organisation's private repositories were invisible to a credential that + // with a token, so an organization's private repositories were invisible to a credential that // could plainly see them — the divergence from AzureDevOpsProvider, which reports everything // its token reaches, under one interface that promises the same coverage of both. // @@ -257,7 +257,7 @@ public async Task EnumeratesTheCredentialsOwnAccountOnTheCurrentUserRouteAsync() Assert.AreEqual("/user/repos", handler.Requests[2].Uri.AbsolutePath); // Owner affiliation, not the unfiltered default: GET /user/repos with no affiliation also - // returns repositories the account merely collaborates on or reaches through an organisation, + // returns repositories the account merely collaborates on or reaches through an organization, // which would report another owner's work under this owner's name. Assert.AreEqual("?affiliation=owner", handler.Requests[2].Uri.Query); diff --git a/GitIntegration.Test/Hosting/GitProviderTests.cs b/GitIntegration.Test/Hosting/GitProviderTests.cs index cb03e83..9d5ea23 100644 --- a/GitIntegration.Test/Hosting/GitProviderTests.cs +++ b/GitIntegration.Test/Hosting/GitProviderTests.cs @@ -146,7 +146,7 @@ public void ThrowsWhenTheCredentialSourceReturnsNull() { // Returning null is a caller bug, not a way to say "unauthenticated" — HostingCredential.None // says that explicitly. Treating null as None would hide the bug, which is the same reason an - // unrecognised Credential subtype throws rather than proceeding. + // unrecognized Credential subtype throws rather than proceeding. TestProvider provider = new() { Owner = "octocat".As(), @@ -181,14 +181,14 @@ public void ProceedsUnauthenticatedForCredentialWithNothing() } [TestMethod] - public void ThrowsForAnUnrecognisedCredentialSubtype() + public void ThrowsForAnUnrecognizedCredentialSubtype() { - PersonaGUID persona = SeedCredential(new UnrecognisedCredential()); + PersonaGUID persona = SeedCredential(new UnrecognizedCredential()); TestProvider provider = CreateProvider(persona); InvalidOperationException exception = Assert.ThrowsExactly(() => _ = provider.CallResolveCredential()); - StringAssert.Contains(exception.Message, nameof(UnrecognisedCredential)); + StringAssert.Contains(exception.Message, nameof(UnrecognizedCredential)); } [TestMethod] @@ -237,12 +237,12 @@ public void ReportsUnauthenticatedForCredentialWithNothing() } [TestMethod] - public void ReportsUnauthenticatedForAnUnrecognisedCredentialSubtype() + public void ReportsUnauthenticatedForAnUnrecognizedCredentialSubtype() { // A subtype ResolveCredential throws on can never be applied to a request, so reporting // authenticated would be false. Reported rather than thrown: a property getter that throws // would make a plain "if (provider.IsAuthenticated)" a hazard. - PersonaGUID persona = SeedCredential(new UnrecognisedCredential()); + PersonaGUID persona = SeedCredential(new UnrecognizedCredential()); TestProvider provider = CreateProvider(persona); Assert.IsFalse(provider.IsAuthenticated); @@ -292,7 +292,7 @@ public void EachProviderOwnsItsSharedTransportRatherThanInheritingOne() } } - private sealed class UnrecognisedCredential : Credential; + private sealed class UnrecognizedCredential : Credential; // The minimal subclass a test needs to reach GitProvider's protected members. Its own three // abstract overrides are never exercised here — this class exists only to expose diff --git a/GitIntegration.Test/Integration/GitRemoteSyncTests.cs b/GitIntegration.Test/Integration/GitRemoteSyncTests.cs index e7ec0be..be4ef19 100644 --- a/GitIntegration.Test/Integration/GitRemoteSyncTests.cs +++ b/GitIntegration.Test/Integration/GitRemoteSyncTests.cs @@ -15,7 +15,7 @@ namespace ktsu.GitIntegration.Test; /// /// /// The remote is a bare repository on the local filesystem, which git treats exactly like any other -/// remote. That gives real push negotiation and real rejection behaviour with no network and no +/// remote. That gives real push negotiation and real rejection behavior with no network and no /// credentials — the two things that would make these tests flaky or unrunnable in CI. /// [TestClass] @@ -301,7 +301,7 @@ public async Task PushingTwiceReportsUpToDateAsync() [TestMethod] public async Task ARejectedPushThrowsAndCarriesTheDetailAsync() { - // The behaviour the whole push design exists for: git exits non-zero and still reports + // The behavior the whole push design exists for: git exits non-zero and still reports // exactly which reference it refused and why. CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); diff --git a/GitIntegration.Test/Integration/GitRoundTripTests.cs b/GitIntegration.Test/Integration/GitRoundTripTests.cs index 7df366a..0c60fd3 100644 --- a/GitIntegration.Test/Integration/GitRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitRoundTripTests.cs @@ -28,14 +28,14 @@ public class GitRoundTripTests private static readonly GitAuthorEmail AuthorEmail = "fixture@example.com".As(); /// - /// Initialises a repository with a deterministic identity and initial branch. + /// Initializes a repository with a deterministic identity and initial branch. /// /// /// The identity is written into the repository's own config rather than taken from the host, /// so the tests neither depend on a configured user nor disturb one. The initial branch is /// named explicitly for the same reason: init.defaultBranch varies by machine. /// - private static async Task InitialiseAsync( + private static async Task InitializeAsync( TemporaryRepository temporary, CancellationToken cancellationToken) { @@ -63,7 +63,7 @@ public async Task InitCreatesARepositoryAndReportsItAsFreshAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); Assert.IsTrue(await repository.IsClonedAsync(cancellationToken).ConfigureAwait(false)); } @@ -75,7 +75,7 @@ public async Task InitReportsAnExistingRepositoryAsAlreadyExistingAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - _ = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + _ = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); GitInitResult second = await IntegrationGitFixture.CreateClient() .Init(temporary.Root) @@ -91,7 +91,7 @@ public async Task AddAndCommitProduceAReadableCommitAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -118,7 +118,7 @@ public async Task CommittingWithNothingStagedThrowsTheDedicatedExceptionAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -138,7 +138,7 @@ public async Task StatusReflectsStagedAndUntrackedWorkAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -165,7 +165,7 @@ public async Task StatusReportsUntrackedWorkEvenWhereTheHostHidesItAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -202,7 +202,7 @@ public async Task BranchCreateCheckoutAndDeleteRoundTripAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -239,7 +239,7 @@ public async Task TagCreateListAndDeleteRoundTripAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -288,14 +288,14 @@ public async Task TagCreateListAndDeleteRoundTripAsync() public async Task CheckoutResolvesATagRatherThanAFileOfTheSameNameAsync() { // Checkout emits a trailing "--" rather than a leading --end-of-options, and this is the - // behaviour that choice buys beyond compatibility with git <= 2.43: the operand is read as a + // behavior that choice buys beyond compatibility with git <= 2.43: the operand is read as a // revision, so a tag whose name also matches a path on disk resolves to the tag instead of // silently restoring the file. CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -333,7 +333,7 @@ public async Task DiffReportsLineCountsFromARealRepositoryAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); // A NUL byte is what makes git classify a file as binary, and a binary file is the case the // nullable counts exist for. @@ -390,7 +390,7 @@ public async Task DiffReportsLineCountsForARenameAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("before.txt", "one\ntwo\nthree\nfour\nfive\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -422,7 +422,7 @@ public async Task RemoteAddSetUrlAndRemoveRoundTripAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); GitRemoteName origin = "origin".As(); GitRepositoryRemotePath first = "https://example.com/one.git".As(); @@ -455,7 +455,7 @@ public async Task CloneReproducesTheSourceHistoryAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository source = new(); - GitRepository origin = await InitialiseAsync(source, cancellationToken).ConfigureAwait(false); + GitRepository origin = await InitializeAsync(source, cancellationToken).ConfigureAwait(false); source.WriteFile("a.txt", "one\n"); _ = await origin.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -485,7 +485,7 @@ public async Task CloneRefusesANonEmptyDestinationAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository source = new(); - _ = await InitialiseAsync(source, cancellationToken).ConfigureAwait(false); + _ = await InitializeAsync(source, cancellationToken).ConfigureAwait(false); using TemporaryRepository occupied = new(); occupied.WriteFile("in-the-way.txt", "x"); @@ -504,7 +504,7 @@ public async Task DiffReportsAStagedRenameAsync() await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("before.txt", "line1\nline2\nline3\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); diff --git a/GitIntegration.Test/Integration/GitSubmoduleTests.cs b/GitIntegration.Test/Integration/GitSubmoduleTests.cs index 0d34e91..084a319 100644 --- a/GitIntegration.Test/Integration/GitSubmoduleTests.cs +++ b/GitIntegration.Test/Integration/GitSubmoduleTests.cs @@ -165,7 +165,7 @@ await IntegrationGitFixture.ConfigureIdentityAsync(checkout, AuthorName, AuthorE } [TestMethod] - public async Task ReportsAnUninitialisedSubmoduleAndThenUpdatesItAsync() + public async Task ReportsAnUninitializedSubmoduleAndThenUpdatesItAsync() { CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); @@ -200,7 +200,7 @@ public async Task ReportsAnUninitialisedSubmoduleAndThenUpdatesItAsync() await clone.Submodules().ExecuteAsync(cancellationToken).ConfigureAwait(false); Assert.AreEqual(1, before.Count); - Assert.AreEqual(GitSubmoduleState.Uninitialised, before[0].State); + Assert.AreEqual(GitSubmoduleState.Uninitialized, before[0].State); // Nothing is checked out, so nothing is reported — even though git prints the recorded // gitlink again on that line, which would otherwise make this look synchronised. @@ -214,7 +214,7 @@ public async Task ReportsAnUninitialisedSubmoduleAndThenUpdatesItAsync() [ "-C", cloneDirectory.RootPath, "-c", "protocol.file.allow=always", - .. clone.UpdateSubmodules().Initialise().Recursive().BuildArguments(), + .. clone.UpdateSubmodules().Initialize().Recursive().BuildArguments(), ], }, cancellationToken).ConfigureAwait(false); diff --git a/GitIntegration.Test/Integration/TemporaryRepository.cs b/GitIntegration.Test/Integration/TemporaryRepository.cs index 99fcbd4..a7581a0 100644 --- a/GitIntegration.Test/Integration/TemporaryRepository.cs +++ b/GitIntegration.Test/Integration/TemporaryRepository.cs @@ -67,7 +67,7 @@ public void WriteFile(string relativePath, string contents) /// /// Two defences rather than one, and the second is what makes the first unnecessary to trust. /// is the API that means "append these segments" — it has - /// no discarding behaviour to guard against at all, so the escape is impossible by construction + /// no discarding behavior to guard against at all, so the escape is impossible by construction /// rather than merely checked for. The explicit rejection is kept above it because a rooted path /// reaching here is a mistake in the calling test worth naming, and silently nesting it under the /// root would hide that. diff --git a/GitIntegration.Test/Parsing/GitDiffParserTests.cs b/GitIntegration.Test/Parsing/GitDiffParserTests.cs index b60955b..9ef98a1 100644 --- a/GitIntegration.Test/Parsing/GitDiffParserTests.cs +++ b/GitIntegration.Test/Parsing/GitDiffParserTests.cs @@ -93,7 +93,7 @@ public void MapsEveryDocumentedStatusLetter() } [TestMethod] - public void ReportsAnUnrecognisedStatusLetterAsUnknownRatherThanThrowing() + public void ReportsAnUnrecognizedStatusLetterAsUnknownRatherThanThrowing() { // git emits 'B' for a broken pairing and 'X' for a state it calls a bug. Neither is worth // failing an entire diff over, and unlike the status format the set is not closed, so an diff --git a/GitIntegration.Test/Parsing/GitLogParserTests.cs b/GitIntegration.Test/Parsing/GitLogParserTests.cs index b4b3479..5a299a1 100644 --- a/GitIntegration.Test/Parsing/GitLogParserTests.cs +++ b/GitIntegration.Test/Parsing/GitLogParserTests.cs @@ -90,7 +90,7 @@ public void ReadsTheAuthorAndCommitterSeparately() [TestMethod] public void PreservesTheCommittedTimeZoneOffset() { - // %aI is strict ISO-8601 with the offset the commit was made in. Normalising to UTC would + // %aI is strict ISO-8601 with the offset the commit was made in. Normalizing to UTC would // throw away the local time, which is information a caller may want. IReadOnlyList commits = GitLogParser.Parse(MergeCommit); diff --git a/GitIntegration.Test/Parsing/GitStatusParserTests.cs b/GitIntegration.Test/Parsing/GitStatusParserTests.cs index 6b4dbb7..2ba8eef 100644 --- a/GitIntegration.Test/Parsing/GitStatusParserTests.cs +++ b/GitIntegration.Test/Parsing/GitStatusParserTests.cs @@ -10,7 +10,7 @@ public class GitStatusParserTests { // Fixtures are inline rather than files on disk: git's porcelain v2 format embeds NUL, which // makes a fixture file binary, unreviewable in a diff, and exempt from this repo's EOL - // normalisation. NUL is written as the six-character escape u0000 (backslash-u-0-0-0-0) rather + // normalization. NUL is written as the six-character escape u0000 (backslash-u-0-0-0-0) rather // than backslash-zero, so it can never read as an octal escape when followed by a digit. private const string Nul = "\u0000"; @@ -191,7 +191,7 @@ public void ReadsAnIgnoredFile() } [TestMethod] - public void RejectsAnUnrecognisedRecordPrefix() + public void RejectsAnUnrecognizedRecordPrefix() { Assert.ThrowsExactly( () => GitStatusParser.Parse("x something" + Nul)); @@ -218,7 +218,7 @@ public void RejectsARenameRecordWithNoFollowingOriginalPath() } [TestMethod] - public void IgnoresAHeaderItDoesNotRecognise() + public void IgnoresAHeaderItDoesNotRecognize() { // Forward compatibility: a future git adding a header must not break every caller. GitStatus status = GitStatusParser.Parse( diff --git a/GitIntegration.Test/Parsing/GitWorktreeParserTests.cs b/GitIntegration.Test/Parsing/GitWorktreeParserTests.cs index 45e0f5f..35139ba 100644 --- a/GitIntegration.Test/Parsing/GitWorktreeParserTests.cs +++ b/GitIntegration.Test/Parsing/GitWorktreeParserTests.cs @@ -173,7 +173,7 @@ public void RejectsARecordWithNoWorktreePath() } [TestMethod] - public void IgnoresAnAttributeItDoesNotRecognise() + public void IgnoresAnAttributeItDoesNotRecognize() { // Git may add attributes. An unknown one must not fail a listing that is otherwise readable. string output = diff --git a/GitIntegration/Builders/GitCommandBuilder.cs b/GitIntegration/Builders/GitCommandBuilder.cs index 9b14aae..cc71e7f 100644 --- a/GitIntegration/Builders/GitCommandBuilder.cs +++ b/GitIntegration/Builders/GitCommandBuilder.cs @@ -10,7 +10,7 @@ namespace ktsu.GitIntegration; using ktsu.Semantics.Paths; /// -/// The shared behaviour of every git command builder: global argument injection, execution, and +/// The shared behavior of every git command builder: global argument injection, execution, and /// failure translation. /// /// @@ -108,7 +108,7 @@ public IReadOnlyList BuildArguments() } // Git must never block on a pager, must not octal-escape non-ASCII paths, and must not - // emit ANSI colour codes, or the output stops being parseable. + // emit ANSI color codes, or the output stops being parseable. arguments.Add("--no-pager"); arguments.Add("-c"); arguments.Add("core.quotepath=false"); @@ -178,7 +178,7 @@ public virtual async Task> TryExecuteAsync(CancellationToken /// Classifies a failed invocation into an exception type. /// /// - /// Virtual so a derived builder can recognise the failures specific to its own verb — a + /// Virtual so a derived builder can recognize the failures specific to its own verb — a /// rev-parse builder seeing "unknown revision", for instance — and fall back to /// base.CreateException for everything else. /// diff --git a/GitIntegration/Builders/GitCommitBuilder.cs b/GitIntegration/Builders/GitCommitBuilder.cs index ae71669..feb8be7 100644 --- a/GitIntegration/Builders/GitCommitBuilder.cs +++ b/GitIntegration/Builders/GitCommitBuilder.cs @@ -214,7 +214,7 @@ protected override string GetDiagnostic(GitProcessResult result) => : result.StandardError; /// - /// Classifies a failed commit, recognising the one failure that is an ordinary program state. + /// Classifies a failed commit, recognizing the one failure that is an ordinary program state. /// /// /// Overridden because the base class inspects standard error and git reports "nothing to diff --git a/GitIntegration/Builders/GitFetchBuilder.cs b/GitIntegration/Builders/GitFetchBuilder.cs index 1281e2e..a8fff39 100644 --- a/GitIntegration/Builders/GitFetchBuilder.cs +++ b/GitIntegration/Builders/GitFetchBuilder.cs @@ -62,7 +62,7 @@ public interface IGitFetchBuilder : IGitCommandBuilder /// /// Whether, and when, to recurse. /// The same builder, to allow chaining. - /// is not a recognised value. + /// is not a recognized value. public IGitFetchBuilder RecursingSubmodules(GitSubmoduleRecursion recursion); /// Reports git's progress output as it arrives. diff --git a/GitIntegration/Builders/GitInitBuilder.cs b/GitIntegration/Builders/GitInitBuilder.cs index 26cd747..31bb222 100644 --- a/GitIntegration/Builders/GitInitBuilder.cs +++ b/GitIntegration/Builders/GitInitBuilder.cs @@ -92,7 +92,7 @@ protected override void AppendVerbArguments(ICollection arguments) /// contract, and both entry points set it immediately before delegating to the base. /// /// The invocation outcome, which carries nothing this result needs. - /// The initialised repository and whether it was already there. + /// The initialized repository and whether it was already there. protected override GitInitResult ParseResult(GitProcessResult result) { Ensure.NotNull(result); diff --git a/GitIntegration/Builders/GitLogBuilder.cs b/GitIntegration/Builders/GitLogBuilder.cs index 3058988..31d4c4f 100644 --- a/GitIntegration/Builders/GitLogBuilder.cs +++ b/GitIntegration/Builders/GitLogBuilder.cs @@ -197,7 +197,7 @@ protected override void AppendVerbArguments(ICollection arguments) // // git also requires --not to precede every non-option argument, so this cannot instead be // deferred until after the operands: "git log --end-of-options HEAD --not --remotes" dies - // with "fatal: option '--not' must come before non-option arguments". Both behaviours + // with "fatal: option '--not' must come before non-option arguments". Both behaviors // verified against git 2.43. arguments.Add("--not"); arguments.Add("--remotes"); diff --git a/GitIntegration/Builders/GitPullBuilder.cs b/GitIntegration/Builders/GitPullBuilder.cs index 9514cda..77841b4 100644 --- a/GitIntegration/Builders/GitPullBuilder.cs +++ b/GitIntegration/Builders/GitPullBuilder.cs @@ -91,7 +91,7 @@ public interface IGitPullBuilder : IGitCommandBuilder /// its submodules together rather than leaving the submodules stale until something else notices. /// /// Overlaps with UpdateSubmodules() without replacing it: this flag updates submodules - /// that are already registered, while that verb's Initialise() also checks out a submodule + /// that are already registered, while that verb's Initialize() also checks out a submodule /// added upstream since this working copy was cloned. Not to be confused with /// IGitPushBuilder.CheckingSubmodules, which git spells with the same flag name but which /// governs an unrelated question. @@ -99,7 +99,7 @@ public interface IGitPullBuilder : IGitCommandBuilder /// /// Whether, and when, to recurse. /// The same builder, to allow chaining. - /// is not a recognised value. + /// is not a recognized value. public IGitPullBuilder RecursingSubmodules(GitSubmoduleRecursion recursion); /// Reports git's progress output as it arrives. @@ -274,7 +274,7 @@ protected override GitCompleted ParseResult(GitProcessResult result) => new() { Arguments = Ensure.NotNull(result).Arguments }; /// - /// Classifies a failed pull, recognising a conflict as its own outcome. + /// Classifies a failed pull, recognizing a conflict as its own outcome. /// /// /// Overridden because the base class inspects standard error while git announces a conflict on @@ -310,7 +310,7 @@ protected override GitCommandException CreateException(GitProcessResult result) /// case) must not gain a trailing blank line from an empty standard output. /// /// Overriding the base class's seam rather than either entry point is what makes the two report - /// the same text. recognises only a conflict and hands everything + /// the same text. recognizes only a conflict and hands everything /// else to the base implementation, whose message is built from this method — so a non-conflict /// failure explained on standard output now reaches a caller of /// as well as one of diff --git a/GitIntegration/Builders/GitPushBuilder.cs b/GitIntegration/Builders/GitPushBuilder.cs index 3797289..9ab4e85 100644 --- a/GitIntegration/Builders/GitPushBuilder.cs +++ b/GitIntegration/Builders/GitPushBuilder.cs @@ -78,7 +78,7 @@ public interface IGitPushBuilder : IGitCommandBuilder /// submodules' working trees or refs". Here it means something else entirely: a superproject /// commit referencing a submodule commit that no remote has is a commit nobody else can use, and /// this governs whether git checks for, or pushes, those submodule commits to their own remotes - /// first. Putting one word on two unrelated behaviours would be the easiest possible thing for a + /// first. Putting one word on two unrelated behaviors would be the easiest possible thing for a /// caller to get wrong, so the two carry different method names and different enums. /// /// @@ -89,7 +89,7 @@ public interface IGitPushBuilder : IGitCommandBuilder /// /// What to do about the submodules' commits. /// The same builder, to allow chaining. - /// is not a recognised value. + /// is not a recognized value. public IGitPushBuilder CheckingSubmodules(GitSubmodulePushCheck check); /// Reports git's progress output as it arrives. diff --git a/GitIntegration/Builders/GitStatusBuilder.cs b/GitIntegration/Builders/GitStatusBuilder.cs index 8de3b6a..7faae4b 100644 --- a/GitIntegration/Builders/GitStatusBuilder.cs +++ b/GitIntegration/Builders/GitStatusBuilder.cs @@ -23,7 +23,7 @@ public interface IGitStatusBuilder : IGitCommandBuilder /// /// The reporting mode. /// The same builder, to allow chaining. - /// is not a recognised value. + /// is not a recognized value. public IGitStatusBuilder WithUntrackedFiles(GitUntrackedFilesMode mode); /// diff --git a/GitIntegration/Builders/GitSubmoduleUpdateBuilder.cs b/GitIntegration/Builders/GitSubmoduleUpdateBuilder.cs index c08de7e..9a74424 100644 --- a/GitIntegration/Builders/GitSubmoduleUpdateBuilder.cs +++ b/GitIntegration/Builders/GitSubmoduleUpdateBuilder.cs @@ -29,23 +29,23 @@ namespace ktsu.GitIntegration; /// /// /// Overlaps with Pull().RecursingSubmodules(...) without being equivalent to it. The flag -/// updates submodules as part of a pull; this verb also initialises newly added ones -/// through , which the flag does only for submodules already registered. +/// updates submodules as part of a pull; this verb also initializes newly added ones +/// through , which the flag does only for submodules already registered. /// Both are worth having. /// /// public interface IGitSubmoduleUpdateBuilder : IGitCommandBuilder { /// - /// Initialises any submodule that is registered but has never been checked out. + /// Initializes any submodule that is registered but has never been checked out. /// /// /// Emits --init. Without it, a submodule added upstream since this working copy was /// cloned is skipped silently rather than checked out, because update alone only touches - /// submodules that are already initialised. + /// submodules that are already initialized. /// /// The same builder, to allow chaining. - public IGitSubmoduleUpdateBuilder Initialise(); + public IGitSubmoduleUpdateBuilder Initialize(); /// Updates submodules nested inside submodules, to any depth. /// The same builder, to allow chaining. @@ -97,16 +97,16 @@ public interface IGitSubmoduleUpdateBuilder : IGitCommandBuilder internal sealed class GitSubmoduleUpdateBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath) : GitCommandBuilder(runner, repositoryPath), IGitSubmoduleUpdateBuilder { - private bool _initialise; + private bool _initialize; private bool _recursive; private bool _fromRemote; private bool _force; private int? _depth; /// - public IGitSubmoduleUpdateBuilder Initialise() + public IGitSubmoduleUpdateBuilder Initialize() { - _initialise = true; + _initialize = true; return this; } @@ -154,7 +154,7 @@ protected override void AppendVerbArguments(ICollection arguments) arguments.Add("submodule"); arguments.Add("update"); - if (_initialise) + if (_initialize) { arguments.Add("--init"); } diff --git a/GitIntegration/Execution/GitExceptions.cs b/GitIntegration/Execution/GitExceptions.cs index 948d4de..3862591 100644 --- a/GitIntegration/Execution/GitExceptions.cs +++ b/GitIntegration/Execution/GitExceptions.cs @@ -92,7 +92,7 @@ public GitCommandException(string message, int exitCode, IReadOnlyList a /// Git did not complete within and was terminated. /// /// -/// Distinct from , which means the caller cancelled. The +/// Distinct from , which means the caller canceled. The /// distinction matters because a timeout is a candidate for retry while a caller's cancellation is /// not. A credential prompt is not among the causes: every invocation runs with /// GIT_TERMINAL_PROMPT=0, so a remote operation needing credentials it does not have fails diff --git a/GitIntegration/Execution/RunCommandGitProcessRunner.cs b/GitIntegration/Execution/RunCommandGitProcessRunner.cs index 7a66e22..b5abb0d 100644 --- a/GitIntegration/Execution/RunCommandGitProcessRunner.cs +++ b/GitIntegration/Execution/RunCommandGitProcessRunner.cs @@ -20,7 +20,7 @@ public sealed class RunCommandGitProcessRunner(GitOptions options) : IGitProcess { // Snapshotted at construction rather than read per invocation. GitOptions is registered as a // mutable singleton, so any consumer that resolves it and sets a property would otherwise - // change the behaviour of every other consumer mid-flight. The properties cannot simply be + // change the behavior of every other consumer mid-flight. The properties cannot simply be // made init-only: the Action configure delegate receives an already-constructed // instance, and init accessors are settable only during object initialization. private readonly string _executablePath = Ensure.NotNull(options).ExecutablePath; @@ -119,7 +119,7 @@ public async Task RunAsync(GitProcessRequest request, Cancella // RunCommand delivers cancellation two ways at once: a registration that kills the process, and // WaitForExitAsync observing the token. When the kill wins that race the process exits before the // await faults, so ExecuteAsync returns normally carrying a killed process's exit code (-1 on - // Windows) and a cancelled run is indistinguishable from an ordinary git failure. Classify it the + // Windows) and a canceled run is indistinguishable from an ordinary git failure. Classify it the // same way the catch clause does, so both paths reach the caller with identical semantics. // // Gated on a non-zero exit code as well, because the token being signalled does not by diff --git a/GitIntegration/GitHubProvider.cs b/GitIntegration/GitHubProvider.cs index afb59e3..49f1855 100644 --- a/GitIntegration/GitHubProvider.cs +++ b/GitIntegration/GitHubProvider.cs @@ -73,7 +73,7 @@ public sealed class GitHubProvider : GitProvider /// /// GitHub has no single endpoint that both honours and reveals /// the private repositories a credential can see, so the route is chosen from what the owner is. - /// GET /orgs/{org}/repos does both for an organisation. GET /user/repos does both + /// GET /orgs/{org}/repos does both for an organization. GET /user/repos does both /// for the credential's own account, but only for that account — it always describes the token's /// own repositories regardless of which owner was configured, so it is reached only once the /// configured owner has been confirmed to be that account. GET /users/{login}/repos, @@ -120,8 +120,8 @@ public override async Task> GetRepositoriesAsync(Ca /// Authenticated, the owner's type decides the route, and it is read from /// GET /users/{login} rather than inferred from GET /orgs/{login}/repos answering /// 404. The inference is the cheaper probe and the wrong one: a token without - /// read:org, or one not authorised for an organisation that enforces SSO, is answered - /// 404 by that route for an organisation that plainly exists, and the inference would + /// read:org, or one not authorised for an organization that enforces SSO, is answered + /// 404 by that route for an organization that plainly exists, and the inference would /// quietly demote it to the public-only user route — reinstating the exact under-reporting this /// method exists to remove, under a condition nothing would report. GET /users/{login} is /// a public endpoint whose type no credential's scope can change, and its 404 says @@ -152,7 +152,7 @@ private async Task> GetOwnerRepositoriesAsync(GitHubCl User account = await client.User.Get(owner).ConfigureAwait(false); - if (IsOrganisation(account)) + if (IsOrganization(account)) { return await client.Repository.GetAllForOrg(owner).ConfigureAwait(false); } @@ -172,7 +172,7 @@ private async Task> GetOwnerRepositoriesAsync(GitHubCl // configured "Octocat" for the account GitHub reports as "octocat" named the same account. return string.Equals(authenticatedLogin, owner, StringComparison.OrdinalIgnoreCase) // Affiliation rather than the default: GetAllForCurrent() unfiltered also returns - // repositories the account merely collaborates on or reaches through an organisation, + // repositories the account merely collaborates on or reaches through an organization, // which are not Owner's repositories and would report a different owner's work under // this owner's name. Owner is the affiliation that makes this route mean what // GET /users/{login}/repos means, minus the public-only limit. @@ -181,10 +181,10 @@ private async Task> GetOwnerRepositoriesAsync(GitHubCl } /// - /// Reports whether an account GitHub described is an organisation. + /// Reports whether an account GitHub described is an organization. /// /// - /// Asks whether GitHub said "organisation" rather than whether it said "user", so every other + /// Asks whether GitHub said "organization" rather than whether it said "user", so every other /// answer routes to the user branches: is nullable and /// already carries and /// alongside the two this decision is really about. An @@ -193,8 +193,8 @@ private async Task> GetOwnerRepositoriesAsync(GitHubCl /// wrong about coverage — only conservative about it. /// /// The account GET /users/{login} reported. - /// when GitHub called the account an organisation; otherwise, . - private static bool IsOrganisation(User account) => account.Type == AccountType.Organization; + /// when GitHub called the account an organization; otherwise, . + private static bool IsOrganization(User account) => account.Type == AccountType.Organization; /// internal override async Task> GetPullRequestsCoreAsync(GitRepositoryAddress repositoryAddress, CancellationToken cancellationToken) @@ -316,7 +316,7 @@ private static long ToOctokitRepositoryId(GitRepositoryAddress repositoryAddress /// /// runs before either transport is constructed: it /// can throw for a credential subtype this library does - /// not recognise, and running it first means that throw can never leave a constructed + /// not recognize, and running it first means that throw can never leave a constructed /// stranded with nothing left to dispose it — there is nothing to /// strand yet. /// @@ -472,7 +472,7 @@ private static GitPullRequestState ToGitPullRequestState(PullRequest pullRequest // ItemState has exactly these two members, mirroring GitHub's own state field, which is // documented to be only ever "open" or "closed" — this is unreachable in practice, but the // switch must still be exhaustive. - _ => throw new NotSupportedException($"GitHub reported an unrecognised pull request state '{pullRequest.State.StringValue}'."), + _ => throw new NotSupportedException($"GitHub reported an unrecognized pull request state '{pullRequest.State.StringValue}'."), }; } @@ -523,14 +523,14 @@ private GitHostingException Translate(ApiException exception) // GitHostingException itself defaults to, rather than throwing while translating a throw. string responseBody = exception.HttpResponse?.Body as string ?? string.Empty; - // A token that is valid but unauthorised for an organisation's single sign-on arrives as a + // A token that is valid but unauthorised for an organization's single sign-on arrives as a // plain 403, indistinguishable in status and body from a bad credential. The header is the // only thing carrying the URL that resolves it, and that URL is the whole remedy — without // it, the two failures a caller most needs to tell apart read identically. string? singleSignOnUrl = TryGetSingleSignOnUrl(exception); string authenticationMessage = singleSignOnUrl is null ? exception.Message - : $"{exception.Message} This organisation requires single sign-on authorisation for " + + : $"{exception.Message} This organization requires single sign-on authorisation for " + $"this credential. Authorise it at: {singleSignOnUrl}"; return exception switch @@ -577,7 +577,7 @@ private GitHostingException Translate(ApiException exception) /// /// Retry-After may also carry an HTTP date rather than a delay in seconds. GitHub sends /// seconds, and a value that does not parse as seconds yields , so an - /// unrecognised form leaves unset rather than + /// unrecognized form leaves unset rather than /// carrying an invented instant. /// /// The failure Octokit reported. diff --git a/GitIntegration/GitProvider.cs b/GitIntegration/GitProvider.cs index a470fc0..efc29c9 100644 --- a/GitIntegration/GitProvider.cs +++ b/GitIntegration/GitProvider.cs @@ -85,11 +85,11 @@ public abstract class GitProvider : IGitHostingProvider /// Reports whether a request this provider issues would actually carry a credential, which is a /// narrower question than whether the credential cache holds an entry. A resolved /// is an entry that says "proceed unauthenticated", and a - /// subtype does not recognise is one no request can ever carry, - /// so both report here. Both share + /// subtype does not recognize is one no request can ever carry, + /// so both report here. Both share /// with so the two can never drift apart. /// - public bool IsAuthenticated => ResolveRecognisedCredential(out _) is { Kind: not HostingCredentialKind.None }; + public bool IsAuthenticated => ResolveRecognizedCredential(out _) is { Kind: not HostingCredentialKind.None }; /// /// Gets or initializes the transport this provider issues HTTP requests through, or @@ -249,12 +249,12 @@ public bool TryGetCredential(out Credential? credential) /// /// Resolves this provider's credential into the shape a subclass applies to its transport, - /// recognising every subtype this library understands. + /// recognizing every subtype this library understands. /// /// /// No credential at all, or a resolved , both proceed /// — enumerating public repositories without - /// credentials is legitimate, and refusing it would break a real use. An unrecognised + /// credentials is legitimate, and refusing it would break a real use. An unrecognized /// subtype throws instead of doing the same: the caller configured /// something this library does not understand, and proceeding as though nothing were /// configured would hide that rather than surface it. @@ -265,11 +265,11 @@ public bool TryGetCredential(out Credential? credential) /// /// The resolved credential. /// - /// A credential was resolved whose runtime type is not one this method recognises. + /// A credential was resolved whose runtime type is not one this method recognizes. /// internal HostingCredential ResolveCredential() { - HostingCredential? resolved = ResolveRecognisedCredential(out Credential? credential); + HostingCredential? resolved = ResolveRecognizedCredential(out Credential? credential); if (resolved is not null) { return resolved; @@ -280,11 +280,11 @@ internal HostingCredential ResolveCredential() // assigns the out parameter; a non-null one means the cache held a subtype not in the table. throw new InvalidOperationException(credential is null ? $"Provider '{Name}' has a {nameof(CredentialSource)} that returned null. Return {nameof(HostingCredential)}.{nameof(HostingCredential.None)} to proceed unauthenticated." - : $"Provider '{Name}' resolved a credential of type '{credential.GetType()}', which this library does not recognise."); + : $"Provider '{Name}' resolved a credential of type '{credential.GetType()}', which this library does not recognize."); } /// - /// Resolves this provider's credential, reporting an unrecognised + /// Resolves this provider's credential, reporting an unrecognized /// subtype as rather than by throwing. /// /// @@ -298,8 +298,8 @@ internal HostingCredential ResolveCredential() /// When this method returns, contains the raw credential the cache held, which is /// non- whenever this method returns . /// - /// The resolved credential, or for an unrecognised subtype. - private HostingCredential? ResolveRecognisedCredential(out Credential? credential) + /// The resolved credential, or for an unrecognized subtype. + private HostingCredential? ResolveRecognizedCredential(out Credential? credential) { credential = null; @@ -309,7 +309,7 @@ internal HostingCredential ResolveCredential() if (CredentialSource is not null) { // A null return leaves `credential` null, which is what tells ResolveCredential to - // report this as a CredentialSource bug rather than an unrecognised cache subtype. It + // report this as a CredentialSource bug rather than an unrecognized cache subtype. It // reaches IsAuthenticated as "no credential a request could carry", which is a property // getter and so must not throw. return CredentialSource(); diff --git a/GitIntegration/Hosting/AzureDevOpsProvider.cs b/GitIntegration/Hosting/AzureDevOpsProvider.cs index 640109f..0751b6b 100644 --- a/GitIntegration/Hosting/AzureDevOpsProvider.cs +++ b/GitIntegration/Hosting/AzureDevOpsProvider.cs @@ -94,7 +94,7 @@ public sealed class AzureDevOpsProvider : GitProvider /// /// Gets or initializes the Azure DevOps project to scope repository enumeration to, or /// to enumerate every repository in 's - /// organisation. + /// organization. /// /// /// Azure DevOps nests repositories under a project, which GitHub has no equivalent of — see @@ -115,7 +115,7 @@ public sealed class AzureDevOpsProvider : GitProvider /// Calls GET https://dev.azure.com/{organization}/[{project}/]_apis/git/repositories — /// the project path segment appears only when is set. This endpoint /// documents no pagination parameters at all: the response is the complete repository list for - /// the organisation or project on every call, so this method issues exactly one request. That is + /// the organization or project on every call, so this method issues exactly one request. That is /// what makes it differ from , which must page. /// public override async Task> GetRepositoriesAsync(CancellationToken cancellationToken = default) @@ -124,7 +124,7 @@ public override async Task> GetRepositoriesAsync(Ca // Both of these can throw — BuildRepositoriesUri is defensive rather than a real risk, but // ResolveCredential genuinely can (InvalidOperationException for a credential subtype this - // library does not recognise). Both run before CreateHttpClient so a throw here never leaves + // library does not recognize). Both run before CreateHttpClient so a throw here never leaves // a constructed HttpClient stranded with nothing left to dispose it. Uri requestUri = BuildRepositoriesUri(); HostingCredential credential = ResolveCredential(); @@ -233,7 +233,7 @@ internal override async Task> GetPullRequestsCoreA /// Calls POST .../repositories/{repositoryId}/pullrequests. /// and /// are bare branch names — this library's own - /// normalisation, matching what a caller gets back from every read path — so they are qualified + /// normalization, matching what a caller gets back from every read path — so they are qualified /// with refs/heads/ here before being sent, the reverse of the stripping /// does on the way back in. The response is /// the created pull request; Microsoft's own worked example reports 201 despite the @@ -486,7 +486,7 @@ private static AuthenticationHeaderValue BasicAuthenticationHeader(string userna /// and /// arrive fully qualified (e.g. refs/heads/main); strips /// the prefix so a caller never has to branch on host to get a bare branch name — see the spec's - /// "Two normalisations" section. is read from + /// "Two normalizations" section. is read from /// , not : /// 's own remarks record that Azure DevOps's identifier is the /// unique name, usually an email address, the same distinction GitHub's login draws on the other @@ -549,7 +549,7 @@ private static AuthenticationHeaderValue BasicAuthenticationHeader(string userna /// completed → , abandoned → /// . notSet and all are query-side-only /// values a host never reports as a pull request's own status, so they fall through to a - /// along with anything else unrecognised. + /// along with anything else unrecognized. /// /// The status Azure DevOps reported. /// The status code Azure DevOps reported for the response carrying . @@ -561,7 +561,7 @@ private static AuthenticationHeaderValue BasicAuthenticationHeader(string userna "completed" => GitPullRequestState.Merged, "abandoned" => GitPullRequestState.Closed, _ => throw new GitHostingRequestException( - $"Azure DevOps reported an unrecognised pull request status '{status}'.", + $"Azure DevOps reported an unrecognized pull request status '{status}'.", Name, statusCode, responseBody), diff --git a/GitIntegration/Hosting/GitHubDeviceFlow.cs b/GitIntegration/Hosting/GitHubDeviceFlow.cs index 50f4b7f..6f7b657 100644 --- a/GitIntegration/Hosting/GitHubDeviceFlow.cs +++ b/GitIntegration/Hosting/GitHubDeviceFlow.cs @@ -101,7 +101,7 @@ public sealed record GitHubDeviceCode /// GitHub's own published documentation shows the device flow using form-url-encoded requests. /// GitHub's actual service accepts the JSON form in practice, which is what let /// avoid the scope-encoding problem above, but this is -/// undocumented behaviour this library now depends on, and nothing in this library's test suite +/// undocumented behavior this library now depends on, and nothing in this library's test suite /// verifies it against the real service, only against the fake transport. /// /// @@ -166,7 +166,7 @@ public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList /// Defaults to . Internal for the same /// reason as : this exists so a test can replace minutes of real waiting /// with an instantaneous one while still exercising the interval, the widening, and the deadline - /// arithmetic that surround it, not so a caller can tune polling behaviour. + /// arithmetic that surround it, not so a caller can tune polling behavior. /// internal Func Delay { get; init; } = Task.Delay; @@ -247,7 +247,7 @@ public async Task RequestDeviceCodeAsync(CancellationToken can /// /// /// Polls until the user authorises, the code expires, or is - /// cancelled, so this may block for as long as . A + /// canceled, so this may block for as long as . A /// authorization_pending answer waits and tries /// again; a slow_down answer widens that wait by first. /// The deadline is enforced by this flow, not left to GitHub: a server that keeps answering @@ -263,9 +263,9 @@ public async Task RequestDeviceCodeAsync(CancellationToken can /// unsupported_grant_type, and device_flow_disabled are a fact about the caller's own /// configuration and become instead: none of the three can /// be fixed by the user trying again, so reporting them as an authentication failure would loop a - /// person through a sign-in that can never succeed. A code this method does not recognise is + /// person through a sign-in that can never succeed. A code this method does not recognize is /// deliberately treated as a as well, on the same - /// reasoning: an unrecognised code is either a GitHub error this library has not been taught yet or + /// reasoning: an unrecognized code is either a GitHub error this library has not been taught yet or /// something upstream of GitHub answering instead, and in both cases a caller is better served /// being told something is wrong with the request than being invited to retry a sign-in for a /// reason nobody has verified sign-in can fix. @@ -279,7 +279,7 @@ public async Task RequestDeviceCodeAsync(CancellationToken can /// /// GitHub refused the request, could not be reached, reported a configuration fault /// (incorrect_client_credentials, unsupported_grant_type, device_flow_disabled), - /// or reported an error code this method does not recognise. + /// or reported an error code this method does not recognize. /// public async Task WaitForTokenAsync(GitHubDeviceCode code, CancellationToken cancellationToken = default) { @@ -360,8 +360,8 @@ public async Task WaitForTokenAsync(GitHubDeviceCode code, Ca throw new GitHostingRequestException( $"GitHub refused the device flow token request: {parsed.Error}. {parsed.ErrorDescription}".TrimEnd()); - case string unrecognisedError: - // A code this library does not recognise is treated as a request fault rather + case string unrecognizedError: + // A code this library does not recognize is treated as a request fault rather // than an authentication failure, deliberately: the two known authentication // codes above are enumerated explicitly, so anything else reaching here is // either a new GitHub error this library has not been taught yet, or a @@ -369,7 +369,7 @@ public async Task WaitForTokenAsync(GitHubDeviceCode code, Ca // another sign-in attempt is more likely to be wrong than treating it as // something the caller or its configuration needs to look at. throw new GitHostingRequestException( - $"GitHub reported an unrecognised device flow error: {unrecognisedError}. {parsed.ErrorDescription}".TrimEnd()); + $"GitHub reported an unrecognized device flow error: {unrecognizedError}. {parsed.ErrorDescription}".TrimEnd()); default: // Every non-null error is handled above, so the error is null by the time control diff --git a/GitIntegration/Hosting/IGitHostingProvider.cs b/GitIntegration/Hosting/IGitHostingProvider.cs index 077ac29..f872e00 100644 --- a/GitIntegration/Hosting/IGitHostingProvider.cs +++ b/GitIntegration/Hosting/IGitHostingProvider.cs @@ -60,7 +60,7 @@ public interface IGitHostingProvider /// to see, private ones included, and falls back to the owner's public repositories only where /// the host publishes no owner-scoped route that a credential widens. That is the contract /// callers may rely on, and it is stated here rather than left to each provider to describe its - /// own behaviour: a caller who has to read two providers' remarks and work out what they have in + /// own behavior: a caller who has to read two providers' remarks and work out what they have in /// common is a caller who will get it wrong. /// /// @@ -68,7 +68,7 @@ public interface IGitHostingProvider /// never takes it — its enumeration is entitlement-scoped /// throughout. takes it in exactly one case: an owner that is a /// GitHub user other than the credential's own account, for which GitHub publishes no - /// authenticated owner-scoped route at all. An organisation owner, and the credential's own user + /// authenticated owner-scoped route at all. An organization owner, and the credential's own user /// account, both reach the full entitled set. See /// for the routes that produces. /// @@ -89,7 +89,7 @@ public interface IGitHostingProvider /// /// Returns open pull requests only. Both hosts happen to default to open/active when /// unfiltered, but relying on that default would make this method's contract a restatement of - /// two vendors' current behaviour, either of which could change without notice — so the filter + /// two vendors' current behavior, either of which could change without notice — so the filter /// is requested explicitly of each host rather than left implicit. Listing merged or abandoned /// pull requests is a filtering feature that can be added later without breaking this contract. /// diff --git a/GitIntegration/IGitClient.cs b/GitIntegration/IGitClient.cs index b734400..9de6f7f 100644 --- a/GitIntegration/IGitClient.cs +++ b/GitIntegration/IGitClient.cs @@ -63,7 +63,7 @@ public interface IGitClient /// Creates a repository at a path. /// /// - /// Safe to run against a path that already holds a repository: git reinitialises it, and the + /// Safe to run against a path that already holds a repository: git reinitializes it, and the /// result reports so a caller can tell. /// /// Where the repository should be. diff --git a/GitIntegration/Models/GitCommit.cs b/GitIntegration/Models/GitCommit.cs index bac79ba..3056982 100644 --- a/GitIntegration/Models/GitCommit.cs +++ b/GitIntegration/Models/GitCommit.cs @@ -51,7 +51,7 @@ public sealed record GitSignature /// /// /// Parsed from git's strict ISO-8601 output, so the original offset is preserved rather than - /// normalised to UTC — the local time a commit was made in is information a caller may want. + /// normalized to UTC — the local time a commit was made in is information a caller may want. /// public required DateTimeOffset Timestamp { get; init; } } diff --git a/GitIntegration/Models/GitEnums.cs b/GitIntegration/Models/GitEnums.cs index 0514db5..0df9710 100644 --- a/GitIntegration/Models/GitEnums.cs +++ b/GitIntegration/Models/GitEnums.cs @@ -64,7 +64,7 @@ public enum GitChangeKind /// The path has conflicting changes from an unfinished merge. Unmerged, - /// Git reported a status letter this library does not recognise. + /// Git reported a status letter this library does not recognize. Unknown, } @@ -106,7 +106,7 @@ public enum GitUntrackedFilesMode public enum GitSubmoduleState { /// - /// Git reported a marker this library does not recognise. + /// Git reported a marker this library does not recognize. /// /// /// Also the state of a gitlink the superproject records but that submodule status did not @@ -126,10 +126,10 @@ public enum GitSubmoduleState DifferentCommit, /// - /// The submodule is registered but not initialised, so its working directory holds no checkout. + /// The submodule is registered but not initialized, so its working directory holds no checkout. /// /// Git's - marker. - Uninitialised, + Uninitialized, /// The submodule has conflicting changes from an unfinished merge. /// Git's U marker. diff --git a/GitIntegration/Models/GitInitResult.cs b/GitIntegration/Models/GitInitResult.cs index 1e3e3e6..5278eb9 100644 --- a/GitIntegration/Models/GitInitResult.cs +++ b/GitIntegration/Models/GitInitResult.cs @@ -3,7 +3,7 @@ namespace ktsu.GitIntegration; /// -/// The outcome of initialising a repository. +/// The outcome of initializing a repository. /// public sealed record GitInitResult { @@ -14,7 +14,7 @@ public sealed record GitInitResult /// Gets a value indicating whether a repository was already present at the target path. /// /// - /// git init is idempotent: run against an existing repository it reinitialises and exits + /// git init is idempotent: run against an existing repository it reinitializes and exits /// zero, announcing the difference only in prose that this library does not parse. It also /// silently ignores --initial-branch on that path, so a caller that asked for a /// particular initial branch and got here did not get the branch it diff --git a/GitIntegration/Models/GitRefUpdate.cs b/GitIntegration/Models/GitRefUpdate.cs index 09d30a3..fff6e03 100644 --- a/GitIntegration/Models/GitRefUpdate.cs +++ b/GitIntegration/Models/GitRefUpdate.cs @@ -33,7 +33,7 @@ public enum GitRefUpdateKind /// A tag was updated. Git's flag is t, and only fetch emits it. TagUpdate, - /// Git used a flag this library does not recognise. + /// Git used a flag this library does not recognize. Unknown, } diff --git a/GitIntegration/Models/GitSubmodule.cs b/GitIntegration/Models/GitSubmodule.cs index c83ce93..1dadce2 100644 --- a/GitIntegration/Models/GitSubmodule.cs +++ b/GitIntegration/Models/GitSubmodule.cs @@ -41,9 +41,9 @@ public sealed record GitSubmodule /// /// Equal to when is /// , and different when it is - /// . for an uninitialised + /// . for an uninitialized /// submodule, whose working directory holds no checkout to report — git prints the recorded - /// gitlink again in that case, which would otherwise make an uninitialised submodule look + /// gitlink again in that case, which would otherwise make an uninitialized submodule look /// indistinguishable from a synchronised one. /// /// Also for a submodule, where @@ -62,7 +62,7 @@ public sealed record GitSubmodule /// /// /// Git's own git describe-style suffix, purely descriptive. It is absent for an - /// uninitialised submodule, since there is nothing checked out to describe. + /// uninitialized submodule, since there is nothing checked out to describe. /// public string? Describe { get; init; } } diff --git a/GitIntegration/Parsing/GitPushParser.cs b/GitIntegration/Parsing/GitPushParser.cs index 66108c2..86da977 100644 --- a/GitIntegration/Parsing/GitPushParser.cs +++ b/GitIntegration/Parsing/GitPushParser.cs @@ -12,7 +12,7 @@ namespace ktsu.GitIntegration; /// Each record is <flag>TAB<local-ref>:<remote-ref>TAB<summary>. /// Records are surrounded by lines that are not records — a leading To <url>, a /// trailing Done, and a tracking notice when the push set an upstream — so the parser -/// recognises records by shape rather than by position. +/// recognizes records by shape rather than by position. /// internal static class GitPushParser { @@ -140,7 +140,7 @@ private static (GitCommitSha? OldSha, GitCommitSha? NewSha) ReadShaRange(string // Deliberately tolerant, unlike the status parser: git's push flags are not a closed set // this library can rely on never growing, and failing a whole push report over one - // unrecognised character would be worse than naming the reference with an unknown kind. + // unrecognized character would be worse than naming the reference with an unknown kind. _ => GitRefUpdateKind.Unknown, }; } diff --git a/GitIntegration/Parsing/GitStatusParser.cs b/GitIntegration/Parsing/GitStatusParser.cs index 28cc77b..6c931f7 100644 --- a/GitIntegration/Parsing/GitStatusParser.cs +++ b/GitIntegration/Parsing/GitStatusParser.cs @@ -81,7 +81,7 @@ internal static GitStatus Parse(string output) break; default: - throw new GitParseException($"Unrecognised status record: '{record}'."); + throw new GitParseException($"Unrecognized status record: '{record}'."); } } @@ -244,6 +244,6 @@ private static string[] SplitFields(string record, int count) 'C' => GitFileState.Copied, 'T' => GitFileState.TypeChanged, 'U' => GitFileState.Unmerged, - _ => throw new GitParseException($"Unrecognised status code '{code}'."), + _ => throw new GitParseException($"Unrecognized status code '{code}'."), }; } diff --git a/GitIntegration/Parsing/GitSubmoduleParser.cs b/GitIntegration/Parsing/GitSubmoduleParser.cs index cba4dce..99ce8a4 100644 --- a/GitIntegration/Parsing/GitSubmoduleParser.cs +++ b/GitIntegration/Parsing/GitSubmoduleParser.cs @@ -18,7 +18,7 @@ namespace ktsu.GitIntegration; /// The split exists because neither command answers the whole question. ls-files is plumbing: /// it is NUL-terminated, so a path may contain anything at all, and its mode field states outright /// which entries are gitlinks. But it reports only what the superproject records — it -/// cannot say whether a submodule is initialised, or whether a different commit is checked out. +/// cannot say whether a submodule is initialized, or whether a different commit is checked out. /// submodule status answers exactly that, but it is a shell wrapper rather than plumbing, so /// its output carries no stability guarantee of the kind for-each-ref and /// status --porcelain=v2 do, and — worse — it has no -z form at all. @@ -273,7 +273,7 @@ private static GitSubmodule Apply(GitSubmodule submodule, StatusLine status, str { State = status.State, - // git prints the recorded gitlink again for an uninitialised submodule, which would make it + // git prints the recorded gitlink again for an uninitialized submodule, which would make it // indistinguishable from a synchronised one. Nothing is checked out there, so nothing is // reported. // @@ -281,7 +281,7 @@ private static GitSubmodule Apply(GitSubmodule submodule, StatusLine status, str // unmerged submodule, where there is no single checked-out commit to name. It is a well-formed // object id as far as the semantic type is concerned, so nothing downstream would catch it — // it would simply read as a commit that happens to be all zeroes. - CheckedOutSha = status.State == GitSubmoduleState.Uninitialised || IsNullObjectId(status.ObjectId) + CheckedOutSha = status.State == GitSubmoduleState.Uninitialized || IsNullObjectId(status.ObjectId) ? null : GitParseValues.ToSemantic(status.ObjectId, "submodule checked-out object id"), Describe = describe, @@ -325,11 +325,11 @@ private static string ToGitSpelling(RelativeDirectoryPath path) => { ' ' => GitSubmoduleState.InSync, '+' => GitSubmoduleState.DifferentCommit, - '-' => GitSubmoduleState.Uninitialised, + '-' => GitSubmoduleState.Uninitialized, 'U' => GitSubmoduleState.Conflicted, // Not a closed set the way the change-kind letters are: submodule status is a shell wrapper, - // and a marker it gains later should report the submodule with an unrecognised state rather + // and a marker it gains later should report the submodule with an unrecognized state rather // than fail the whole listing. _ => GitSubmoduleState.Unknown, }; diff --git a/GitIntegration/Parsing/GitVersionParser.cs b/GitIntegration/Parsing/GitVersionParser.cs index 9ae1f5e..17f5fbf 100644 --- a/GitIntegration/Parsing/GitVersionParser.cs +++ b/GitIntegration/Parsing/GitVersionParser.cs @@ -26,7 +26,7 @@ internal static GitVersion Parse(string output) if (!trimmed.StartsWith(Prefix, StringComparison.Ordinal)) { - throw new GitParseException($"Unrecognised 'git --version' output: '{trimmed}'."); + throw new GitParseException($"Unrecognized 'git --version' output: '{trimmed}'."); } string raw = trimmed[Prefix.Length..]; @@ -37,7 +37,7 @@ internal static GitVersion Parse(string output) // suffix such as ".windows.1" makes trailing components non-numeric by design. if (!int.TryParse(components[0], NumberStyles.None, CultureInfo.InvariantCulture, out int major)) { - throw new GitParseException($"Unrecognised git version number: '{raw}'."); + throw new GitParseException($"Unrecognized git version number: '{raw}'."); } return new GitVersion diff --git a/GitIntegration/SemanticTypes/GitProviderTypes.cs b/GitIntegration/SemanticTypes/GitProviderTypes.cs index 5d541cf..5fa256b 100644 --- a/GitIntegration/SemanticTypes/GitProviderTypes.cs +++ b/GitIntegration/SemanticTypes/GitProviderTypes.cs @@ -12,7 +12,7 @@ public sealed record GitProviderName : SemanticString { } /// /// A strongly-typed owner of repositories within a hosting provider: a GitHub user or -/// organisation, or an Azure DevOps organisation. +/// organization, or an Azure DevOps organization. /// [HasNonWhitespaceContent] public sealed record GitProviderOwner : SemanticString { } @@ -45,7 +45,7 @@ public sealed record GitPullRequestTitle : SemanticString { /// /// The hosts do not agree on what identifies a user: GitHub supplies a login, Azure DevOps a /// unique name that is usually an email address. This type carries whichever the host gave, -/// unaltered, rather than normalising two different concepts into one that matches neither. +/// unaltered, rather than normalizing two different concepts into one that matches neither. /// [HasNonWhitespaceContent] public sealed record GitPullRequestAuthor : SemanticString { } diff --git a/GitIntegration/SemanticTypes/GitRefTypes.cs b/GitIntegration/SemanticTypes/GitRefTypes.cs index 5d2878f..964a384 100644 --- a/GitIntegration/SemanticTypes/GitRefTypes.cs +++ b/GitIntegration/SemanticTypes/GitRefTypes.cs @@ -42,7 +42,7 @@ public sealed record GitRefName : SemanticString { } /// /// /// Values are canonicalised to lowercase, because git emits lowercase but accepts either case as -/// input, and callers should be able to compare two SHAs for equality without normalising first. +/// input, and callers should be able to compare two SHAs for equality without normalizing first. /// /// /// The upper bound is 64, not 40: a repository created with --object-format=sha256 emits diff --git a/docs/superpowers/plans/2026-08-19-gitintegration-v2-phase1-2-foundation.md b/docs/superpowers/plans/2026-08-19-gitintegration-v2-phase1-2-foundation.md index e80c70b..418f65d 100644 --- a/docs/superpowers/plans/2026-08-19-gitintegration-v2-phase1-2-foundation.md +++ b/docs/superpowers/plans/2026-08-19-gitintegration-v2-phase1-2-foundation.md @@ -426,7 +426,7 @@ public sealed record GitRefName : SemanticString { } /// /// /// Values are canonicalised to lowercase, because git emits lowercase but accepts either case as -/// input, and callers should be able to compare two SHAs for equality without normalising first. +/// input, and callers should be able to compare two SHAs for equality without normalizing first. /// [RegexMatch("^[0-9a-fA-F]{4,40}$")] public sealed record GitCommitSha : SemanticString @@ -492,7 +492,7 @@ public sealed record GitProviderName : SemanticString { } /// /// A strongly-typed owner of repositories within a hosting provider: a GitHub user or -/// organisation, or an Azure DevOps organisation. +/// organization, or an Azure DevOps organization. /// [HasNonWhitespaceContent] public sealed record GitProviderOwner : SemanticString { } @@ -1373,7 +1373,7 @@ using ktsu.Semantics.Strings; [TestClass] public class GitCommandBuilderTests { - /// A minimal concrete builder, exercising only the base class behaviour. + /// A minimal concrete builder, exercising only the base class behavior. private sealed class EchoBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath? repositoryPath) : GitCommandBuilder(runner, repositoryPath) { @@ -1548,7 +1548,7 @@ using System.Threading.Tasks; using ktsu.Semantics.Paths; /// -/// The shared behaviour of every git command builder: global argument injection, execution, and +/// The shared behavior of every git command builder: global argument injection, execution, and /// failure translation. /// /// The parsed result type. @@ -1594,7 +1594,7 @@ public abstract class GitCommandBuilder(IGitProcessRunner runner, Absol } // Git must never block on a pager, must not octal-escape non-ASCII paths, and must not - // emit ANSI colour codes, or the output stops being parseable. + // emit ANSI color codes, or the output stops being parseable. arguments.Add("--no-pager"); arguments.Add("-c"); arguments.Add("core.quotepath=false"); diff --git a/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase3-read-only-verbs.md b/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase3-read-only-verbs.md index de32db1..f2e2f0f 100644 --- a/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase3-read-only-verbs.md +++ b/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase3-read-only-verbs.md @@ -218,7 +218,7 @@ The `result is not null` clause is load-bearing — without it the compiler emit ## File Structure -New folders `Models/` and `Parsing/` join the existing `Builders/`, `Execution/`, and `SemanticTypes/`. Namespace stays `ktsu.GitIntegration` everywhere; the folders are organisational only, matching how `Execution/` and `SemanticTypes/` already work. +New folders `Models/` and `Parsing/` join the existing `Builders/`, `Execution/`, and `SemanticTypes/`. Namespace stays `ktsu.GitIntegration` everywhere; the folders are organizational only, matching how `Execution/` and `SemanticTypes/` already work. **Library — `GitIntegration/`** @@ -275,10 +275,10 @@ Each verb's interface and builder share one file because they change together an | `Builders/GitRemoteListBuilderTests.cs` | argv assertions | | `Builders/GitRevParseBuilderTests.cs` | argv assertions | | `Builders/GitVersionBuilderTests.cs` | argv assertions | -| `GitClientTests.cs` | client behaviour over the scripted runner | +| `GitClientTests.cs` | client behavior over the scripted runner | | `GitRepositoryVerbTests.cs` | verb factories, `ProcessRunner` guard, `IsClonedAsync` | -**Fixtures are inline `const string` fields, not files on disk.** Git's machine formats embed NUL and `0x1F`, which make a fixture file binary: the repo's `* text=auto eol=lf` would skip it for EOL normalisation, `git diff` would render it unreadable, and a reviewer could not see what changed. Inline literals using `\u0000` and `\u001f` are reviewable in a diff and impossible to mis-encode. +**Fixtures are inline `const string` fields, not files on disk.** Git's machine formats embed NUL and `0x1F`, which make a fixture file binary: the repo's `* text=auto eol=lf` would skip it for EOL normalization, `git diff` would render it unreadable, and a reviewer could not see what changed. Inline literals using `\u0000` and `\u001f` are reviewable in a diff and impossible to mis-encode. **Write NUL as `\u0000`, never `\0`.** In a fixture like `"…renamed.txt\0a.txt"` the escape is unambiguous, but `"\0" + "1 A. N…"` written as `"\01 A. N…"` reads as an octal escape to anyone skimming it. `\u0000` never does. @@ -547,7 +547,7 @@ public enum GitChangeKind /// The path has conflicting changes from an unfinished merge. Unmerged, - /// Git reported a status letter this library does not recognise. + /// Git reported a status letter this library does not recognize. Unknown, } @@ -696,7 +696,7 @@ public sealed record GitSignature /// /// /// Parsed from git's strict ISO-8601 output, so the original offset is preserved rather than - /// normalised to UTC — the local time a commit was made in is information a caller may want. + /// normalized to UTC — the local time a commit was made in is information a caller may want. /// public required DateTimeOffset Timestamp { get; init; } } @@ -1107,7 +1107,7 @@ internal static class GitVersionParser if (!trimmed.StartsWith(Prefix, StringComparison.Ordinal)) { - throw new GitParseException($"Unrecognised 'git --version' output: '{trimmed}'."); + throw new GitParseException($"Unrecognized 'git --version' output: '{trimmed}'."); } string raw = trimmed[Prefix.Length..]; @@ -1119,7 +1119,7 @@ internal static class GitVersionParser if (components.Length == 0 || !int.TryParse(components[0], NumberStyles.None, CultureInfo.InvariantCulture, out int major)) { - throw new GitParseException($"Unrecognised git version number: '{raw}'."); + throw new GitParseException($"Unrecognized git version number: '{raw}'."); } return new GitVersion @@ -1287,7 +1287,7 @@ public class GitStatusParserTests { // Fixtures are inline rather than files on disk: git's porcelain v2 format embeds NUL, which // makes a fixture file binary, unreviewable in a diff, and exempt from this repo's EOL - // normalisation. NUL is written as the six-character escape u0000 (backslash-u-0-0-0-0) rather + // normalization. NUL is written as the six-character escape u0000 (backslash-u-0-0-0-0) rather // than backslash-zero, so it can never read as an octal escape when followed by a digit. private const string Nul = "\u0000"; @@ -1468,7 +1468,7 @@ public class GitStatusParserTests } [TestMethod] - public void RejectsAnUnrecognisedRecordPrefix() + public void RejectsAnUnrecognizedRecordPrefix() { Assert.ThrowsExactly( () => GitStatusParser.Parse("x something" + Nul)); @@ -1492,7 +1492,7 @@ public class GitStatusParserTests } [TestMethod] - public void IgnoresAHeaderItDoesNotRecognise() + public void IgnoresAHeaderItDoesNotRecognize() { // Forward compatibility: a future git adding a header must not break every caller. GitStatus status = GitStatusParser.Parse( @@ -1597,7 +1597,7 @@ internal static class GitStatusParser break; default: - throw new GitParseException($"Unrecognised status record: '{record}'."); + throw new GitParseException($"Unrecognized status record: '{record}'."); } } @@ -1760,7 +1760,7 @@ internal static class GitStatusParser 'C' => GitFileState.Copied, 'T' => GitFileState.TypeChanged, 'U' => GitFileState.Unmerged, - _ => throw new GitParseException($"Unrecognised status code '{code}'."), + _ => throw new GitParseException($"Unrecognized status code '{code}'."), }; } ``` @@ -1880,7 +1880,7 @@ public class GitStatusBuilderTests } [TestMethod] - public void RejectsAnUnrecognisedUntrackedFilesMode() + public void RejectsAnUnrecognizedUntrackedFilesMode() { RecordingGitProcessRunner runner = new(); GitStatusBuilder builder = new(runner, TestPaths.Root); @@ -2144,7 +2144,7 @@ public class GitLogParserTests [TestMethod] public void PreservesTheCommittedTimeZoneOffset() { - // %aI is strict ISO-8601 with the offset the commit was made in. Normalising to UTC would + // %aI is strict ISO-8601 with the offset the commit was made in. Normalizing to UTC would // throw away the local time, which is information a caller may want. IReadOnlyList commits = GitLogParser.Parse(MergeCommit); @@ -2775,7 +2775,7 @@ public class GitDiffParserTests } [TestMethod] - public void ReportsAnUnrecognisedStatusLetterAsUnknownRatherThanThrowing() + public void ReportsAnUnrecognizedStatusLetterAsUnknownRatherThanThrowing() { // git emits 'B' for a broken pairing and 'X' for a state it calls a bug. Neither is worth // failing an entire diff over, and unlike the status format the set is not closed, so an @@ -3543,7 +3543,7 @@ public class GitBranchListBuilderTests { // Asserted literally rather than through GitOutputFormats. The leading %(refname) is what // lets the parser tell a local branch from a remote-tracking one and drop the remote HEAD - // symbolic reference, so silently losing it would break both behaviours at once. + // symbolic reference, so silently losing it would break both behaviors at once. RecordingGitProcessRunner runner = new(); GitBranchListBuilder builder = new(runner, TestPaths.Root); @@ -5198,7 +5198,7 @@ The spec lists five. Two overlap this phase and are addressed; three do not. git add GitIntegration/Execution/GitProcessRequest.cs git commit -m "[patch] Document that the progress sink must be thread-safe" ``` -- [ ] **`RunCommandGitProcessRunner`'s cancellation doc.** `IGitCommandBuilder` and the runner still promise `OperationCanceledException` whenever the caller's token is signalled, but the implementation deliberately returns the result when git exited 0 first — a zero exit code proves git finished on its own, and discarding a valid result would be wrong. The behaviour is right and the promise is stale. Correct the `` documentation to say that cancellation is reported only when git did not complete. Documentation only; commit on its own before Task 1: +- [ ] **`RunCommandGitProcessRunner`'s cancellation doc.** `IGitCommandBuilder` and the runner still promise `OperationCanceledException` whenever the caller's token is signalled, but the implementation deliberately returns the result when git exited 0 first — a zero exit code proves git finished on its own, and discarding a valid result would be wrong. The behavior is right and the promise is stale. Correct the `` documentation to say that cancellation is reported only when git did not complete. Documentation only; commit on its own before Task 1: ```bash git add GitIntegration/Execution/RunCommandGitProcessRunner.cs GitIntegration/Builders/IGitCommandBuilder.cs diff --git a/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase4-mutating-verbs.md b/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase4-mutating-verbs.md index 038ce3c..1463c46 100644 --- a/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase4-mutating-verbs.md +++ b/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase4-mutating-verbs.md @@ -31,7 +31,7 @@ Every task's requirements implicitly include this section. - **Every `await` in the library gets `.ConfigureAwait(false)`** (CA2007). Test code too. - **Tests use MSTest** with semantic assertions. Async test methods end in `Async` (MSTEST0032/0065). Classes needing a token declare `public TestContext TestContext { get; set; } = null!;`. - **When asserting that a value-returning call throws, discard the result** so the lambda binds to `Action`: `Assert.ThrowsExactly(() => _ = thing.Method(null!));` -- **Commit message tags:** `[minor]` on feature commits. Recognised tags are `[major]`, `[minor]`, `[patch]`, `[pre]` — **`[fix]` is not one of them** and silently fails to signal a version bump. Never add `Co-Authored-By` lines. +- **Commit message tags:** `[minor]` on feature commits. Recognized tags are `[major]`, `[minor]`, `[patch]`, `[pre]` — **`[fix]` is not one of them** and silently fails to signal a version bump. Never add `Co-Authored-By` lines. - **Do not edit** `VERSION.md`, `CHANGELOG.md`, `LATEST_CHANGELOG.md`, `LICENSE.md` — generated. - **`ktsu.Sdk` regenerates `.gitignore` on every `dotnet build`.** Do not hand-edit or commit it. - **Build:** `dotnet build`. **Test:** `dotnet test` — **never `dotnet test --nologo`**, which silently runs zero tests under Microsoft.Testing.Platform and exits 5. @@ -51,7 +51,7 @@ Every task's requirements implicitly include this section. ## Findings from probing the installed git -Captured from `git version 2.50.1.windows.1` with `LC_ALL=C`. **These are the behaviours the tasks are built on — do not re-derive them.** +Captured from `git version 2.50.1.windows.1` with `LC_ALL=C`. **These are the behaviors the tasks are built on — do not re-derive them.** ### Exit codes are not uniform, and that matters @@ -190,7 +190,7 @@ The shipped package's dependency graph is unchanged, and the full suite passed w |---|---| | `Fakes/FakeFileSystemProvider.cs` | Wraps `MockFileSystem` as an `IFileSystemProvider` | | `Builders/GitAddBuilderTests.cs` … one per verb | argv assertions and result handling | -| `GitClientMutatingTests.cs` | `Init` / `Clone` behaviour over the scripted runner and the fake filesystem | +| `GitClientMutatingTests.cs` | `Init` / `Clone` behavior over the scripted runner and the fake filesystem | | `Integration/TemporaryRepository.cs` | Per-test temp repo helper with a Windows-safe recursive delete | | `Integration/GitRoundTripTests.cs` | Tier-3 tests against a real git binary, self-skipping when absent | @@ -405,7 +405,7 @@ public sealed record GitCompleted namespace ktsu.GitIntegration; /// -/// The outcome of initialising a repository. +/// The outcome of initializing a repository. /// public sealed record GitInitResult { @@ -416,7 +416,7 @@ public sealed record GitInitResult /// Gets a value indicating whether a repository was already present at the target path. /// /// - /// git init is idempotent: run against an existing repository it reinitialises and exits + /// git init is idempotent: run against an existing repository it reinitializes and exits /// zero, announcing the difference only in prose that this library does not parse. It also /// silently ignores --initial-branch on that path, so a caller that asked for a /// particular initial branch and got here did not get the branch it @@ -2417,7 +2417,7 @@ internal sealed class GitCommitBuilder( } /// - /// Classifies a failed commit, recognising the one failure that is an ordinary program state. + /// Classifies a failed commit, recognizing the one failure that is an ordinary program state. /// /// /// Overridden because the base class inspects standard error and git reports "nothing to @@ -2507,7 +2507,7 @@ The second two-invocation verb, and the first consumer of `IFileSystemProvider`. ### Why the probe comes first -`git init` against an existing repository reinitialises it, exits 0, and says so only in prose — and **silently ignores `--initial-branch` on that path**. A caller who asked for `main` and got an existing repository on `master` has no way to tell from the exit code. So the builder runs `rev-parse --is-inside-work-tree` against the target first and reports the answer as `GitInitResult.AlreadyExisted`. +`git init` against an existing repository reinitializes it, exits 0, and says so only in prose — and **silently ignores `--initial-branch` on that path**. A caller who asked for `main` and got an existing repository on `master` has no way to tell from the exit code. So the builder runs `rev-parse --is-inside-work-tree` against the target first and reports the answer as `GitInitResult.AlreadyExisted`. The probe is expected to fail when the directory is not a repository — or does not exist at all — so it uses `TryExecuteAsync` and treats any non-zero exit as "no repository here". @@ -2590,7 +2590,7 @@ public class GitInitBuilderTests } [TestMethod] - public async Task ProbesBeforeInitialisingAndReportsAFreshRepositoryAsync() + public async Task ProbesBeforeInitializingAndReportsAFreshRepositoryAsync() { ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() .Then(standardError: "fatal: not a git repository (or any of the parent directories): .git\n", exitCode: 128) @@ -2775,7 +2775,7 @@ internal sealed class GitInitBuilder(IGitProcessRunner runner, AbsoluteDirectory /// contract, and both entry points set it immediately before delegating to the base. /// /// The invocation outcome, which carries nothing this result needs. - /// The initialised repository and whether it was already there. + /// The initialized repository and whether it was already there. protected override GitInitResult ParseResult(GitProcessResult result) { Ensure.NotNull(result); @@ -3678,7 +3678,7 @@ Append these members to the `IGitClient` interface in `GitIntegration/IGitClient /// Creates a repository at a path. /// /// - /// Safe to run against a path that already holds a repository: git reinitialises it, and the + /// Safe to run against a path that already holds a repository: git reinitializes it, and the /// result reports so a caller can tell. /// /// Where the repository should be. @@ -3963,14 +3963,14 @@ public class GitRoundTripTests } /// - /// Initialises a repository with a deterministic identity and initial branch. + /// Initializes a repository with a deterministic identity and initial branch. /// /// /// The identity is written into the repository's own config rather than taken from the host, /// so the tests neither depend on a configured user nor disturb one. The initial branch is /// named explicitly for the same reason: init.defaultBranch varies by machine. /// - private static async Task InitialiseAsync( + private static async Task InitializeAsync( TemporaryRepository temporary, CancellationToken cancellationToken) { @@ -4003,7 +4003,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); Assert.IsTrue(await repository.IsClonedAsync(cancellationToken).ConfigureAwait(false)); } @@ -4015,7 +4015,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - _ = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + _ = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); GitInitResult second = await CreateClient() .Init(temporary.Root) @@ -4031,7 +4031,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -4058,7 +4058,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -4078,7 +4078,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -4100,7 +4100,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("a.txt", "one\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -4133,7 +4133,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); GitRemoteName origin = "origin".As(); GitRepositoryRemotePath first = "https://example.com/one.git".As(); @@ -4166,7 +4166,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository source = new(); - GitRepository origin = await InitialiseAsync(source, cancellationToken).ConfigureAwait(false); + GitRepository origin = await InitializeAsync(source, cancellationToken).ConfigureAwait(false); source.WriteFile("a.txt", "one\n"); _ = await origin.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -4196,7 +4196,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository source = new(); - _ = await InitialiseAsync(source, cancellationToken).ConfigureAwait(false); + _ = await InitializeAsync(source, cancellationToken).ConfigureAwait(false); using TemporaryRepository occupied = new(); occupied.WriteFile("in-the-way.txt", "x"); @@ -4215,7 +4215,7 @@ public class GitRoundTripTests await RequireGitAsync(cancellationToken).ConfigureAwait(false); using TemporaryRepository temporary = new(); - GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + GitRepository repository = await InitializeAsync(temporary, cancellationToken).ConfigureAwait(false); temporary.WriteFile("before.txt", "line1\nline2\nline3\n"); _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); @@ -4248,7 +4248,7 @@ public class GitRoundTripTests Run: `dotnet test --filter "TestCategory=Integration"` -Expected: PASS, 10 tests. If git is not on PATH they report Inconclusive rather than failing — that is the intended behaviour, not a problem to fix. +Expected: PASS, 10 tests. If git is not on PATH they report Inconclusive rather than failing — that is the intended behavior, not a problem to fix. - [ ] **Step 4: Confirm the whole suite is still green** diff --git a/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase5a-remote-sync.md b/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase5a-remote-sync.md index f812ae9..241a429 100644 --- a/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase5a-remote-sync.md +++ b/docs/superpowers/plans/2026-08-20-gitintegration-v2-phase5a-remote-sync.md @@ -44,7 +44,7 @@ Every task's requirements implicitly include this section. - **`.ConfigureAwait(false)` on every await**, library and tests. - **Validate arguments before state.** A method that null-checks an argument and also requires object state checks the argument first. - **MSTest**, semantic assertions, async test methods end in `Async`, `TestContext.CancellationTokenSource.Token` for cancellation. Discard the result when asserting a value-returning call throws: `Assert.ThrowsExactly(() => _ = x.M(null!))`. -- **Commit tags:** `[minor]` for features. Recognised: `[major]`, `[minor]`, `[patch]`, `[pre]`. **`[fix]` is not recognised.** No `Co-Authored-By` lines. +- **Commit tags:** `[minor]` for features. Recognized: `[major]`, `[minor]`, `[patch]`, `[pre]`. **`[fix]` is not recognized.** No `Co-Authored-By` lines. - **Do not edit** `VERSION.md`, `CHANGELOG.md`, `LATEST_CHANGELOG.md`, `LICENSE.md`. - **Build:** `dotnet build`. **Test:** `dotnet test`. **Never `dotnet test --nologo`** — it runs zero tests and exits 5. - **A library `PackageReference` added for analyzer KTSU0006 needs both `PrivateAssets="all"` and a `VersionOverride`** pinned to the lowest version any consumer could resolve. This phase should need no new package at all; if you think you do, stop and report it as a blocker. @@ -168,7 +168,7 @@ Both are documented on the interface. This is deliberate and is the one place in | File | Responsibility | |---|---| | `Parsing/GitPushParserTests.cs`, `Parsing/GitFetchParserTests.cs` | fixtures captured above | -| `Builders/GitPushBuilderTests.cs`, `Builders/GitFetchBuilderTests.cs`, `Builders/GitPullBuilderTests.cs` | argv and behaviour | +| `Builders/GitPushBuilderTests.cs`, `Builders/GitFetchBuilderTests.cs`, `Builders/GitPullBuilderTests.cs` | argv and behavior | | `GitRepositoryRemoteVerbTests.cs` | the three factories | | `Integration/GitRemoteSyncTests.cs` | round trip against a local bare remote | @@ -346,7 +346,7 @@ public enum GitRefUpdateKind /// A tag was updated. Git's flag is t, and only fetch emits it. TagUpdate, - /// Git used a flag this library does not recognise. + /// Git used a flag this library does not recognize. Unknown, } @@ -774,7 +774,7 @@ using System.Collections.Generic; /// Each record is <flag>TAB<local-ref>:<remote-ref>TAB<summary>. /// Records are surrounded by lines that are not records — a leading To <url>, a /// trailing Done, and a tracking notice when the push set an upstream — so the parser -/// recognises records by shape rather than by position. +/// recognizes records by shape rather than by position. /// internal static class GitPushParser { @@ -884,7 +884,7 @@ internal static class GitPushParser // Deliberately tolerant, unlike the status parser: git's push flags are not a closed set // this library can rely on never growing, and failing a whole push report over one - // unrecognised character would be worse than naming the reference with an unknown kind. + // unrecognized character would be worse than naming the reference with an unknown kind. _ => GitRefUpdateKind.Unknown, }; } @@ -2324,7 +2324,7 @@ public class GitPullBuilderTests } [TestMethod] - public async Task RecognisesARebaseConflictTooAsync() + public async Task RecognizesARebaseConflictTooAsync() { // A rebase reports its conflicts with different prose but the same "CONFLICT" marker, and // leaves the repository mid-rebase rather than mid-merge. Both are conflicts to a caller. @@ -2571,7 +2571,7 @@ internal sealed class GitPullBuilder(IGitProcessRunner runner, AbsoluteDirectory new() { Arguments = Ensure.NotNull(result).Arguments }; /// - /// Classifies a failed pull, recognising a conflict as its own outcome. + /// Classifies a failed pull, recognizing a conflict as its own outcome. /// /// /// Overridden because the base class inspects standard error while git announces a conflict on @@ -2821,7 +2821,7 @@ using ktsu.Semantics.Strings; /// /// /// The remote is a bare repository on the local filesystem, which git treats exactly like any other -/// remote. That gives real push negotiation and real rejection behaviour with no network and no +/// remote. That gives real push negotiation and real rejection behavior with no network and no /// credentials — the two things that would make these tests flaky or unrunnable in CI. /// [TestClass] @@ -2972,7 +2972,7 @@ public class GitRemoteSyncTests [TestMethod] public async Task ARejectedPushThrowsAndCarriesTheDetailAsync() { - // The behaviour the whole push design exists for: git exits non-zero and still reports + // The behavior the whole push design exists for: git exits non-zero and still reports // exactly which reference it refused and why. CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; await RequireGitAsync(cancellationToken).ConfigureAwait(false); @@ -3185,7 +3185,7 @@ The file needs `using System.Linq;` for `Any`. Run: `dotnet test --filter "TestCategory=Integration"` -Expected: PASS, 18 tests — Phase 4's 10 plus these 8. If they report Inconclusive, git is not on PATH; that is the designed behaviour, not something to fix. +Expected: PASS, 18 tests — Phase 4's 10 plus these 8. If they report Inconclusive, git is not on PATH; that is the designed behavior, not something to fix. - [ ] **Step 3: Run them with git treated as required** diff --git a/docs/superpowers/plans/2026-08-21-gitintegration-phase5b-hosting-layer.md b/docs/superpowers/plans/2026-08-21-gitintegration-phase5b-hosting-layer.md index 6b45d2e..4af37a5 100644 --- a/docs/superpowers/plans/2026-08-21-gitintegration-phase5b-hosting-layer.md +++ b/docs/superpowers/plans/2026-08-21-gitintegration-phase5b-hosting-layer.md @@ -29,7 +29,7 @@ - **Comments explain *why*, not what.** Never mention tasks, phases, or plans in shipped source. - **Build:** `dotnet build`. **Test:** plain `dotnet test`. **NEVER `dotnet test --nologo`** — under Microsoft Testing Platform it runs zero tests and exits 5, which looks like success. - **No new `PackageReference` is needed for this phase.** Octokit and `ktsu.CredentialCache` are already referenced; `System.Text.Json` is in-box on both target frameworks. If you believe a package is required, stop and report a blocker — a library `PackageReference` added to satisfy analyzer KTSU0006 needs **both** `PrivateAssets="all"` **and** a `VersionOverride` pinned to the lowest version a consumer could resolve, and getting that wrong shipped a `FileNotFoundException` to every consumer in Phase 4. -- **Version tag:** the branch ships as `[minor]` → 2.4.0. Removing `RefreshRemoteRepositories()` and `Repositories` is a deliberate exception to semver, decided because neither ever worked. Recognised tags are `[major]`, `[minor]`, `[patch]`, `[pre]` — `[fix]` is not one and silently fails to signal a bump. Never add `Co-Authored-By` lines. Never edit `VERSION.md`, `CHANGELOG.md`, `LATEST_CHANGELOG.md`, `LICENSE.md`. +- **Version tag:** the branch ships as `[minor]` → 2.4.0. Removing `RefreshRemoteRepositories()` and `Repositories` is a deliberate exception to semver, decided because neither ever worked. Recognized tags are `[major]`, `[minor]`, `[patch]`, `[pre]` — `[fix]` is not one and silently fails to signal a bump. Never add `Co-Authored-By` lines. Never edit `VERSION.md`, `CHANGELOG.md`, `LATEST_CHANGELOG.md`, `LICENSE.md`. - **The test project has `InternalsVisibleTo("ktsu.GitIntegration.Test")`**, so tests reach `internal` types directly. ## Known hazards, learned the expensive way @@ -47,9 +47,9 @@ Only newline (0x0A) and tab (0x09) may appear. -- **Three tests across Phase 5a passed for the wrong reason** — twice a hand-built fixture made a parser read one character off while the test asserted only an unaffected field, and once a test asserted an invocation count for a behaviour the fake could not observe. Two rules follow, and they are binding: +- **Three tests across Phase 5a passed for the wrong reason** — twice a hand-built fixture made a parser read one character off while the test asserted only an unaffected field, and once a test asserted an invocation count for a behavior the fake could not observe. Two rules follow, and they are binding: 1. **Never hand-author an API response fixture.** Every JSON fixture comes from Task 1's captured files. - 2. **A test whose name claims a forwarding or wiring behaviour must assert on the specific property it reads.** A count, or "it did not throw", is not an assertion about wiring. + 2. **A test whose name claims a forwarding or wiring behavior must assert on the specific property it reads.** A count, or "it did not throw", is not an assertion about wiring. ## File Structure @@ -274,7 +274,7 @@ public sealed record GitPullRequestTitle : SemanticString { /// /// The hosts do not agree on what identifies a user: GitHub supplies a login, Azure DevOps a /// unique name that is usually an email address. This type carries whichever the host gave, -/// unaltered, rather than normalising two different concepts into one that matches neither. +/// unaltered, rather than normalizing two different concepts into one that matches neither. /// public sealed record GitPullRequestAuthor : SemanticString { } @@ -521,7 +521,7 @@ This is the task the two provider pairs build on. It removes the two dead member - `public abstract class GitProvider : IGitHostingProvider` keeping `Name`, `Owner`, `PersonaGUID`, `IsAuthenticated`, `TryGetCredential`. - `internal HttpMessageHandler? Handler { get; init; }` — the transport seam. Null means "construct a real one". - `protected HttpClient CreateHttpClient()` — returns a client over `Handler` when set, a real one otherwise. - - `protected HostingCredential ResolveCredential()` — returns a discriminated result the providers apply; throws for an unrecognised credential subtype. + - `protected HostingCredential ResolveCredential()` — returns a discriminated result the providers apply; throws for an unrecognized credential subtype. - `public abstract Task> GetRepositoriesAsync(CancellationToken ct = default);` - `public abstract Task> GetPullRequestsAsync(GitRepositoryName repo, CancellationToken ct = default);` - `public interface IGitPullRequestCreateBuilder` — **declared here, not in Task 6**, exactly as the spec's **The create builder** section defines it. `IGitHostingProvider.CreatePullRequest` returns this type, so it must exist by the end of this task or the interface will not compile: a `` or return type naming something a later task creates is CS1574, an error here. Task 6 supplies the implementation. @@ -547,7 +547,7 @@ public void ProceedsUnauthenticatedWhenNoCredentialIsResolved() { /* assert the public void ProceedsUnauthenticatedForCredentialWithNothing() { /* same */ } [TestMethod] -public void ThrowsForAnUnrecognisedCredentialSubtype() { /* assert InvalidOperationException naming the type */ } +public void ThrowsForAnUnrecognizedCredentialSubtype() { /* assert InvalidOperationException naming the type */ } ``` Fill in each body against the real `ktsu.CredentialCache` API — read it first; do not assume the seeding method's name. If the cache cannot be seeded in-process, introduce a `protected virtual` credential-resolution hook on `GitProvider` that the test double overrides, and say so in your report; do **not** skip these tests. @@ -951,7 +951,7 @@ public async Task ThrowsWhenAPullRequestIsCreatedWithoutAProjectAsync() { /* the public async Task StripsRefsHeadsFromTheBranchNamesAsync() { // The fixture carries refs/heads/... — assert SourceBranch and TargetBranch are bare. - // This is the normalisation that stops callers branching on host. + // This is the normalization that stops callers branching on host. } [TestMethod] @@ -1078,7 +1078,7 @@ git commit -m "[minor] Register and document the hosting layer" ## Self-review notes -Spec coverage was checked section by section. Every section maps to a task: the abstraction and removals to Task 5, credentials to Task 5, models and semantic types to Task 2, state mapping and both normalisations to Tasks 7 and 9, errors to Tasks 3 and 9, transport and the fake to Tasks 4-5, the research requirement to Task 1, the project-required decision to Task 9, and the documentation amendment to Task 10. +Spec coverage was checked section by section. Every section maps to a task: the abstraction and removals to Task 5, credentials to Task 5, models and semantic types to Task 2, state mapping and both normalizations to Tasks 7 and 9, errors to Tasks 3 and 9, transport and the fake to Tasks 4-5, the research requirement to Task 1, the project-required decision to Task 9, and the documentation amendment to Task 10. Two spec statements are deliberately **not** tasks: that this phase adds no integration tier (an absence, recorded in Task 10's documentation step) and the versioning decision (recorded in Global Constraints). diff --git a/docs/superpowers/plans/2026-09-21-worktrees-and-github-auth.md b/docs/superpowers/plans/2026-09-21-worktrees-and-github-auth.md index e449724..ced62e8 100644 --- a/docs/superpowers/plans/2026-09-21-worktrees-and-github-auth.md +++ b/docs/superpowers/plans/2026-09-21-worktrees-and-github-auth.md @@ -2,7 +2,7 @@ > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. -**Goal:** Add `git worktree` verbs, GitHub repository enumeration that can see an organisation's private repositories, and a GitHub OAuth device flow, so a desktop launcher can build a tree of every repository its user can reach and manage branches as one worktree each. +**Goal:** Add `git worktree` verbs, GitHub repository enumeration that can see an organization's private repositories, and a GitHub OAuth device flow, so a desktop launcher can build a tree of every repository its user can reach and manage branches as one worktree each. **Architecture:** Three independent additions. The worktree verbs follow the library's existing builder-plus-parser pattern exactly: a `GitWorktree` record, a `GitWorktreeParser` over `git worktree list --porcelain`, four builders, and four factory methods on `GitRepository`. The GitHub change is an owner-kind property that routes `GetRepositoriesAsync` to whichever endpoint can answer for that kind of owner, defaulting to today's route. The device flow is a standalone type in `Hosting/` that obtains a `HostingCredential` and stores nothing. @@ -12,12 +12,12 @@ ## Global Constraints -- **Version: `[minor]` — 3.2.0.** Additive only. No existing member changes shape or behaviour. +- **Version: `[minor]` — 3.2.0.** Additive only. No existing member changes shape or behavior. - **Never run `dotnet test --nologo`.** Under Microsoft Testing Platform it silently runs zero tests and exits 5. Always plain `dotnet test`. - **Copyright header on every new file:** `// Copyright (c) 2023-2026 ktsu-dev contributors` - **Namespace:** `ktsu.GitIntegration` for library files, `ktsu.GitIntegration.Test` for test files. File-scoped, `using` directives inside the namespace, matching every existing file. - **Tabs, not spaces.** The repository indents with tabs. -- **British spelling in prose and documentation comments** (`behaviour`, `organisation`, `normalise`), matching the existing codebase. +- **British spelling in prose and documentation comments** (`behavior`, `organization`, `normalize`), matching the existing codebase. - **Every public member needs an XML doc comment.** The build treats missing documentation as an error. - **`Ensure.NotNull` in the library, `ArgumentNullException.ThrowIfNull` in tests.** The library takes Polyfill with `PrivateAssets="all"`, so `Ensure` is not visible to the test project. - **Caller-supplied operands go after `--end-of-options`,** via `GitCommandBuilder.AppendOperands`. @@ -272,7 +272,7 @@ public sealed class GitWorktreeParserTests } [TestMethod] - public void IgnoresAnAttributeItDoesNotRecognise() + public void IgnoresAnAttributeItDoesNotRecognize() { // Git may add attributes. An unknown one must not fail a listing that is otherwise readable. string output = @@ -1440,7 +1440,7 @@ git commit -m "feat: add the worktree removal and prune verbs" **Important:** an existing test, `GitHubProviderTests`, asserts `"/users/contoso/repos"` as the route. The default must keep that test passing untouched. If it fails, the default is wrong, not the test. -- [ ] **Step 1: Create the organisation fixture** +- [ ] **Step 1: Create the organization fixture** Copy `GitIntegration.Test/Fixtures/github-repositories.json` to `github-org-repositories.json`, then edit the copy so that it contains exactly two entries: the first with `"name": "org-public-repo"`, `"private": false`, and an owner block whose `"login"` is `"contoso"`; the second with `"name": "org-private-repo"`, `"private": true`, and the same owner login. Update each entry's `full_name`, `html_url`, and `clone_url` to match its name under `contoso`. Leave every other key exactly as captured. @@ -1452,10 +1452,10 @@ Append to `GitIntegration.Test/Hosting/GitHubProviderTests.cs`, inside the class ```csharp [TestMethod] - public async Task EnumeratesAnOrganisationThroughTheOrgsRoute() + public async Task EnumeratesAnOrganizationThroughTheOrgsRoute() { // The route is a documented part of this provider's contract, not an Octokit detail: - // GET /orgs/{org}/repos is the only one of the three that reports an organisation's private + // GET /orgs/{org}/repos is the only one of the three that reports an organization's private // repositories to a token that can see them. using FakeHttpMessageHandler handler = new(); _ = handler.Respond(HttpStatusCode.OK, Fixture("github-org-repositories.json"), ("Content-Type", "application/json")); @@ -1575,14 +1575,14 @@ public enum GitHubOwnerKind User, /// - /// An organisation. Enumerates the organisation's repositories, including private ones the + /// An organization. Enumerates the organization's repositories, including private ones the /// credential can see. /// Organization, /// /// The account the credential belongs to. Enumerates every repository that account owns or can - /// reach through an organisation membership, narrowed to + /// reach through an organization membership, narrowed to /// . /// AuthenticatedUser, @@ -1599,7 +1599,7 @@ In `GitIntegration/GitHubProvider.cs`, add the property after `Name`: /// /// /// by default, which is the route and the coverage this - /// provider has always had. A caller needing an organisation's private repositories sets + /// provider has always had. A caller needing an organization's private repositories sets /// ; one needing its own sets /// . /// @@ -1633,7 +1633,7 @@ Replace the body of `GetRepositoriesAsync`, keeping its existing signature, and /// Without the filter, selecting that kind would silently ignore a configured owner, which is the /// objection that kept this method on the user route in the first place. Filtering rather than /// validating the token's login against costs no extra request - /// and still serves an organisation the token is merely a member of. + /// and still serves an organization the token is merely a member of. /// /// public override async Task> GetRepositoriesAsync(CancellationToken cancellationToken = default) @@ -1724,7 +1724,7 @@ git commit -m "feat: route GitHub repository enumeration by owner kind" ### Background -A token that is valid but not authorised for an organisation's SAML single sign-on gets `403` with an `X-GitHub-SSO` header. Its value looks like `required; url=https://github.com/orgs/contoso/sso?authorization_request=ABC123`. Without surfacing that URL, "bad token" and "good token one click from working" are the same exception with the same text. +A token that is valid but not authorised for an organization's SAML single sign-on gets `403` with an `X-GitHub-SSO` header. Its value looks like `required; url=https://github.com/orgs/contoso/sso?authorization_request=ABC123`. Without surfacing that URL, "bad token" and "good token one click from working" are the same exception with the same text. - [ ] **Step 1: Write the failing tests** @@ -1833,14 +1833,14 @@ Add to `GitIntegration/GitHubProvider.cs`, next to `TryGetRetryAfterSeconds`: `Translate`'s arms each pass `exception.Message`, so the amended message is computed once before the switch and used by the two authentication arms only. Add below the existing `responseBody` line: ```csharp - // A token that is valid but unauthorised for an organisation's single sign-on arrives as a + // A token that is valid but unauthorised for an organization's single sign-on arrives as a // plain 403, indistinguishable in status and body from a bad credential. The header is the // only thing carrying the URL that resolves it, and that URL is the whole remedy — without // it, the two failures a caller most needs to tell apart read identically. string? singleSignOnUrl = TryGetSingleSignOnUrl(exception); string authenticationMessage = singleSignOnUrl is null ? exception.Message - : $"{exception.Message} This organisation requires single sign-on authorisation for " + + : $"{exception.Message} This organization requires single sign-on authorisation for " + $"this credential. Authorise it at: {singleSignOnUrl}"; ``` @@ -2211,7 +2211,7 @@ public sealed class GitHubDeviceFlow(GitHubOAuthClientId clientId, IReadOnlyList /// /// /// Polls until the user authorises, the code expires, or is - /// cancelled, so this may block for as long as . Octokit + /// canceled, so this may block for as long as . Octokit /// handles the authorization_pending and slow_down responses internally, at the /// interval GitHub asked for. /// @@ -2389,7 +2389,7 @@ CredentialCache.Instance.AddOrReplace(persona, new CredentialWithToken { Token = The two calls are split so the user code can stay on screen for the minutes the wait may take. The flow stores nothing: where the credential lives is the caller's decision. -### Enumerating an Organisation's Private Repositories +### Enumerating an Organization's Private Repositories ```csharp GitHubProvider provider = new() @@ -2404,7 +2404,7 @@ IReadOnlyList repositories = await provider.GetRepositoriesAsync( `OwnerKind` defaults to `GitHubOwnerKind.User`, which enumerates public repositories only — the route this provider has always used. `Organization` and `AuthenticatedUser` report private repositories the -credential can see. A token that is valid but not authorised for an organisation's single sign-on +credential can see. A token that is valid but not authorised for an organization's single sign-on raises `GitHostingAuthenticationException` carrying the URL to authorise it at. ```` @@ -2429,6 +2429,6 @@ git commit -m "docs: document worktree verbs, owner kinds, and device flow" ## Open item, carried out of this plan -**The OAuth App does not exist yet.** Every test here runs against a fake transport, so no task is blocked. But nothing has been confirmed against GitHub itself, and it cannot be until someone with organisation ownership registers an OAuth App with device flow enabled and approves it for SAML single sign-on, scopes `repo` and `read:org`. +**The OAuth App does not exist yet.** Every test here runs against a fake transport, so no task is blocked. But nothing has been confirmed against GitHub itself, and it cannot be until someone with organization ownership registers an OAuth App with device flow enabled and approves it for SAML single sign-on, scopes `repo` and `read:org`. Record this in the pull request description as an unchecked item rather than claiming the device flow is verified end to end. diff --git a/docs/superpowers/plans/2026-09-24-patch-verbs.md b/docs/superpowers/plans/2026-09-24-patch-verbs.md index b8caeb6..ec27389 100644 --- a/docs/superpowers/plans/2026-09-24-patch-verbs.md +++ b/docs/superpowers/plans/2026-09-24-patch-verbs.md @@ -405,7 +405,7 @@ git add GitIntegration/Parsing/GitPatchParser.cs GitIntegration.Test/Parsing/Git git commit -m "[minor] Parse a unified diff into files, hunks and lines Each hunk keeps the bytes git emitted alongside the parsed lines. Combined -format from an unmerged path is recognised and left unparsed rather than +format from an unmerged path is recognized and left unparsed rather than turned into ordinary hunks, which would produce patches git rejects with nothing explaining why, and a rename that also changed content keeps its hunks." diff --git a/docs/superpowers/research/2026-08-21-azure-devops-rest-findings.md b/docs/superpowers/research/2026-08-21-azure-devops-rest-findings.md index ca89c01..ecc927b 100644 --- a/docs/superpowers/research/2026-08-21-azure-devops-rest-findings.md +++ b/docs/superpowers/research/2026-08-21-azure-devops-rest-findings.md @@ -169,7 +169,7 @@ internally consistent). The response example is the **only** one of the three fetched pages that shows a populated `_links` object. Its keys, verbatim: `self`, `repository`, `workItems`, `sourceBranch`, `targetBranch`, `sourceCommit`, `targetCommit`, `createdBy`, `iterations`. **There is no `web` key.** See the -contradiction section below — this directly bears on the spec's `WebURI` normalisation rule. +contradiction section below — this directly bears on the spec's `WebURI` normalization rule. ## 5. Pagination @@ -277,7 +277,7 @@ into them. 1. **`_links.web.href` is settled as unconfirmed anywhere in official Microsoft sources — not merely "not found in the first pass," but actively checked and absent everywhere it could plausibly appear.** - The spec's "Two normalisations" section states "the web link is at `_links.web.href`." Four separate + The spec's "Two normalizations" section states "the web link is at `_links.web.href`." Four separate official sources were checked, at both REST 7.1 and 7.2 where applicable, and none of them show it: - The *only* example response across every page fetched (7.1 and 7.2, both List and Create) that shows a populated `_links` at all is the Create response, and its keys are `self`, `repository`, diff --git a/docs/superpowers/specs/2026-08-19-gitintegration-v2-design.md b/docs/superpowers/specs/2026-08-19-gitintegration-v2-design.md index 9c190db..f89c0c6 100644 --- a/docs/superpowers/specs/2026-08-19-gitintegration-v2-design.md +++ b/docs/superpowers/specs/2026-08-19-gitintegration-v2-design.md @@ -44,9 +44,9 @@ the design and are not worked around silently. | No working-directory parameter | Cannot set the child process cwd | Scope every command with `git -C ` | | No environment-variable support | Cannot set `GIT_ASKPASS`, `GIT_TERMINAL_PROMPT`, `GIT_CONFIG_*` | Remote operations rely on ambient credential configuration; git may block on an auth prompt, bounded by `GitOptions.Timeout` and the caller's `CancellationToken` | | `RunCommand` is a static class | Cannot implement an interface | GitIntegration owns `IGitProcessRunner`; the shipped implementation delegates to the static API | -| `RunCommand` can return normally from a cancelled run | A cancelled command is indistinguishable from an ordinary git failure | `RunCommandGitProcessRunner` checks the linked token after the call returns and classifies it explicitly | +| `RunCommand` can return normally from a canceled run | A canceled command is indistinguishable from an ordinary git failure | `RunCommandGitProcessRunner` checks the linked token after the call returns and classifies it explicitly | -That last row is a measured behaviour of the dependency, not a theoretical one. `RunCommand.RunAsync` +That last row is a measured behavior of the dependency, not a theoretical one. `RunCommand.RunAsync` delivers cancellation two ways simultaneously — a `cancellationToken.Register(() => TryKill(process))` that kills the process, and `process.WaitForExitAsync(cancellationToken)` racing to observe the same token. When the kill wins, the process exits before the await faults, so `ExecuteAsync` returns @@ -56,7 +56,7 @@ cancellation reaching `TryKill` can lose the race, including a caller cancelling `fetch`. Left undetected this would matter a great deal, because the verb builders translate any non-success -`GitProcessResult` into `GitCommandException`: a cancelled fetch would reach the caller as +`GitProcessResult` into `GitCommandException`: a canceled fetch would reach the caller as "`GitCommandException`, exit code -1, no output", indistinguishable from a genuine git failure. So the runner checks `linked.IsCancellationRequested` after the call returns and applies the same classification as its catch clause — caller cancellation first, then timeout. @@ -140,7 +140,7 @@ public sealed class GitOptions `RunCommandGitProcessRunner` is the shipped implementation. It calls `RunCommand.ExecuteAsync(executablePath, arguments, outputHandler, cancellationToken)` — reading an executable path and timeout **snapshotted into readonly fields at construction**, so a consumer -mutating the shared `GitOptions` singleton cannot change a runner's behaviour mid-flight — +mutating the shared `GitOptions` singleton cannot change a runner's behavior mid-flight — accumulating stdout and stderr into separate `StringBuilder` instances through an `OutputHandler`. When `Timeout` is set it links a timeout token to the caller's token; `RunCommand` kills the process tree on cancellation. @@ -328,7 +328,7 @@ code and returns a `GitResult`; it still propagates cancellation and programm `GitResult` is a sealed record *class* with a private constructor, not a struct, and `Success` is derived from `Error` rather than stored. Both choices close the same hole: a struct has a -reachable `default` — from an uninitialised field, an array allocation, or a failed +reachable `default` — from an uninitialized field, an array allocation, or a failed `TryGetValue` — and with three independently stored members that default reads as "failed, but with no error", so a consumer writing `result.Error!.ExitCode` on the failure branch gets a `NullReferenceException`. Deriving `Success` alone would only move the trap to "succeeded with a @@ -659,7 +659,7 @@ hosting work and could proceed in parallel if desired. |---|---| | git output format drift between versions | Parse only documented machine formats; pin with fixtures; assert a minimum git version | | `fetch --porcelain` requires git ≥ 2.41 | Detect version once via `GetVersionAsync`; fall back to stderr parsing below that | -| Remote operations block on an auth prompt | `GitOptions.Timeout` plus caller `CancellationToken`; RunCommand kills the process tree. The timeout surfaces as `GitTimeoutException`, never as a bare `OperationCanceledException`, so a caller can tell "git hung" (retryable) from "I cancelled" (not) | +| Remote operations block on an auth prompt | `GitOptions.Timeout` plus caller `CancellationToken`; RunCommand kills the process tree. The timeout surfaces as `GitTimeoutException`, never as a bare `OperationCanceledException`, so a caller can tell "git hung" (retryable) from "I canceled" (not) | | Azure DevOps client packages are large and `netstandard2.0` | Acceptable; the same pair is already used by `ktsu.BuildMonitor` | | Merged `GitRepository` mixes local and hosting concerns | Metadata is nullable rather than blank; `RemotePath` back-filled from `origin`; verbs fail with a specific exception type | @@ -671,7 +671,7 @@ Every RunCommand limitation this design originally worked around was filed again | Issue | Gap | Fix | Status here | |---|---|---|---| -| #38 | A cancelled run could return normally with a killed process's exit code | `ThrowIfCancellationRequested()` after the await | Adopted; our own guard kept as defence in depth | +| #38 | A canceled run could return normally with a killed process's exit code | `ThrowIfCancellationRequested()` after the await | Adopted; our own guard kept as defence in depth | | #39 | No working-directory support | `CommandOptions.WorkingDirectory` | Available; `git -C` retained deliberately | | #40 | No environment-variable support | `CommandOptions.EnvironmentVariables` | **Adopted** — see below | | #41 | `Execute(string)` split on the first space | String overloads obsoleted | Not applicable; only the argv overload is used | diff --git a/docs/superpowers/specs/2026-08-21-gitintegration-hosting-layer-design.md b/docs/superpowers/specs/2026-08-21-gitintegration-hosting-layer-design.md index c01318e..e2ba5cb 100644 --- a/docs/superpowers/specs/2026-08-21-gitintegration-hosting-layer-design.md +++ b/docs/superpowers/specs/2026-08-21-gitintegration-hosting-layer-design.md @@ -33,7 +33,7 @@ largest), PR comments and reviews, repository creation, and webhooks. This removes two public members, which normally forces a major bump. It ships as **`[minor]` — 2.4.0** by explicit decision: the members never worked, so no consumer can depend on their -behaviour, and the removal produces a compile error rather than a silent behavioural change. Burning +behavior, and the removal produces a compile error rather than a silent behavioral change. Burning a major version on withdrawing a no-op is not worth the signal it sends. ## The abstraction @@ -129,19 +129,19 @@ exists in `SemanticTypes/GitProviderTypes.cs` with nothing behind it. `ktsu.CredentialCache` resolves a credential by `PersonaGUID`. Handling, in full: -| Credential | Behaviour | +| Credential | Behavior | |---|---| | `CredentialWithToken` | Used. GitHub: token auth. Azure DevOps: Basic, empty username, token as password. | | `CredentialWithUsernamePassword` | Used as supplied. | | `CredentialWithNothing`, or none resolved | Proceed **unauthenticated**, documented. | | Any other subtype | Throw. | -Today's `GitHubProvider` recognises only `CredentialWithUsernamePassword` and silently does nothing +Today's `GitHubProvider` recognizes only `CredentialWithUsernamePassword` and silently does nothing for anything else, so a `CredentialWithToken` — the natural fit for a PAT on both hosts — is ignored without a word. That is the bug this table fixes. Proceeding unauthenticated is deliberate and is not the same bug: enumerating public repositories -without credentials is legitimate, and refusing it would break a real use. An *unrecognised* subtype +without credentials is legitimate, and refusing it would break a real use. An *unrecognized* subtype is different — it means the caller configured something we do not understand, and continuing as though nothing were configured would hide that. @@ -186,9 +186,9 @@ The one place a faithful-looking mapping could quietly lie, so it is written dow | Azure DevOps | `status: completed` | `Merged` | | Azure DevOps | `status: abandoned` | `Closed` | -### Two normalisations +### Two normalizations -- **Branch refs.** Azure DevOps returns `refs/heads/main`; GitHub returns `main`. Both normalise to a +- **Branch refs.** Azure DevOps returns `refs/heads/main`; GitHub returns `main`. Both normalize to a bare `GitBranchName`, so callers never branch on host. - **Web URI.** Azure DevOps's `url` field is the **API** URL, not the browser one; the web link is at `_links.web.href`. Populating `WebURI` from `url` would put something plausible and wrong in a diff --git a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md index f1036b6..933251d 100644 --- a/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md +++ b/docs/superpowers/specs/2026-09-21-worktrees-and-github-auth-design.md @@ -15,18 +15,18 @@ Three things it needs do not exist here: creates, lists, or removes anything. 2. **GitHub repositories it can actually see.** `GitHubProvider.GetRepositoriesAsync` calls `GET /users/{login}/repos`, which returns public repositories only. The launcher's repositories are - private and live under an organisation behind SAML single sign-on, so today it would enumerate an + private and live under an organization behind SAML single sign-on, so today it would enumerate an empty list and be right to. 3. **A way to sign in.** Credentials resolve from `ktsu.CredentialCache` or from a `CredentialSource` callback. Both assume a credential already exists. Nothing in the library obtains one, and a desktop application cannot ask its user to run `gh auth login` first. -Everything here is additive. No existing member changes shape, and no existing behaviour changes for +Everything here is additive. No existing member changes shape, and no existing behavior changes for a caller that does not opt in. ## Versioning -**`[minor]` — 3.2.0.** New types and new members only. The one existing method whose behaviour is +**`[minor]` — 3.2.0.** New types and new members only. The one existing method whose behavior is touched, `GitHubProvider.GetRepositoriesAsync`, keeps its current route and its current documented coverage under the new property's default. @@ -116,7 +116,7 @@ detached prunable gitdir file points to non-existent location ``` -`branch` arrives fully qualified and is stripped to a bare `refs/heads/` name, the same normalisation +`branch` arrives fully qualified and is stripped to a bare `refs/heads/` name, the same normalization `AzureDevOpsProvider.StripRefsHeadsPrefix` already applies on the hosting side. A caller should not have to know which half of this library produced a branch name to know its shape. @@ -197,18 +197,18 @@ English prose to tell a caller something it could have read structurally. ## GitHub owner kinds This capability was superseded before it shipped. While this branch was in flight, upstream merged -PR #115, which reached the same goal — an organisation's private repositories becoming visible to a +PR #115, which reached the same goal — an organization's private repositories becoming visible to a credential that can see them — by inferring the owner's account type from `GET /users/{login}` and routing automatically, rather than by taking an explicit `OwnerKind` from the caller. That approach also handles cases the design below did not: an unauthenticated provider skips the probe entirely, a GitHub App installation token whose `GET /user` answers `403` falls back to the public route instead of failing, and the routing decision is never inferred from a `404`, which an SSO-blocked -organisation answers just as an absent one would. This branch ships upstream's version; the +organization answers just as an absent one would. This branch ships upstream's version; the `GitHubOwnerKind` enum and `OwnerKind` property described below were not merged. ### The single sign-on failure -A token that is valid but not authorised for an organisation's SAML single sign-on receives `403` with +A token that is valid but not authorised for an organization's SAML single sign-on receives `403` with an `X-GitHub-SSO` header whose value carries the URL the user must visit to authorise it. `Translate` already routes `403` without rate-limit headers to `GitHostingAuthenticationException`, @@ -311,7 +311,7 @@ No new exception family. Everything lands in the hosting hierarchy that already | `access_denied` (user refused) | `GitHostingAuthenticationException` | | `expired_token` (code timed out) | `GitHostingAuthenticationException` | | `incorrect_client_credentials`, `unsupported_grant_type`, `device_flow_disabled` | `GitHostingRequestException` | -| an error code this library does not recognise | `GitHostingRequestException` | +| an error code this library does not recognize | `GitHostingRequestException` | | transport failure, unparsable body, a non-absolute `verification_uri` | `GitHostingRequestException` | Denial and expiry share an exception and are distinguished by message. They are the same fact to a @@ -339,7 +339,7 @@ this library is not that party. ### The external prerequisite **Nothing in this section can be exercised against GitHub until an OAuth App exists.** Someone with -organisation ownership must register one with device flow enabled and approve it for SAML single +organization ownership must register one with device flow enabled and approve it for SAML single sign-on. Scopes: `repo` and `read:org`. Until then the implementation is written and tested entirely against a fake transport, which is how the diff --git a/docs/superpowers/specs/2026-09-24-patch-verbs-design.md b/docs/superpowers/specs/2026-09-24-patch-verbs-design.md index 9e23185..1d6048e 100644 --- a/docs/superpowers/specs/2026-09-24-patch-verbs-design.md +++ b/docs/superpowers/specs/2026-09-24-patch-verbs-design.md @@ -160,7 +160,7 @@ wanting to unstage a binary file has nothing else, since a binary file has no hu Four cases where the model reports rather than pretends: **Conflicted files.** An unmerged path produces combined format, with `@@@` and two columns, which is -not an applyable patch. The parser recognises it and sets `IsConflicted`, leaving `Hunks` empty. A +not an applyable patch. The parser recognizes it and sets `IsConflicted`, leaving `Hunks` empty. A caller stages such a file whole or not at all. Mis-parsing combined hunks into ordinary ones would produce patches git rejects, with nothing on screen explaining why. From 4faaf97b0205bd892beba123684b2abc78a9f51e Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 20:41:24 +1000 Subject: [PATCH 09/13] [minor] Add the index-moved case Apply's Checked() actually guards ToIndex() maps to --cached, and git's own manual for it says --cached checks only the index entry. The working-tree case already covered here never touches the index between reading a patch and applying it, so it cannot be the guarantee ToIndex().Checked() depends on. This pins the case that is: staging an unrelated edit between reading the patch and applying it, so the index no longer matches the preimage, and confirming the index still holds what was staged rather than the patch's content afterward. --- .../Integration/GitPatchRoundTripTests.cs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs index 638e92a..961c0b2 100644 --- a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs @@ -106,5 +106,52 @@ await IntegrationGitFixture.ConfigureIdentityAsync( Assert.AreEqual(0, staged.Count, "--check must change nothing."); } + [TestMethod] + public async Task CheckedReportsFailureWhenTheIndexMovedAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + + // Seed and commit a file, then change it without staging the change. + GitInitResult init = await client.Init(repository.Root) + .WithInitialBranch("main".As()) + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + await IntegrationGitFixture.ConfigureIdentityAsync( + init.Repository, AuthorName, AuthorEmail, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + repository.WriteFile("f.txt", "one\ntwo\nthree\n"); + _ = await init.Repository.Add().All() + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + _ = await init.Repository.Commit("seed".As()) + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + repository.WriteFile("f.txt", "one\nCHANGED\nthree\n"); + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + GitFilePatch file = (await opened.Patch().ExecuteAsync().ConfigureAwait(false)).Files.Single(); + string text = file.PatchFor(file.Hunks); + + // Stage a different edit to the same line, so the index no longer holds the content the + // patch's preimage expects. --cached never reads the working tree, so only moving the + // index this way can make ToIndex().Checked() refuse. + repository.WriteFile("f.txt", "one\nDIFFERENT\nthree\n"); + _ = await opened.Add().All().ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + GitResult result = await opened.Apply(text).ToIndex().Checked() + .TryExecuteAsync().ConfigureAwait(false); + + Assert.IsFalse(result.Success, "A caller's refusal depends on this reporting failure rather than throwing."); + + string indexContent = await new GitTextBuilder(opened.ProcessRunner!, opened.LocalPath, "show", ":f.txt") + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + Assert.AreEqual( + "one\nDIFFERENT\nthree", + indexContent, + "--check must change nothing: the index should still hold what was staged, not the patch's content."); + } + public TestContext TestContext { get; set; } = null!; } From 16b88351f2cbbf37fde1a5c5a432a05b8e841ad9 Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 20:54:33 +1000 Subject: [PATCH 10/13] [minor] Add Unstage for the whole-file case git restore arrived in 2.23, so this probes the installed version and falls back to reset below it, the same shape Fetch already uses for porcelain. Unstaging a hunk is Apply reversed. This is the file-level verb, and the only one available for a binary file, which has no hunks. --- .../Builders/GitRestoreBuilderTests.cs | 70 +++++++++++ GitIntegration/Builders/GitRestoreBuilder.cs | 113 ++++++++++++++++++ GitIntegration/GitRepository.cs | 6 + 3 files changed, 189 insertions(+) create mode 100644 GitIntegration.Test/Builders/GitRestoreBuilderTests.cs create mode 100644 GitIntegration/Builders/GitRestoreBuilder.cs diff --git a/GitIntegration.Test/Builders/GitRestoreBuilderTests.cs b/GitIntegration.Test/Builders/GitRestoreBuilderTests.cs new file mode 100644 index 0000000..96c4cc4 --- /dev/null +++ b/GitIntegration.Test/Builders/GitRestoreBuilderTests.cs @@ -0,0 +1,70 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration.Test; + +using System; +using System.Collections.Generic; +using System.Threading.Tasks; + +using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; + +[TestClass] +public class GitRestoreBuilderTests +{ + [TestMethod] + public void BuildsTheRestoreVectorOnAModernGit() + { + RecordingGitProcessRunner runner = new(); + GitRestoreBuilder builder = new(runner, TestPaths.Root, "f.txt".As()); + + IReadOnlyList arguments = builder.BuildArguments(); + + Assert.IsTrue(arguments.Contains("restore")); + Assert.IsTrue(arguments.Contains("--staged")); + Assert.IsTrue(arguments.Contains("f.txt")); + } + + [TestMethod] + public void RefusesANullPath() + { + RecordingGitProcessRunner runner = new(); + GitRepository repository = new() { LocalPath = TestPaths.Root, ProcessRunner = runner }; + + _ = Assert.ThrowsExactly(() => _ = repository.Unstage(null!)); + } + + [TestMethod] + public async Task FallsBackToResetOnAGitOlderThanRestoreAsync() + { + // git restore arrived in 2.23. Below that, unstaging goes through reset HEAD instead, which + // every supported git understands. + ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() + .Then(standardOutput: "git version 2.22.0\n") + .Then(standardOutput: string.Empty); + GitRestoreBuilder builder = new(runner, TestPaths.Root, "f.txt".As()); + + _ = await builder.ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + string[] arguments = [.. runner.Invocations[1]]; + Assert.AreSequenceEqual(["reset", "HEAD", "--end-of-options", "f.txt"], arguments[^4..]); + } + + [TestMethod] + public async Task TreatsExactlyTwoTwentyThreeAsSupportedAsync() + { + // The documented floor, asserted exactly: an off-by-one here silently falls back to reset + // for every user on the first version that supports restore. + ScriptedGitProcessRunner runner = new ScriptedGitProcessRunner() + .Then(standardOutput: "git version 2.23.0\n") + .Then(standardOutput: string.Empty); + GitRestoreBuilder builder = new(runner, TestPaths.Root, "f.txt".As()); + + _ = await builder.ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + string[] arguments = [.. runner.Invocations[1]]; + Assert.AreSequenceEqual(["restore", "--staged", "--end-of-options", "f.txt"], arguments[^4..]); + } + + public TestContext TestContext { get; set; } = null!; +} diff --git a/GitIntegration/Builders/GitRestoreBuilder.cs b/GitIntegration/Builders/GitRestoreBuilder.cs new file mode 100644 index 0000000..510e70d --- /dev/null +++ b/GitIntegration/Builders/GitRestoreBuilder.cs @@ -0,0 +1,113 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.GitIntegration; + +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +using ktsu.Semantics.Paths; + +/// +/// Removes a whole file's staged changes, leaving the working tree alone. +/// +/// +/// Unstaging one hunk of a file's patch is Apply(text).ToIndex().Reversed(). This builder is +/// the file-level verb, and the only option for a binary file, which has no hunks to apply in +/// reverse. +/// +public interface IGitRestoreBuilder : IGitCommandBuilder +{ +} + +/// +/// Builds git restore --staged, with a fallback to git reset HEAD on a git older than +/// the one that introduced restore. +/// +/// +/// git restore arrived in git 2.23. Below that, unstaging a path goes through git reset +/// HEAD -- <path> instead, which every supported git understands. The choice follows the +/// same shape as : a version probe runs in +/// and before the vector is built, because BuildArguments is +/// documented as a pure computation with no I/O and so cannot probe for itself. +/// +/// Runs the assembled command. +/// The repository to scope the command to. +/// The path, relative to the repository root, to unstage. +internal sealed class GitRestoreBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath, RelativeFilePath path) + : GitCommandBuilder(runner, repositoryPath), IGitRestoreBuilder +{ + /// The first git release whose restore command exists. + private const int RestoreMajor = 2; + private const int RestoreMinor = 23; + + private readonly RelativeFilePath _path = Ensure.NotNull(path); + + /// + /// Gets or sets a value indicating whether the installed git is new enough for restore. + /// + /// + /// Defaults true so BuildArguments emits the modern form until an execution path tells it + /// otherwise, matching 's own default. + /// + private bool RestoreSupportedByVersion { get; set; } = true; + + /// + protected override void AppendVerbArguments(ICollection arguments) + { + Ensure.NotNull(arguments); + + if (RestoreSupportedByVersion) + { + arguments.Add("restore"); + arguments.Add("--staged"); + } + else + { + arguments.Add("reset"); + arguments.Add("HEAD"); + } + + AppendOperands(arguments, _path.WeakString); + } + + /// + protected override GitCompleted ParseResult(GitProcessResult result) => + new() { Arguments = Ensure.NotNull(result).Arguments }; + + /// + public override async Task ExecuteAsync(CancellationToken cancellationToken = default) + { + await ProbeVersionAsync(cancellationToken).ConfigureAwait(false); + + return await base.ExecuteAsync(cancellationToken).ConfigureAwait(false); + } + + /// + public override async Task> TryExecuteAsync(CancellationToken cancellationToken = default) + { + await ProbeVersionAsync(cancellationToken).ConfigureAwait(false); + + return await base.TryExecuteAsync(cancellationToken).ConfigureAwait(false); + } + + /// + /// Asks the installed git what version it is, so the vector can be built to suit. + /// + /// + /// Goes through rather than + /// , and the same way regardless of which + /// of this builder's own two entry points is running: a failed probe means the version genuinely + /// could not be established, and falling back to reset, which every supported git + /// understands, is the safer default. Mirroring each caller's own strictness would make + /// throw a version exception for what is really a restore problem. + /// + /// A token to observe while probing. + private async Task ProbeVersionAsync(CancellationToken cancellationToken) + { + GitResult probe = await new GitVersionBuilder(Runner) + .TryExecuteAsync(cancellationToken).ConfigureAwait(false); + + RestoreSupportedByVersion = probe.Success && probe.Value!.AtLeast(RestoreMajor, RestoreMinor); + } +} diff --git a/GitIntegration/GitRepository.cs b/GitIntegration/GitRepository.cs index 199f4f7..55c26b0 100644 --- a/GitIntegration/GitRepository.cs +++ b/GitIntegration/GitRepository.cs @@ -152,6 +152,12 @@ public IGitApplyBuilder Apply(string patchText) return new GitApplyBuilder(RequireRunner(), RequireLocalPath(), patchText); } + /// Removes a path's staged changes, leaving the working tree alone. + /// The path, relative to the repository root. + /// The builder. + public IGitRestoreBuilder Unstage(RelativeFilePath path) => + new GitRestoreBuilder(RequireRunner(), RequireLocalPath(), Ensure.NotNull(path)); + /// Resolves a revision to the object id it names. /// The revision to resolve. /// A fresh builder. From 6f6391b1bb431a1c42d242b322b79dabe69eba3b Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 21:24:13 +1000 Subject: [PATCH 11/13] [patch] Make Patch survive a hostile diff configuration diff.noprefix and diff.mnemonicPrefix rewrite the a/ and b/ path prefixes the header parser finds a file by, so either setting killed the verb outright for the user who had it, and left a patch that needed -p0 to apply. The vector now pins --src-prefix=a/ and --dst-prefix=b/, which override both. diff.suppressBlankEmpty prints an empty context line as a bare newline, which ended the hunk body at the first blank line and then handed a content line to the hunk header parser, where it failed as an index exception rather than as GitParseException. Both halves are fixed: -c diff.suppressBlankEmpty=false on the vector, and a loop that requires "@@ " before it parses a hunk at all. WithContext now refuses zero. git apply reads a zero-context patch only with --unidiff-zero, which this library does not offer, so it was the one pairing of the two new verbs that could never work, and the failure named the index rather than the setting. A conflicted file's Kind is Unmerged, which is what GitDiffParser already reports for the same path, so a caller switching on GitChangeKind no longer gets two answers from two verbs. Two fixtures added for the new file and deleted file headers, which nothing had ever run through this parser, captured from git 2.54.0 the way the other six were. --- .../Builders/GitPatchBuilderTests.cs | 50 +++++++++++++++++ .../Fixtures/patch-deleted-file.txt | 9 ++++ .../Fixtures/patch-new-file.txt | 9 ++++ .../Parsing/GitPatchParserTests.cs | 52 +++++++++++++++++- GitIntegration/Builders/GitPatchBuilder.cs | 53 +++++++++++++++++-- GitIntegration/Parsing/GitPatchParser.cs | 11 +++- 6 files changed, 176 insertions(+), 8 deletions(-) create mode 100644 GitIntegration.Test/Fixtures/patch-deleted-file.txt create mode 100644 GitIntegration.Test/Fixtures/patch-new-file.txt diff --git a/GitIntegration.Test/Builders/GitPatchBuilderTests.cs b/GitIntegration.Test/Builders/GitPatchBuilderTests.cs index 8407738..56d090e 100644 --- a/GitIntegration.Test/Builders/GitPatchBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitPatchBuilderTests.cs @@ -3,6 +3,7 @@ namespace ktsu.GitIntegration.Test; using System; +using System.Collections.Generic; using System.Linq; [TestClass] @@ -20,15 +21,51 @@ public void BuildsTheDefaultPatchVector() "--no-pager", "-c", "core.quotepath=false", "-c", "color.ui=false", + "-c", "diff.suppressBlankEmpty=false", "diff", "--no-ext-diff", "--no-textconv", "--no-color", + "--src-prefix=a/", + "--dst-prefix=b/", ]; Assert.AreSequenceEqual(expectedArguments, builder.BuildArguments()); } + [TestMethod] + public void AlwaysPinsBothPathPrefixes() + { + RecordingGitProcessRunner runner = new(); + GitPatchBuilder builder = new(runner, TestPaths.Root); + + IReadOnlyList arguments = builder.BuildArguments(); + + Assert.IsTrue( + arguments.Contains("--src-prefix=a/"), + "diff.noprefix emits 'diff --git f.txt f.txt' and diff.mnemonicPrefix emits 'diff --git i/f.txt w/f.txt'. Neither header carries a new-side path this parser can read, and neither patch applies without -p0."); + Assert.IsTrue( + arguments.Contains("--dst-prefix=b/"), + "diff.noprefix emits 'diff --git f.txt f.txt' and diff.mnemonicPrefix emits 'diff --git i/f.txt w/f.txt'. Neither header carries a new-side path this parser can read, and neither patch applies without -p0."); + } + + [TestMethod] + public void AlwaysDisablesBlankEmptySuppression() + { + RecordingGitProcessRunner runner = new(); + GitPatchBuilder builder = new(runner, TestPaths.Root); + + List arguments = [.. builder.BuildArguments()]; + int setting = arguments.IndexOf("diff.suppressBlankEmpty=false"); + + Assert.IsTrue( + setting > 0 && arguments[setting - 1] == "-c", + "diff.suppressBlankEmpty prints an empty context line as a bare newline, which ends the hunk body early and truncates the text a round trip depends on."); + Assert.IsTrue( + setting < arguments.IndexOf("diff"), + "Git reads -c only before the subcommand."); + } + [TestMethod] public void MapsTheOptionFlags() { @@ -66,4 +103,17 @@ public void RefusesANegativeContextCount() _ = Assert.ThrowsExactly(() => _ = builder.WithContext(-1)); } + + [TestMethod] + public void RefusesAZeroContextCount() + { + RecordingGitProcessRunner runner = new(); + GitPatchBuilder builder = new(runner, TestPaths.Root); + + ArgumentOutOfRangeException thrown = Assert.ThrowsExactly( + () => _ = builder.WithContext(0), + "git apply refuses a zero-context patch without --unidiff-zero, which IGitApplyBuilder does not offer, so this is the one pairing of the library's own two verbs that could never work."); + + StringAssert.Contains(thrown.Message, "--unidiff-zero", StringComparison.Ordinal); + } } diff --git a/GitIntegration.Test/Fixtures/patch-deleted-file.txt b/GitIntegration.Test/Fixtures/patch-deleted-file.txt new file mode 100644 index 0000000..e87385a --- /dev/null +++ b/GitIntegration.Test/Fixtures/patch-deleted-file.txt @@ -0,0 +1,9 @@ +diff --git a/gone.txt b/gone.txt +deleted file mode 100644 +index 8b05eae..0000000 +--- a/gone.txt ++++ /dev/null +@@ -1,3 +0,0 @@ +-alpha +-bravo +-charlie diff --git a/GitIntegration.Test/Fixtures/patch-new-file.txt b/GitIntegration.Test/Fixtures/patch-new-file.txt new file mode 100644 index 0000000..9769e1f --- /dev/null +++ b/GitIntegration.Test/Fixtures/patch-new-file.txt @@ -0,0 +1,9 @@ +diff --git a/added.txt b/added.txt +new file mode 100644 +index 0000000..ff6e6b1 +--- /dev/null ++++ b/added.txt +@@ -0,0 +1,3 @@ ++first ++second ++third diff --git a/GitIntegration.Test/Parsing/GitPatchParserTests.cs b/GitIntegration.Test/Parsing/GitPatchParserTests.cs index 45dca26..013e6de 100644 --- a/GitIntegration.Test/Parsing/GitPatchParserTests.cs +++ b/GitIntegration.Test/Parsing/GitPatchParserTests.cs @@ -10,7 +10,9 @@ namespace ktsu.GitIntegration.Test; public class GitPatchParserTests { // Fixtures captured from git version 2.54.0 (Apple Git-157) on macOS, with - // `git -c color.ui=false diff --no-ext-diff --no-textconv -U3`. + // `git -c color.ui=false diff --no-ext-diff --no-textconv -U3`. patch-new-file.txt and + // patch-deleted-file.txt add `--cached` to that, because `new file mode` and `deleted file + // mode` headers only appear for a change that is already in the index. /// Reads a captured fixture's raw text from the test output's Fixtures directory. private static string Fixture(string name) => @@ -93,6 +95,54 @@ public void ParsesCarriageReturnContentWithoutStrippingIt() "A patch is byte-sensitive, so content line endings survive parsing."); } + [TestMethod] + public void ReadsAnAddedFileAsAdded() + { + GitFilePatch file = GitPatchParser.Parse(Fixture("patch-new-file.txt")).Files.Single(); + + Assert.AreEqual(GitChangeKind.Added, file.Kind); + Assert.AreEqual("added.txt", file.Path.WeakString); + Assert.AreEqual(1, file.Hunks.Count); + Assert.IsTrue( + file.Hunks.Single().Lines.All(line => line.Kind == GitPatchLineKind.Added), + "A new file's only hunk is every one of its lines added, with no context to anchor it."); + } + + [TestMethod] + public void ReadsADeletedFileAsDeleted() + { + GitFilePatch file = GitPatchParser.Parse(Fixture("patch-deleted-file.txt")).Files.Single(); + + Assert.AreEqual(GitChangeKind.Deleted, file.Kind); + Assert.AreEqual("gone.txt", file.Path.WeakString); + Assert.AreEqual(1, file.Hunks.Count); + Assert.IsTrue( + file.Hunks.Single().Lines.All(line => line.Kind == GitPatchLineKind.Removed), + "A deleted file's only hunk is every one of its lines removed."); + } + + [TestMethod] + public void ReadsAConflictedFileAsUnmerged() => + Assert.AreEqual( + GitChangeKind.Unmerged, + GitPatchParser.Parse(Fixture("patch-conflict.txt")).Files.Single().Kind, + "GitDiffParser reports the same path as Unmerged, and a caller switching on one enum must not get two answers for it."); + + [TestMethod] + public void StopsAtALineThatIsNotAHunkHeader() + { + // diff.suppressBlankEmpty writes an empty context line as a bare newline. The hunk body + // ends there, and what follows must not reach the header parser, which would index past + // the end of a short line. + string output = + "diff --git a/f.txt b/f.txt\nindex 111..222 100644\n--- a/f.txt\n+++ b/f.txt\n" + + "@@ -1,3 +1,3 @@\n a\n\n-b\n+B\n"; + + GitFilePatch file = GitPatchParser.Parse(output).Files.Single(); + + Assert.AreEqual(1, file.Hunks.Count); + } + [TestMethod] public void ParsesAnEmptyDiffAsNoFiles() => Assert.AreEqual(0, GitPatchParser.Parse(string.Empty).Files.Count); diff --git a/GitIntegration/Builders/GitPatchBuilder.cs b/GitIntegration/Builders/GitPatchBuilder.cs index 7c6808f..d290aec 100644 --- a/GitIntegration/Builders/GitPatchBuilder.cs +++ b/GitIntegration/Builders/GitPatchBuilder.cs @@ -11,6 +11,18 @@ namespace ktsu.GitIntegration; /// Reads a patch: the hunks and lines a caller needs to show or stage a change, rather than the /// per-file counts reports. /// +/// +/// An untracked file never appears, because git diff does not show one. Staging it is +/// . A staging view therefore draws its file list from +/// and its hunks from here, and those two sources disagree about +/// untracked content. +/// +/// Git's output is decoded as UTF-8, so a file whose bytes are not valid UTF-8 comes back with +/// every invalid byte replaced by U+FFFD. Git diffs such a file as text, because it decides binary +/// on NUL bytes rather than on encoding validity, and refuses a +/// patch carrying that character rather than staging the replacement bytes. +/// +/// public interface IGitPatchBuilder : IGitCommandBuilder { /// Compares the index against HEAD instead of the working tree against the index. @@ -18,9 +30,18 @@ public interface IGitPatchBuilder : IGitCommandBuilder public IGitPatchBuilder Staged(); /// Sets how many lines of surrounding context each hunk carries. - /// The number of context lines. Git's own default applies when this is never called. + /// + /// Zero is refused. A zero-context patch is one git apply accepts only under + /// --unidiff-zero, an option does not offer, so a hunk read + /// that way could never be staged through this library and the failure would name the index + /// rather than the setting that caused it. + /// + /// + /// The number of context lines, at least one. Git's own default applies when this is never + /// called. + /// /// The same builder, to allow chaining. - /// is negative. + /// is less than one. public IGitPatchBuilder WithContext(int lines); /// Limits the result to this path. May be called more than once. @@ -54,8 +75,8 @@ public interface IGitPatchBuilder : IGitCommandBuilder } /// -/// Builds git diff --no-ext-diff --no-textconv --no-color and parses its output through -/// . +/// Builds git diff, with every option and setting pinned that would otherwise change the +/// shape of the patch, and parses its output through . /// /// Runs the assembled command. /// The repository to scope the command to. @@ -80,7 +101,16 @@ public IGitPatchBuilder Staged() /// public IGitPatchBuilder WithContext(int lines) { - ArgumentOutOfRangeException.ThrowIfNegative(lines); + if (lines < 1) + { + throw new ArgumentOutOfRangeException( + nameof(lines), + lines, + "A hunk needs at least one line of context. Git apply reads a zero-context patch only " + + "with --unidiff-zero, which this library does not offer, so a patch read that way " + + "could never be staged back through Apply."); + } + _contextLines = lines; return this; } @@ -118,6 +148,13 @@ protected override void AppendVerbArguments(ICollection arguments) { Ensure.NotNull(arguments); + // diff.suppressBlankEmpty makes git print an empty context line as a bare newline with no + // leading space, which a patch reader cannot tell from the end of a hunk. A -c on the + // vector beats the value in any config file, and it has to precede the subcommand to be + // read at all. + arguments.Add("-c"); + arguments.Add("diff.suppressBlankEmpty=false"); + arguments.Add("diff"); // Correctness, not tidiness. A repository with a gitattributes diff driver emits @@ -129,6 +166,12 @@ protected override void AppendVerbArguments(ICollection arguments) arguments.Add("--no-textconv"); arguments.Add("--no-color"); + // diff.noprefix drops the a/ and b/ prefixes entirely, and diff.mnemonicPrefix replaces + // them with i/ and w/. Either leaves a header with no new-side path for the parser to find, + // and a patch that needs -p0 to apply. Pinning both prefixes overrides both settings. + arguments.Add("--src-prefix=a/"); + arguments.Add("--dst-prefix=b/"); + if (_staged) { arguments.Add("--cached"); diff --git a/GitIntegration/Parsing/GitPatchParser.cs b/GitIntegration/Parsing/GitPatchParser.cs index 336ff38..453c4a9 100644 --- a/GitIntegration/Parsing/GitPatchParser.cs +++ b/GitIntegration/Parsing/GitPatchParser.cs @@ -144,8 +144,11 @@ private static GitFilePatch ParseFile(string output, List<(int Start, int End)> if (boundary.StartsWith(ConflictHunkPrefix, StringComparison.Ordinal)) { // Combined format from an unmerged path is not a patch git apply accepts, so its - // body is skipped rather than misread as ordinary hunks. + // body is skipped rather than misread as ordinary hunks. Kind follows the same + // enum member GitDiffParser reports for the path, so a caller switching on + // GitChangeKind gets one answer from both verbs. isConflicted = true; + kind = GitChangeKind.Unmerged; index++; while (index < lines.Count && !IsFileStart(Line(output, lines, index))) @@ -155,7 +158,11 @@ private static GitFilePatch ParseFile(string output, List<(int Start, int End)> } else if (!isBinary) { - while (index < lines.Count && !IsFileStart(Line(output, lines, index))) + // Only a hunk header may reach ParseHunk. Stopping at anything else keeps a content + // line git emitted in a shape this parser does not recognize out of a range parser, + // where it would fail as an index exception rather than as GitParseException. + while (index < lines.Count && + Line(output, lines, index).StartsWith(HunkPrefix, StringComparison.Ordinal)) { hunks.Add(ParseHunk(output, lines, ref index)); } From 550c981ad314a4079e00b98436b24d1c1c5baaf1 Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 21:24:24 +1000 Subject: [PATCH 12/13] [patch] Refuse the patch inputs Apply cannot honor PatchFor filtered this file's own hunks and counted the caller's input, so a hunk from another file was dropped without a word. Pass only foreign hunks and a header with no body reached git as the corrupt-patch error the empty check exists to prevent. Pass a mix, as a multi-file selection would, and some hunks were staged while the rest vanished. It now counts what it emitted and says which file the rest did not belong to. The remark claiming apply would catch this is replaced by one describing what the code does. Apply refuses patch text carrying U+FFFD. Git's output is read as UTF-8, so a repository holding Latin-1 or Shift-JIS text, which git diffs as text because it decides binary on NUL bytes, loses every invalid byte to the replacement character. With ASCII context around it the patch applied and staged mojibake while the working tree kept the original bytes. A byte-level runner is the real fix and is out of scope, so the constraint is documented on GitHunk.Text and IGitPatchBuilder and the corruption is turned into a refusal. GitRestoreBuilder separates its path with a bare -- rather than through AppendOperands. --end-of-options arrived in git 2.24, one release after restore itself, so on the old versions the reset fallback exists for it was read as a pathspec and the command failed. The remarks say so, and no other builder changes. Apply's temporary file is written inside the try whose finally removes it, and the removal no longer replaces git's exception with its own. Unstage validates its path before reaching for the runner, and both new verbs document the exceptions their siblings already did. --- .../Builders/GitApplyBuilderTests.cs | 13 ++ .../Builders/GitRestoreBuilderTests.cs | 21 ++- .../Integration/GitPatchRoundTripTests.cs | 132 +++++++++++++++++- GitIntegration.Test/Models/GitPatchTests.cs | 22 +++ GitIntegration/Builders/GitApplyBuilder.cs | 39 ++++-- GitIntegration/Builders/GitRestoreBuilder.cs | 19 ++- GitIntegration/GitRepository.cs | 43 +++++- GitIntegration/Models/GitPatch.cs | 47 ++++++- 8 files changed, 310 insertions(+), 26 deletions(-) diff --git a/GitIntegration.Test/Builders/GitApplyBuilderTests.cs b/GitIntegration.Test/Builders/GitApplyBuilderTests.cs index 30f8d42..1f67bbf 100644 --- a/GitIntegration.Test/Builders/GitApplyBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitApplyBuilderTests.cs @@ -47,6 +47,19 @@ public void RefusesEmptyPatchText() _ = Assert.ThrowsExactly(() => _ = repository.Apply(" ")); } + [TestMethod] + public void RefusesPatchTextCarryingTheReplacementCharacter() + { + RecordingGitProcessRunner runner = new(); + GitRepository repository = new() { LocalPath = TestPaths.Root, ProcessRunner = runner }; + + ArgumentException thrown = Assert.ThrowsExactly( + () => _ = repository.Apply("@@ -1 +1 @@\n-caf\uFFFD\n+cafe\n"), + "Bytes that are not valid UTF-8 decode to U+FFFD, and with ASCII context around them the patch applies and stages the replacement character while the working tree keeps the original bytes."); + + StringAssert.Contains(thrown.Message, "U+FFFD", StringComparison.Ordinal); + } + [TestMethod] public async Task DeletesTheTemporaryFileEvenWhenGitFailsAsync() { diff --git a/GitIntegration.Test/Builders/GitRestoreBuilderTests.cs b/GitIntegration.Test/Builders/GitRestoreBuilderTests.cs index 96c4cc4..7e1fa02 100644 --- a/GitIntegration.Test/Builders/GitRestoreBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitRestoreBuilderTests.cs @@ -25,6 +25,23 @@ public void BuildsTheRestoreVectorOnAModernGit() Assert.IsTrue(arguments.Contains("f.txt")); } + [TestMethod] + public void SeparatesThePathWithABareDoubleDash() + { + RecordingGitProcessRunner runner = new(); + GitRestoreBuilder builder = new(runner, TestPaths.Root, "f.txt".As()); + + string[] arguments = [.. builder.BuildArguments()]; + + Assert.AreSequenceEqual( + ["restore", "--staged", "--", "f.txt"], + arguments[^4..], + "--end-of-options arrived in git 2.24, one release after restore, so the very versions this builder's reset fallback serves would read it as a pathspec and fail."); + Assert.IsFalse( + arguments.Contains("--end-of-options"), + "--end-of-options arrived in git 2.24, one release after restore, so the very versions this builder's reset fallback serves would read it as a pathspec and fail."); + } + [TestMethod] public void RefusesANullPath() { @@ -47,7 +64,7 @@ public async Task FallsBackToResetOnAGitOlderThanRestoreAsync() _ = await builder.ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); string[] arguments = [.. runner.Invocations[1]]; - Assert.AreSequenceEqual(["reset", "HEAD", "--end-of-options", "f.txt"], arguments[^4..]); + Assert.AreSequenceEqual(["reset", "HEAD", "--", "f.txt"], arguments[^4..]); } [TestMethod] @@ -63,7 +80,7 @@ public async Task TreatsExactlyTwoTwentyThreeAsSupportedAsync() _ = await builder.ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); string[] arguments = [.. runner.Invocations[1]]; - Assert.AreSequenceEqual(["restore", "--staged", "--end-of-options", "f.txt"], arguments[^4..]); + Assert.AreSequenceEqual(["restore", "--staged", "--", "f.txt"], arguments[^4..]); } public TestContext TestContext { get; set; } = null!; diff --git a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs index 961c0b2..87fa9ad 100644 --- a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs @@ -2,7 +2,9 @@ namespace ktsu.GitIntegration.Test; +using System; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Threading.Tasks; @@ -101,9 +103,14 @@ await IntegrationGitFixture.ConfigureIdentityAsync( Assert.IsFalse(result.Success, "A caller's refusal depends on this reporting failure rather than throwing."); - IReadOnlyList staged = await opened.Diff().Staged().ExecuteAsync().ConfigureAwait(false); + string workingTree = await File.ReadAllTextAsync( + Path.Combine(repository.RootPath, "f.txt"), + TestContext.CancellationTokenSource.Token).ConfigureAwait(false); - Assert.AreEqual(0, staged.Count, "--check must change nothing."); + Assert.AreEqual( + "something else entirely\n", + workingTree, + "--check must change nothing: the working tree is what it reads, so it is what --check could have disturbed."); } [TestMethod] @@ -153,5 +160,126 @@ await IntegrationGitFixture.ConfigureIdentityAsync( "--check must change nothing: the index should still hold what was staged, not the patch's content."); } + [TestMethod] + public async Task CarriesCarriageReturnsThroughTheRealRunnerAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + + GitRepository seeded = await SeedAsync( + client, + repository, + [("core.autocrlf", "false"), ("core.eol", "lf")]).ConfigureAwait(false); + + repository.WriteFile("f.txt", "one\r\ntwo\r\nthree\r\n"); + await CommitAllAsync(seeded).ConfigureAwait(false); + + repository.WriteFile("f.txt", "one\r\nTWO\r\nthree\r\n"); + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + GitFilePatch file = (await opened.Patch().ExecuteAsync().ConfigureAwait(false)).Files.Single(); + + StringAssert.Contains( + file.Hunks.Single().Text, + "\r", + StringComparison.Ordinal, + "The fixture tier proves the parser keeps carriage returns. Only the real runner proves the process boundary does."); + + _ = await opened.Apply(file.PatchFor(file.Hunks)).ToIndex().ExecuteAsync().ConfigureAwait(false); + + IReadOnlyList unstaged = await opened.Diff().ExecuteAsync().ConfigureAwait(false); + + Assert.AreEqual( + 0, + unstaged.Count, + "The index now matches the working tree byte for byte. A carriage return lost anywhere between reading the patch and staging it would leave a difference here."); + } + + [TestMethod] + public async Task RoundTripsUnderHostileDiffConfigurationAsync() + { + await IntegrationGitFixture.RequireGitAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + using TemporaryRepository repository = new(); + GitClient client = IntegrationGitFixture.CreateClient(); + + // Each of these three rewrites the diff into a shape a reader that trusts git's defaults + // cannot handle: two change the a/ and b/ path prefixes the header is found by, and the + // third prints an empty context line as a bare newline that ends the hunk body early. + GitRepository seeded = await SeedAsync( + client, + repository, + [ + ("diff.noprefix", "true"), + ("diff.mnemonicPrefix", "true"), + ("diff.suppressBlankEmpty", "true"), + ]).ConfigureAwait(false); + + repository.WriteFile("f.txt", "one\n\ntwo\nthree\nfour\n"); + await CommitAllAsync(seeded).ConfigureAwait(false); + + repository.WriteFile("f.txt", "one\n\ntwo\nthree\nFOUR\n"); + + GitRepository opened = await client.OpenAsync(repository.Root).ConfigureAwait(false); + GitFilePatch file = (await opened.Patch().ExecuteAsync().ConfigureAwait(false)).Files.Single(); + + Assert.AreEqual("f.txt", file.Path.WeakString); + StringAssert.Contains( + file.Hunks.Single().Text, + "FOUR", + StringComparison.Ordinal, + "A hunk truncated at the blank context line would stop before the change itself."); + + _ = await opened.Apply(file.PatchFor(file.Hunks)).ToIndex().ExecuteAsync().ConfigureAwait(false); + + IReadOnlyList unstaged = await opened.Diff().ExecuteAsync().ConfigureAwait(false); + + Assert.AreEqual( + 0, + unstaged.Count, + "The patch read under this configuration has to apply back cleanly, or the verbs work only for users whose git is configured the way the tests assume."); + } + + /// + /// Creates an empty repository with the fixture identity and any extra configuration a test + /// needs, before anything is committed. + /// + /// The client to initialize through. + /// The throwaway directory to initialize in. + /// Extra config keys and values to pin locally. + /// The initialized repository. + private async Task SeedAsync( + GitClient client, + TemporaryRepository repository, + IEnumerable<(string Key, string Value)> configuration) + { + GitInitResult init = await client.Init(repository.Root) + .WithInitialBranch("main".As()) + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + await IntegrationGitFixture.ConfigureIdentityAsync( + init.Repository, AuthorName, AuthorEmail, TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + + foreach ((string key, string value) in configuration) + { + _ = await new GitTextBuilder(init.Repository.ProcessRunner!, init.Repository.LocalPath, "config", key, value) + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + } + + return init.Repository; + } + + /// Stages everything in the working tree and commits it. + /// The repository to commit in. + private async Task CommitAllAsync(GitRepository repository) + { + _ = await repository.Add().All() + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + _ = await repository.Commit("seed".As()) + .ExecuteAsync(TestContext.CancellationTokenSource.Token).ConfigureAwait(false); + } + public TestContext TestContext { get; set; } = null!; } diff --git a/GitIntegration.Test/Models/GitPatchTests.cs b/GitIntegration.Test/Models/GitPatchTests.cs index 48f0669..8b11c81 100644 --- a/GitIntegration.Test/Models/GitPatchTests.cs +++ b/GitIntegration.Test/Models/GitPatchTests.cs @@ -75,6 +75,28 @@ public void PatchForNoHunksRefuses() => public void PatchForNullRefuses() => Assert.ThrowsExactly(() => _ = FileWithTwoHunks.PatchFor(null!)); + [TestMethod] + public void PatchForAHunkFromAnotherFileRefuses() + { + GitHunk foreign = HunkOne with { Text = "@@ -1,3 +1,3 @@\n x\n-y\n+Y\n z\n" }; + + ArgumentException thrown = Assert.ThrowsExactly( + () => _ = FileWithTwoHunks.PatchFor([foreign]), + "Filtering silently would return a header with no body, which reaches git as the corrupt-patch error the empty check exists to prevent."); + + StringAssert.Contains(thrown.Message, "f.txt", StringComparison.Ordinal); + } + + [TestMethod] + public void PatchForAMixOfOwnAndForeignHunksRefuses() + { + GitHunk foreign = HunkOne with { Text = "@@ -1,3 +1,3 @@\n x\n-y\n+Y\n z\n" }; + + _ = Assert.ThrowsExactly( + () => _ = FileWithTwoHunks.PatchFor([HunkOne, foreign]), + "A multi-file selection that staged some hunks and dropped the rest without a word is the worse half of this failure."); + } + [TestMethod] public void PatchPreservesAHunkVerbatimIncludingTheNoNewlineMarker() { diff --git a/GitIntegration/Builders/GitApplyBuilder.cs b/GitIntegration/Builders/GitApplyBuilder.cs index cfce755..319b424 100644 --- a/GitIntegration/Builders/GitApplyBuilder.cs +++ b/GitIntegration/Builders/GitApplyBuilder.cs @@ -34,7 +34,7 @@ public interface IGitApplyBuilder : IGitCommandBuilder /// /// Git reads a patch from standard input or from a file, and carries /// no standard input, so the patch text goes to a temporary file whose path is computed once, in the -/// constructor. AppendVerbArguments only names that path as an operand; nothing is written +/// constructor. AppendVerbArguments only names that path as an operand. Nothing is written /// to disk until or actually runs git, and /// the file is always removed afterwards, in a finally, whether git succeeded or failed. /// @@ -108,30 +108,30 @@ protected override GitCompleted ParseResult(GitProcessResult result) => /// public override async Task ExecuteAsync(CancellationToken cancellationToken = default) { - WritePatchFile(); - try { + WritePatchFile(); + return await base.ExecuteAsync(cancellationToken).ConfigureAwait(false); } finally { - File.Delete(_temporaryPath); + DeletePatchFile(); } } /// public override async Task> TryExecuteAsync(CancellationToken cancellationToken = default) { - WritePatchFile(); - try { + WritePatchFile(); + return await base.TryExecuteAsync(cancellationToken).ConfigureAwait(false); } finally { - File.Delete(_temporaryPath); + DeletePatchFile(); } } @@ -141,8 +141,31 @@ public override async Task> TryExecuteAsync(Cancellation /// /// /// A rewritten line ending, or a byte order mark git reads as part of the first line, makes the - /// patch unapplyable. + /// patch unapplyable. Called inside the try whose finally deletes the file, so a + /// write that fails partway through still gets cleaned up. /// private void WritePatchFile() => File.WriteAllText(_temporaryPath, _patchText, new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)); + + /// + /// Removes the temporary patch file, and says nothing when it cannot. + /// + /// + /// A delete that throws from inside a finally replaces whatever git reported with an I/O + /// error about a file the caller never knew existed. Git has already applied or refused the + /// patch by this point, so its own outcome is the one the caller needs, and a file left in the + /// temporary directory is the smaller cost. Deleting a path that was never written is already a + /// no-op. + /// + private void DeletePatchFile() + { + try + { + File.Delete(_temporaryPath); + } + catch (IOException) + { + // Nothing useful can be done here, and the caller is owed git's result rather than this. + } + } } diff --git a/GitIntegration/Builders/GitRestoreBuilder.cs b/GitIntegration/Builders/GitRestoreBuilder.cs index 510e70d..90218ce 100644 --- a/GitIntegration/Builders/GitRestoreBuilder.cs +++ b/GitIntegration/Builders/GitRestoreBuilder.cs @@ -52,7 +52,21 @@ internal sealed class GitRestoreBuilder(IGitProcessRunner runner, AbsoluteDirect /// private bool RestoreSupportedByVersion { get; set; } = true; - /// + /// + /// Appends the verb and the path, separating the two with a bare -- rather than through + /// AppendOperands. + /// + /// + /// The one builder in this library that does not use AppendOperands, and deliberately so. + /// AppendOperands writes --end-of-options, which git gained in 2.24, one release + /// after restore itself. This builder exists to serve a git older than 2.23, and on those + /// versions --end-of-options is not an option at all: git reads it as a pathspec and the + /// command fails, which would make the fallback unreachable and break the restore path on 2.23 + /// exactly. A bare -- has separated options from pathspecs for git's whole history and is + /// what both restore and reset want here, so it gives the same protection against + /// a dash-leading path on every version this builder can run against. + /// + /// The vector being assembled. protected override void AppendVerbArguments(ICollection arguments) { Ensure.NotNull(arguments); @@ -68,7 +82,8 @@ protected override void AppendVerbArguments(ICollection arguments) arguments.Add("HEAD"); } - AppendOperands(arguments, _path.WeakString); + arguments.Add("--"); + arguments.Add(_path.WeakString); } /// diff --git a/GitIntegration/GitRepository.cs b/GitIntegration/GitRepository.cs index 55c26b0..53b60ee 100644 --- a/GitIntegration/GitRepository.cs +++ b/GitIntegration/GitRepository.cs @@ -16,6 +16,12 @@ namespace ktsu.GitIntegration; /// public class GitRepository { + /// + /// What a byte sequence that is not valid UTF-8 decodes to, and so the marker that a patch has + /// already lost bytes before ever sees it. + /// + private const string ReplacementCharacter = "\uFFFD"; + /// /// Gets the local filesystem path where the repository is, or is intended to be, cloned, or /// when it is not known. @@ -121,11 +127,12 @@ private static Task IsClonedCoreAsync(IGitProcessRunner runner, AbsoluteDi /// Reads a patch, with the hunks and lines a caller needs to show or stage a change. /// The builder. + /// This repository has no . public IGitPatchBuilder Patch() => new GitPatchBuilder(RequireRunner(), RequireLocalPath()); /// /// Puts a patch into the index or the working tree. Staging one hunk of a file's patch is - /// Apply(text).ToIndex(); unstaging one already staged is the same call with + /// Apply(text).ToIndex(), and unstaging one already staged is the same call with /// Reversed() added. /// /// @@ -134,8 +141,9 @@ private static Task IsClonedCoreAsync(IGitProcessRunner runner, AbsoluteDi /// /// A fresh builder. /// - /// is null, empty, or all whitespace. Git reports an empty patch as - /// a corrupt-patch error that explains nothing, so this is caught before the process is started. + /// is null, empty, or all whitespace, or it carries the Unicode + /// replacement character. Git reports an empty patch as a corrupt-patch error that explains + /// nothing, so this is caught before the process is started. /// /// This repository has no . public IGitApplyBuilder Apply(string patchText) @@ -149,14 +157,39 @@ public IGitApplyBuilder Apply(string patchText) nameof(patchText)); } + // Git's output is read as UTF-8, so bytes that are not valid UTF-8 arrive as U+FFFD and + // would be written back as its encoding. With ASCII context around them git accepts the + // patch and stages the replacement characters while the working tree keeps the original + // bytes, which is silent corruption through a verb whose whole job is fidelity. + if (patchText.Contains(ReplacementCharacter, StringComparison.Ordinal)) + { + throw new ArgumentException( + "This patch carries the Unicode replacement character U+FFFD, which is what a byte " + + "git emitted that is not valid UTF-8 decodes to. Applying it would stage that " + + "character in place of the original bytes. A file that genuinely contains U+FFFD " + + "is refused here too, because nothing in the decoded text tells the two apart, and " + + "refusing a patch that would have worked is the lesser harm next to silently " + + "corrupting one that would not.", + nameof(patchText)); + } + return new GitApplyBuilder(RequireRunner(), RequireLocalPath(), patchText); } /// Removes a path's staged changes, leaving the working tree alone. /// The path, relative to the repository root. /// The builder. - public IGitRestoreBuilder Unstage(RelativeFilePath path) => - new GitRestoreBuilder(RequireRunner(), RequireLocalPath(), Ensure.NotNull(path)); + /// is . + /// This repository has no . + public IGitRestoreBuilder Unstage(RelativeFilePath path) + { + // Argument validation before RequireRunner(): left-to-right evaluation would otherwise + // report the missing runner for a null path on a metadata-only repository, which is the + // wrong diagnostic for what the caller got wrong. + Ensure.NotNull(path); + + return new GitRestoreBuilder(RequireRunner(), RequireLocalPath(), path); + } /// Resolves a revision to the object id it names. /// The revision to resolve. diff --git a/GitIntegration/Models/GitPatch.cs b/GitIntegration/Models/GitPatch.cs index c58f582..8e45513 100644 --- a/GitIntegration/Models/GitPatch.cs +++ b/GitIntegration/Models/GitPatch.cs @@ -68,6 +68,13 @@ public sealed record GitHunk /// Kept verbatim rather than regenerated from , because a detail such as the /// no-newline-at-end-of-file marker has no home in the parsed lines and would otherwise be lost, /// leaving git apply to reject the reassembled hunk. + /// + /// Verbatim covers the bytes git wrote, not the encoding it wrote them in. Git's output is + /// decoded as UTF-8, so a repository whose files hold Latin-1 or Shift-JIS text, which git + /// diffs as text because it decides binary on NUL bytes rather than on encoding validity, loses + /// every invalid byte to U+FFFD here. refuses patch text + /// carrying that character rather than staging the replacement bytes. + /// /// public required string Text { get; init; } } @@ -81,18 +88,33 @@ public sealed record GitFilePatch public required RelativeFilePath Path { get; init; } /// - /// Gets the path this file came from for a rename or a copy, or - /// otherwise. + /// Gets the path this file came from for a rename, or otherwise. /// + /// + /// A copy is not reported here. Git writes copy from and copy to only under + /// --find-copies, which never asks for. + /// public RelativeFilePath? OriginalPath { get; init; } /// Gets what happened to the path. + /// + /// never appears, because git does not report a type + /// change as one file. A regular file becoming a symbolic link comes back as two entries for + /// the same path, one and one , + /// where diff --name-status reports a single T. + /// public required GitChangeKind Kind { get; init; } /// Gets whether git treated this file as binary rather than diffing its lines. + /// is empty when this is . public required bool IsBinary { get; init; } /// Gets whether this file has conflicting changes from an unfinished merge. + /// + /// is empty when this is . Git prints an unmerged path + /// in the combined format, which git apply does not accept, so its body is not parsed + /// into hunks that could not be staged anyway. + /// public required bool IsConflicted { get; init; } /// @@ -111,15 +133,18 @@ public sealed record GitFilePatch /// Hunks are emitted in the order this file holds them rather than the order they were given, /// because git reads a patch top to bottom and rejects one whose hunks run backwards. /// - /// The hunks are not checked for belonging to this file. The comparison would cost a pass per - /// call to catch a mistake no reasonable caller makes, and a hunk from elsewhere fails at apply - /// with git's own message. + /// A hunk this file does not hold is refused rather than dropped. Only the hunks in + /// can be written under this file's header, so a caller passing one from + /// another file would otherwise get a shorter patch than it asked for, or a header with no body + /// at all, and neither outcome says which hunk went missing. /// /// /// The hunks to include. /// The patch text. /// is . - /// is empty. + /// + /// is empty, or holds a hunk this file does not. + /// public string PatchFor(IEnumerable hunks) { Ensure.NotNull(hunks); @@ -135,13 +160,21 @@ public string PatchFor(IEnumerable hunks) } StringBuilder builder = new(Header); + int emitted = 0; foreach (GitHunk hunk in Hunks.Where(wanted.Contains)) { _ = builder.Append(hunk.Text); + emitted++; } - return builder.ToString(); + return emitted < wanted.Count + ? throw new ArgumentException( + $"{wanted.Count - emitted} of the {wanted.Count} hunks given do not belong to " + + $"'{Path.WeakString}'. Only this file's own hunks can be written under its header, " + + "so the rest would be dropped without a word.", + nameof(hunks)) + : builder.ToString(); } } From dbe56403ce4880666c31ee5b5d8a84557c822783 Mon Sep 17 00:00:00 2001 From: Matt Edmondson Date: Thu, 24 Sep 2026 22:30:25 +1000 Subject: [PATCH 13/13] [patch] Join path segments rather than combining them Path.Combine resets to a later segment when one is rooted, which is why TemporaryRepository already documents Path.Join as the API that means append these segments. The three call sites this branch added now follow that, and so do the two fixture loaders they were modeled on, which were identical and would otherwise have been left in a second shape. --- GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs | 2 +- GitIntegration.Test/Hosting/GitHubProviderTests.cs | 2 +- GitIntegration.Test/Integration/GitPatchRoundTripTests.cs | 2 +- GitIntegration.Test/Parsing/GitPatchParserTests.cs | 2 +- GitIntegration/Builders/GitApplyBuilder.cs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs b/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs index 7e416f9..3f36153 100644 --- a/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs +++ b/GitIntegration.Test/Hosting/AzureDevOpsProviderTests.cs @@ -41,7 +41,7 @@ public sealed class AzureDevOpsProviderTests /// never packed, and no request is ever issued against either. /// private static string Fixture(string name) => - File.ReadAllText(Path.Combine(AppContext.BaseDirectory, "Fixtures", name)); + File.ReadAllText(Path.Join(AppContext.BaseDirectory, "Fixtures", name)); /// /// Wraps the single captured create-response fixture in the pull-request-list envelope diff --git a/GitIntegration.Test/Hosting/GitHubProviderTests.cs b/GitIntegration.Test/Hosting/GitHubProviderTests.cs index 63a532c..6d8f97d 100644 --- a/GitIntegration.Test/Hosting/GitHubProviderTests.cs +++ b/GitIntegration.Test/Hosting/GitHubProviderTests.cs @@ -27,7 +27,7 @@ public sealed class GitHubProviderTests /// Reads a captured fixture's raw JSON text from the test output's Fixtures directory. private static string Fixture(string name) => - File.ReadAllText(Path.Combine(AppContext.BaseDirectory, "Fixtures", name)); + File.ReadAllText(Path.Join(AppContext.BaseDirectory, "Fixtures", name)); /// /// Wraps the single captured pull request fixture in a one-element array — the shape diff --git a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs index 87fa9ad..40bb9cc 100644 --- a/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitPatchRoundTripTests.cs @@ -104,7 +104,7 @@ await IntegrationGitFixture.ConfigureIdentityAsync( Assert.IsFalse(result.Success, "A caller's refusal depends on this reporting failure rather than throwing."); string workingTree = await File.ReadAllTextAsync( - Path.Combine(repository.RootPath, "f.txt"), + Path.Join(repository.RootPath, "f.txt"), TestContext.CancellationTokenSource.Token).ConfigureAwait(false); Assert.AreEqual( diff --git a/GitIntegration.Test/Parsing/GitPatchParserTests.cs b/GitIntegration.Test/Parsing/GitPatchParserTests.cs index 013e6de..eac6db7 100644 --- a/GitIntegration.Test/Parsing/GitPatchParserTests.cs +++ b/GitIntegration.Test/Parsing/GitPatchParserTests.cs @@ -16,7 +16,7 @@ public class GitPatchParserTests /// Reads a captured fixture's raw text from the test output's Fixtures directory. private static string Fixture(string name) => - File.ReadAllText(Path.Combine(AppContext.BaseDirectory, "Fixtures", name)); + File.ReadAllText(Path.Join(AppContext.BaseDirectory, "Fixtures", name)); [TestMethod] public void ParsesTwoHunksWithTheirLineNumbers() diff --git a/GitIntegration/Builders/GitApplyBuilder.cs b/GitIntegration/Builders/GitApplyBuilder.cs index 319b424..c2bcae8 100644 --- a/GitIntegration/Builders/GitApplyBuilder.cs +++ b/GitIntegration/Builders/GitApplyBuilder.cs @@ -45,7 +45,7 @@ internal sealed class GitApplyBuilder(IGitProcessRunner runner, AbsoluteDirector : GitCommandBuilder(runner, repositoryPath), IGitApplyBuilder { private readonly string _patchText = Ensure.NotNull(patchText); - private readonly string _temporaryPath = Path.Combine(Path.GetTempPath(), $"ktsu-git-apply-{Path.GetRandomFileName()}.patch"); + private readonly string _temporaryPath = Path.Join(Path.GetTempPath(), $"ktsu-git-apply-{Path.GetRandomFileName()}.patch"); private bool _toIndex; private bool _reversed;