diff --git a/CLAUDE.md b/CLAUDE.md index a56a1f2..4096dcd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -328,6 +328,17 @@ Non-obvious, load-bearing design points: positional, because a rename is spelled differently in each section. A binary file's counts are `-`, which is why they are nullable — a binary change is not a zero-line change. +14. **`Status()` emits `--untracked-files` whether or not the caller chose a mode.** Git resolves the + flag from `status.showUntrackedFiles` when a command omits it, and a host — a minimal CI image, a + developer's `~/.gitconfig` — can set that to `no` to keep `git status` fast in a large tree. The + same working copy would then report `IsClean == true` with untracked files sitting in it, which is + exactly the answer a caller deciding whether a directory is safe to discard must not get. So + `GitStatusBuilder.DefaultUntrackedFiles` (git's own documented default, `normal`) is pinned into + the argument vector alongside the `--no-pager`, `core.quotepath=false` and `color.ui=false` that + `GitCommandBuilder.BuildArguments` already pins — same reasoning, and this was the one verb still + leaving a host-configurable default unspecified. `WithUntrackedFiles(...)` replaces the default + rather than joining it, so the vector never carries two `--untracked-files` values. + **Hosting layer.** `GitProvider` is an abstract base with two implementations: `GitHubProvider` over Octokit, and `AzureDevOpsProvider` over a raw `HttpClient` — Azure DevOps has no client library this library uses (see the dependency note below). Both go through the same shape: every request-issuing diff --git a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs index 231d96c..828aa3e 100644 --- a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs @@ -28,6 +28,11 @@ public void BuildsTheDefaultStatusVector() "--porcelain=v2", "--branch", "-z", + + // Present without the caller asking for it: left off, the mode would come from the host's + // status.showUntrackedFiles, and a host that sets it to "no" would report a working copy + // with untracked files in it as clean. + "--untracked-files=normal", ]; CollectionAssert.AreEqual(expectedArguments, arguments.ToArray()); } @@ -50,6 +55,25 @@ public void MapsTheUntrackedFilesModeToItsOption() CollectionAssert.Contains(all.BuildArguments().ToArray(), "--untracked-files=all"); } + [TestMethod] + public void ChoosingAModeReplacesThePinnedDefaultRatherThanJoiningIt() + { + // The default is emitted unconditionally, so the risk it introduces is a vector carrying both + // --untracked-files=normal and the caller's choice. Git would honour the last one, which is the + // caller's, but only by accident of ordering — and a No caller who silently got a "normal" pass + // over a large tree first would pay for a mode they declined. + RecordingGitProcessRunner runner = new(); + GitStatusBuilder builder = new(runner, TestPaths.Root); + _ = builder.WithUntrackedFiles(GitUntrackedFilesMode.No); + + IReadOnlyList arguments = builder.BuildArguments(); + + string untracked = Assert.ContainsSingle( + argument => argument.StartsWith("--untracked-files=", StringComparison.Ordinal), + arguments); + Assert.AreEqual("--untracked-files=no", untracked); + } + [TestMethod] public void AddsTheIgnoredOptionOnlyWhenAsked() { diff --git a/GitIntegration.Test/Integration/GitRoundTripTests.cs b/GitIntegration.Test/Integration/GitRoundTripTests.cs index 9180e99..7df366a 100644 --- a/GitIntegration.Test/Integration/GitRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitRoundTripTests.cs @@ -153,6 +153,48 @@ public async Task StatusReflectsStagedAndUntrackedWorkAsync() Assert.IsFalse(dirty.IsClean); } + [TestMethod] + public async Task StatusReportsUntrackedWorkEvenWhereTheHostHidesItAsync() + { + // status.showUntrackedFiles is a host setting — a CI image or a developer's ~/.gitconfig can set + // it to "no" to keep git status fast in a large tree — and git applies it to any status command + // carrying no --untracked-files of its own. Written into the throwaway repository's own config + // rather than the runner's global one: it is the same variable resolved from the nearest scope, + // so the override is exercised without the test depending on, or disturbing, the host. + CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; + await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); + + using TemporaryRepository temporary = new(); + GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + + temporary.WriteFile("a.txt", "one\n"); + _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); + _ = await repository.Commit("c1".As()).ExecuteAsync(cancellationToken).ConfigureAwait(false); + + _ = await new GitTextBuilder( + repository.ProcessRunner!, repository.LocalPath, "config", "status.showUntrackedFiles", "no") + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + + temporary.WriteFile("untracked.txt", "two\n"); + + GitStatus status = await repository.Status().ExecuteAsync(cancellationToken).ConfigureAwait(false); + + // IsClean is asserted beside the entry because it is the property a caller reads before + // discarding a working copy — reporting true here would mean losing that file. + Assert.IsFalse(status.IsClean); + Assert.Contains( + entry => entry.Path.WeakString.EndsWith("untracked.txt", StringComparison.Ordinal), + status.Entries, + "Status() dropped the untracked file because the repository's status.showUntrackedFiles said to."); + + // A caller's own choice still wins, including in the direction the config happens to agree with: + // the pinned default replaces the host's setting rather than overriding the caller's. + GitStatus suppressed = await repository.Status() + .WithUntrackedFiles(GitUntrackedFilesMode.No) + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + Assert.IsTrue(suppressed.IsClean); + } + [TestMethod] public async Task BranchCreateCheckoutAndDeleteRoundTripAsync() { diff --git a/GitIntegration/Builders/GitStatusBuilder.cs b/GitIntegration/Builders/GitStatusBuilder.cs index 19c6b19..8de3b6a 100644 --- a/GitIntegration/Builders/GitStatusBuilder.cs +++ b/GitIntegration/Builders/GitStatusBuilder.cs @@ -16,6 +16,11 @@ public interface IGitStatusBuilder : IGitCommandBuilder /// /// Sets how much untracked detail git should report. /// + /// + /// Without this call the command reports untracked files the way + /// describes, whatever the host's + /// status.showUntrackedFiles is set to. + /// /// The reporting mode. /// The same builder, to allow chaining. /// is not a recognised value. @@ -29,14 +34,23 @@ public interface IGitStatusBuilder : IGitCommandBuilder } /// -/// Builds git status --porcelain=v2 --branch -z. +/// Builds git status --porcelain=v2 --branch -z --untracked-files=normal. /// /// Runs the assembled command. /// The repository to scope the command to. internal sealed class GitStatusBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath) : GitCommandBuilder(runner, repositoryPath), IGitStatusBuilder { - private GitUntrackedFilesMode? _untrackedFiles; + /// + /// The untracked reporting mode emitted when the caller never calls . + /// + /// + /// Git's own documented default for status.showUntrackedFiles, so pinning it changes nothing + /// on a host that has not set the variable and everything on a host that has. + /// + internal const GitUntrackedFilesMode DefaultUntrackedFiles = GitUntrackedFilesMode.Normal; + + private GitUntrackedFilesMode _untrackedFiles = DefaultUntrackedFiles; private bool _includeIgnored; /// @@ -76,10 +90,13 @@ protected override void AppendVerbArguments(ICollection arguments) arguments.Add("--branch"); arguments.Add("-z"); - if (_untrackedFiles is GitUntrackedFilesMode mode) - { - arguments.Add("--untracked-files=" + ToOptionValue(mode)); - } + // Emitted whether or not the caller chose a mode. Omitting the flag hands the decision to + // status.showUntrackedFiles, which a CI image or a developer's ~/.gitconfig can set to "no" to + // speed up status in a large tree — and then the same working copy reports IsClean == true with + // an untracked file sitting in it. A caller asking "is there work here I would destroy?" + // deserves the same answer on every host, so the default is pinned in the argument vector + // beside --no-pager, core.quotepath and color.ui rather than left to the host. + arguments.Add("--untracked-files=" + ToOptionValue(_untrackedFiles)); if (_includeIgnored) {