From 7549beb46ba843b865a89dc1f30261528d653eb7 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 09:25:44 +0000 Subject: [PATCH 1/2] Pin --ignore-submodules=none in Status, Diff and Patch [patch] A host's diff.ignoreSubmodules=all made Status() report clean, and Diff()/Patch() report nothing, while a submodule held unpushed commits and untracked files. Pinning none reports the submodule however the host is configured, the same way --untracked-files is already pinned. Fixes ktsu-dev/GitIntegration#190 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016vf9SXXkfFir6zugMSNM3b --- CLAUDE.md | 3 ++ .../Builders/GitDiffBuilderTests.cs | 3 ++ .../Builders/GitPatchBuilderTests.cs | 1 + .../Builders/GitStatusBuilderTests.cs | 4 ++ .../Integration/GitSubmoduleTests.cs | 46 +++++++++++++++++++ GitIntegration/Builders/GitDiffBuilder.cs | 5 ++ GitIntegration/Builders/GitPatchBuilder.cs | 4 ++ GitIntegration/Builders/GitStatusBuilder.cs | 6 +++ 8 files changed, 72 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 0736bd4..26bedd4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -374,6 +374,9 @@ Non-obvious, load-bearing design points: `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. + `Status()`, `Diff()` and `Patch()` likewise pin `--ignore-submodules=none`: a host's + `diff.ignoreSubmodules=all` would otherwise hide a submodule's new commits and untracked files, + and `Status()` would report clean over unpushed submodule work (#190). 15. **`Log()` passes `--no-show-signature`.** With `log.showSignature=true`, which is common among people who sign commits, git prints each signed commit's verification to standard output ahead diff --git a/GitIntegration.Test/Builders/GitDiffBuilderTests.cs b/GitIntegration.Test/Builders/GitDiffBuilderTests.cs index fb2c1c6..256e584 100644 --- a/GitIntegration.Test/Builders/GitDiffBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitDiffBuilderTests.cs @@ -29,6 +29,9 @@ public void BuildsTheDefaultDiffVector() "diff", "--name-status", "-z", + + // Pinned so a host's diff.ignoreSubmodules cannot drop a changed submodule. + "--ignore-submodules=none", "--no-renames", ]; CollectionAssert.AreEqual(expectedArguments, arguments.ToArray()); diff --git a/GitIntegration.Test/Builders/GitPatchBuilderTests.cs b/GitIntegration.Test/Builders/GitPatchBuilderTests.cs index a5d534c..117f82f 100644 --- a/GitIntegration.Test/Builders/GitPatchBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitPatchBuilderTests.cs @@ -31,6 +31,7 @@ public void BuildsTheDefaultPatchVector() "--src-prefix=a/", "--dst-prefix=b/", "--submodule=short", + "--ignore-submodules=none", "-U3", "--no-renames", ]; diff --git a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs index c64e9f0..5d1c08f 100644 --- a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs @@ -33,6 +33,10 @@ public void BuildsTheDefaultStatusVector() // 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", + + // The same for submodules: a host's diff.ignoreSubmodules=all would hide a submodule's + // unpushed commits and untracked files. + "--ignore-submodules=none", ]; CollectionAssert.AreEqual(expectedArguments, arguments.ToArray()); } diff --git a/GitIntegration.Test/Integration/GitSubmoduleTests.cs b/GitIntegration.Test/Integration/GitSubmoduleTests.cs index 135bf9d..c4df282 100644 --- a/GitIntegration.Test/Integration/GitSubmoduleTests.cs +++ b/GitIntegration.Test/Integration/GitSubmoduleTests.cs @@ -416,6 +416,52 @@ private static async Task RecordGitlinkAsync( .ExecuteAsync(cancellationToken).ConfigureAwait(false); } + [TestMethod] + public async Task StatusDiffAndPatchReportASubmoduleTheHostIgnoresAsync() + { + // diff.ignoreSubmodules is a host setting, and git applies it to status, diff and a patch + // alike when the command carries no --ignore-submodules of its own. Written into the + // superproject's own config, the nearest scope, so the host is left alone. + CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; + await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); + + using TemporaryRepository subDirectory = new(); + using TemporaryRepository superDirectory = new(); + + GitRepository sub = await CreateRepositoryAsync(subDirectory, "s.txt", cancellationToken).ConfigureAwait(false); + GitRepository super = await CreateRepositoryAsync(superDirectory, "m.txt", cancellationToken).ConfigureAwait(false); + + await AddSubmoduleAsync(super, sub.LocalPath!, "libs/sub", cancellationToken).ConfigureAwait(false); + + _ = await new GitTextBuilder( + super.ProcessRunner!, super.LocalPath, "config", "diff.ignoreSubmodules", "all") + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + + // Unpushed work in the submodule: a new commit, and an untracked file beside it. + GitRepository checkout = new() + { + LocalPath = Path.Join(superDirectory.RootPath, "libs", "sub").As(), + ProcessRunner = super.ProcessRunner, + }; + await IntegrationGitFixture.ConfigureIdentityAsync(checkout, AuthorName, AuthorEmail, cancellationToken) + .ConfigureAwait(false); + _ = await CommitInSubmoduleAsync(checkout, superDirectory, Main, "two\n", cancellationToken).ConfigureAwait(false); + superDirectory.WriteFile("libs/sub/untracked.txt", "three\n"); + + GitStatus status = await super.Status().ExecuteAsync(cancellationToken).ConfigureAwait(false); + + // IsClean is the property a caller reads before discarding a working copy, and true here + // would mean losing the submodule's commit. + Assert.IsFalse(status.IsClean, "Status() reported clean because the host ignores submodules."); + Assert.Contains(entry => entry.Path.WeakString == "libs/sub", status.Entries); + + IReadOnlyList diff = await super.Diff().ExecuteAsync(cancellationToken).ConfigureAwait(false); + Assert.Contains(entry => entry.Path.WeakString == "libs/sub", diff, "Diff() dropped the submodule."); + + GitPatch patch = await super.Patch().ExecuteAsync(cancellationToken).ConfigureAwait(false); + Assert.Contains(file => file.Path.WeakString == "libs/sub", patch.Files, "Patch() dropped the submodule."); + } + [TestMethod] public async Task ReportsNoSubmodulesForARepositoryWithNoneAsync() { diff --git a/GitIntegration/Builders/GitDiffBuilder.cs b/GitIntegration/Builders/GitDiffBuilder.cs index 2ddfcd5..0ab9bbd 100644 --- a/GitIntegration/Builders/GitDiffBuilder.cs +++ b/GitIntegration/Builders/GitDiffBuilder.cs @@ -194,6 +194,11 @@ protected override void AppendVerbArguments(ICollection arguments) arguments.Add("-z"); + // diff.ignoreSubmodules=all in a host's config would drop a moved or dirty submodule from the + // list entirely, so the same working copy reports different changes on different machines + // (ktsu-dev/GitIntegration#190). + arguments.Add("--ignore-submodules=none"); + if (_staged) { arguments.Add("--cached"); diff --git a/GitIntegration/Builders/GitPatchBuilder.cs b/GitIntegration/Builders/GitPatchBuilder.cs index 591f62a..9d95856 100644 --- a/GitIntegration/Builders/GitPatchBuilder.cs +++ b/GitIntegration/Builders/GitPatchBuilder.cs @@ -206,6 +206,10 @@ protected override void AppendVerbArguments(ICollection arguments) // Only the short form is a gitlink patch that stages (ktsu-dev/GitIntegration#124). arguments.Add("--submodule=short"); + // And diff.ignoreSubmodules=all would leave the gitlink out of the patch altogether, so the + // submodule's change could never be staged from it (ktsu-dev/GitIntegration#190). + arguments.Add("--ignore-submodules=none"); + if (_staged) { arguments.Add("--cached"); diff --git a/GitIntegration/Builders/GitStatusBuilder.cs b/GitIntegration/Builders/GitStatusBuilder.cs index 7faae4b..17cb456 100644 --- a/GitIntegration/Builders/GitStatusBuilder.cs +++ b/GitIntegration/Builders/GitStatusBuilder.cs @@ -98,6 +98,12 @@ protected override void AppendVerbArguments(ICollection arguments) // beside --no-pager, core.quotepath and color.ui rather than left to the host. arguments.Add("--untracked-files=" + ToOptionValue(_untrackedFiles)); + // Same reasoning, for submodules. diff.ignoreSubmodules=all in a host's config (status reads + // it too) hides a submodule's new commits and untracked files, so the superproject reports + // IsClean == true over unpushed work. Pinning "none" reports the submodule however the host + // is configured (ktsu-dev/GitIntegration#190). + arguments.Add("--ignore-submodules=none"); + if (_includeIgnored) { // "matching" rather than "traditional": it lists the ignored paths themselves instead of From 50839512ea85808415e9157ad3170a9f1ae8bfc9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 09:30:39 +0000 Subject: [PATCH 2/2] Compare the submodule's path as a typed path so the test holds on Windows [patch] Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016vf9SXXkfFir6zugMSNM3b --- GitIntegration.Test/Integration/GitSubmoduleTests.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/GitIntegration.Test/Integration/GitSubmoduleTests.cs b/GitIntegration.Test/Integration/GitSubmoduleTests.cs index c4df282..5d4aa89 100644 --- a/GitIntegration.Test/Integration/GitSubmoduleTests.cs +++ b/GitIntegration.Test/Integration/GitSubmoduleTests.cs @@ -448,18 +448,22 @@ await IntegrationGitFixture.ConfigureIdentityAsync(checkout, AuthorName, AuthorE _ = await CommitInSubmoduleAsync(checkout, superDirectory, Main, "two\n", cancellationToken).ConfigureAwait(false); superDirectory.WriteFile("libs/sub/untracked.txt", "three\n"); + // Compared as a typed path rather than as text, since the path type spells separators + // natively and the text differs on Windows. + RelativeFilePath submodulePath = "libs/sub".As(); + GitStatus status = await super.Status().ExecuteAsync(cancellationToken).ConfigureAwait(false); // IsClean is the property a caller reads before discarding a working copy, and true here // would mean losing the submodule's commit. Assert.IsFalse(status.IsClean, "Status() reported clean because the host ignores submodules."); - Assert.Contains(entry => entry.Path.WeakString == "libs/sub", status.Entries); + Assert.Contains(entry => entry.Path == submodulePath, status.Entries); IReadOnlyList diff = await super.Diff().ExecuteAsync(cancellationToken).ConfigureAwait(false); - Assert.Contains(entry => entry.Path.WeakString == "libs/sub", diff, "Diff() dropped the submodule."); + Assert.Contains(entry => entry.Path == submodulePath, diff, "Diff() dropped the submodule."); GitPatch patch = await super.Patch().ExecuteAsync(cancellationToken).ConfigureAwait(false); - Assert.Contains(file => file.Path.WeakString == "libs/sub", patch.Files, "Patch() dropped the submodule."); + Assert.Contains(file => file.Path == submodulePath, patch.Files, "Patch() dropped the submodule."); } [TestMethod]