diff --git a/CLAUDE.md b/CLAUDE.md index c9bad2c..b29c3ba 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,7 @@ dotnet publish --configuration Release --output ./staging Tests live in `ProjectDirector.Test` (MSTest, via `MSTest.Sdk` + `ktsu.Sdk`). The app exposes its internals to the test project through `InternalsVisibleTo` in `ProjectDirector/AssemblyInfo.cs`. `GitCliTests` drives `GitCli` against throwaway repositories under the temp directory; the ImGui layer is not unit-tested. -That last point is why the repository actions are shaped the way they are: the part of each with a rule in it is pulled out into a plain method so it can be driven without a live ImGui context or a display. `ProjectDirector.DecidePull` decides whether pulling interrupts the user first (`PullDecisionTests`), and `GitCli.ListPendingChanges` plus `ProjectDirector.DescribePendingChanges` decide what a commit will sweep up and how that is shown (`CommitTests`). `DiffTake` builds the file a take arrow in the diff view produces and writes it back with the destination's own line ending and BOM (`DiffTakeTests`). Anything genuinely worth testing that is still tangled up with drawing is usually worth extracting the same way. +That last point is why the repository actions are shaped the way they are: the part of each with a rule in it is pulled out into a plain method so it can be driven without a live ImGui context or a display. `ProjectDirector.DecidePull` decides whether pulling interrupts the user first (`PullDecisionTests`), and `GitCli.ListPendingChanges` plus `ProjectDirector.DescribePendingChanges` decide what a commit will sweep up and how that is shown (`CommitTests`). `RepoBrowsing` holds the repo and compare browsers' path handling: every entry is relative to the repository root and already includes the browse path, and folders are recognised on disk rather than by a trailing separator (`RepoBrowsingTests`). `DiffTake` builds the file a take arrow in the diff view produces and writes it back with the destination's own line ending and BOM (`DiffTakeTests`). Anything genuinely worth testing that is still tangled up with drawing is usually worth extracting the same way. ```powershell dotnet test --configuration Release diff --git a/ProjectDirector.Test/RepoBrowsingTests.cs b/ProjectDirector.Test/RepoBrowsingTests.cs new file mode 100644 index 0000000..91b00c3 --- /dev/null +++ b/ProjectDirector.Test/RepoBrowsingTests.cs @@ -0,0 +1,166 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector.Test; + +using System; +using System.Collections.ObjectModel; +using System.IO; +using System.Linq; + +using ktsu.Semantics.Paths; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests the path handling behind the repo and compare browsers below the top level. +/// +/// +/// Entries used to be combined with the browse path a second time, so opening src looked for +/// src/src/Inner: folders showed as files, navigation stopped at the first level, Give on a +/// file threw on the render thread, and Give on a folder created a stray src/src/... in the +/// other repository. Folders were also recognised by a trailing separator that +/// strips, so the compare browser called +/// and on folders. +/// +[TestClass] +public sealed class RepoBrowsingTests +{ + private string _workspace = string.Empty; + private string _repoA = string.Empty; + private string _repoB = string.Empty; + + [TestInitialize] + public void CreateRepos() + { + _workspace = Path.Join(Path.GetTempPath(), $"ktsu_pd_browse_{Guid.NewGuid():N}"); + _repoA = Path.Join(_workspace, "A"); + _repoB = Path.Join(_workspace, "B"); + + _ = Directory.CreateDirectory(Path.Join(_repoA, "src", "Inner")); + File.WriteAllText(Path.Join(_repoA, "src", "a.cs"), "class A { }\n"); + _ = Directory.CreateDirectory(Path.Join(_repoA, "only")); + _ = Directory.CreateDirectory(Path.Join(_repoB, "src")); + } + + [TestCleanup] + public void DeleteRepos() + { + try + { + Directory.Delete(_workspace, recursive: true); + } + catch (IOException) + { + // A leaked temp directory is not worth failing an otherwise passing test over. + } + } + + private static RelativePath Entry(string path) => RelativePath.Create(path); + + [TestMethod] + public void List_ANestedFolder_ReturnsEntriesRelativeToTheRepoRoot() + { + Collection entries = RepoBrowsing.List(_repoA, "src"); + + CollectionAssert.AreEquivalent( + new[] { Entry(Path.Join("src", "Inner")), Entry(Path.Join("src", "a.cs")) }, + entries.ToArray()); + } + + [TestMethod] + public void List_AFolderTheRepoDoesNotHave_ReturnsNothing() + { + Assert.IsEmpty(RepoBrowsing.List(_repoB, "only")); + } + + [TestMethod] + public void IsDirectory_ANestedFolder_IsAFolder() + { + Collection entries = RepoBrowsing.List(_repoA, "src"); + + RelativePath[] directories = [.. entries.Where(entry => RepoBrowsing.IsDirectory(entry, _repoA, _repoB))]; + + CollectionAssert.AreEqual(new[] { Entry(Path.Join("src", "Inner")) }, directories); + } + + [TestMethod] + public void IsDirectory_AFolderInOnlyOneRepo_IsAFolder() + { + Assert.IsTrue(RepoBrowsing.IsDirectory(Entry("only"), _repoB, _repoA)); + } + + [TestMethod] + public void Navigating_IntoANestedFolder_ListsItsContents() + { + File.WriteAllText(Path.Join(_repoA, "src", "Inner", "b.cs"), "class B { }\n"); + RelativePath inner = Entry(Path.Join("src", "Inner")); + + Collection entries = RepoBrowsing.List(_repoA, inner.WeakString); + + CollectionAssert.AreEqual(new[] { Entry(Path.Join("src", "Inner", "b.cs")) }, entries.ToArray()); + } + + [TestMethod] + public void Copy_ANestedFile_LandsAtTheSamePathInTheOtherRepo() + { + string? failure = RepoBrowsing.Copy(Entry(Path.Join("src", "a.cs")), _repoA, _repoB); + + Assert.IsNull(failure); + Assert.IsTrue(File.Exists(Path.Join(_repoB, "src", "a.cs"))); + Assert.IsFalse(Directory.Exists(Path.Join(_repoB, "src", "src"))); + } + + [TestMethod] + public void Copy_ANestedFolder_CreatesItWithoutAStrayParent() + { + string? failure = RepoBrowsing.Copy(Entry(Path.Join("src", "Inner")), _repoA, _repoB); + + Assert.IsNull(failure); + Assert.IsTrue(Directory.Exists(Path.Join(_repoB, "src", "Inner"))); + Assert.IsFalse(Directory.Exists(Path.Join(_repoB, "src", "src"))); + } + + [TestMethod] + public void Copy_AFolderInOnlyOneRepo_CreatesTheFolderRatherThanThrowing() + { + string? failure = RepoBrowsing.Copy(Entry("only"), _repoA, _repoB); + + Assert.IsNull(failure); + Assert.IsTrue(Directory.Exists(Path.Join(_repoB, "only"))); + } + + [TestMethod] + public void Copy_ASourceThatIsGone_ReportsTheFailureRatherThanThrowing() + { + string? failure = RepoBrowsing.Copy(Entry(Path.Join("src", "missing.cs")), _repoA, _repoB); + + Assert.IsNotNull(failure); + } + + [TestMethod] + public void Delete_AFolder_RemovesItRatherThanThrowing() + { + string? failure = RepoBrowsing.Delete(Entry("only"), _repoA); + + Assert.IsNull(failure); + Assert.IsFalse(Directory.Exists(Path.Join(_repoA, "only"))); + } + + [TestMethod] + public void Delete_ANestedFile_RemovesOnlyThatFile() + { + string? failure = RepoBrowsing.Delete(Entry(Path.Join("src", "a.cs")), _repoA); + + Assert.IsNull(failure); + Assert.IsFalse(File.Exists(Path.Join(_repoA, "src", "a.cs"))); + Assert.IsTrue(Directory.Exists(Path.Join(_repoA, "src", "Inner"))); + } + + [TestMethod] + public void Delete_AFolderWithContents_ReportsTheFailureAndKeepsIt() + { + string? failure = RepoBrowsing.Delete(Entry("src"), _repoA); + + Assert.IsNotNull(failure); + Assert.IsTrue(File.Exists(Path.Join(_repoA, "src", "a.cs"))); + } +} diff --git a/ProjectDirector/ProjectDirector.cs b/ProjectDirector/ProjectDirector.cs index 1e4ab07..5070bd4 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -1976,7 +1976,9 @@ private static void ShowDiffSummaryText(int linesDeleted, int linesAdded) private void ShowCompareBrowser() { IEnumerable allFilesystemEntries = BrowserContentsBase.Union(BrowserContentsCompare); - Collection directories = allFilesystemEntries.Where(x => x.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal)).ToCollection(); + string baseRoot = Options.Repos[Options.BaseRepo].LocalPath; + string compareRoot = Options.Repos[Options.CompareRepo].LocalPath; + Collection directories = allFilesystemEntries.Where(x => RepoBrowsing.IsDirectory(x, baseRoot, compareRoot)).ToCollection(); Collection files = allFilesystemEntries.Except(directories).ToCollection(); if (ImGui.BeginTable("CompareBrowser", 3, ImGuiTableFlags.Borders)) @@ -2022,7 +2024,7 @@ private void ShowCompareBrowser() GitRepository repoB = Options.Repos[Options.CompareRepo]; if (repoA is GitHubRepository githubRepoA && repoB is GitHubRepository githubRepoB) { - SwitchCompareBrowserPath(GetFullyQualifiedRepoName(githubRepoA.OwnerName, githubRepoA.RepoName), GetFullyQualifiedRepoName(githubRepoB.OwnerName, githubRepoB.RepoName), RelativeDirectoryPath.Create(Path.Combine(Options.BrowsePath, path))); + SwitchCompareBrowserPath(GetFullyQualifiedRepoName(githubRepoA.OwnerName, githubRepoA.RepoName), GetFullyQualifiedRepoName(githubRepoB.OwnerName, githubRepoB.RepoName), RelativeDirectoryPath.Create(path.WeakString)); } else @@ -2036,11 +2038,11 @@ private void ShowCompareBrowser() { if (existsInA && ImGui.ArrowButton("##Copy", ImGuiDir.Right)) { - _ = Directory.CreateDirectory(Path.Combine(Options.Repos[Options.CompareRepo].LocalPath, Options.BrowsePath, path)); + LogBrowserFailure("Copying", path, RepoBrowsing.Copy(path, baseRoot, compareRoot)); } else if (existsInB && ImGui.ArrowButton("##Copy", ImGuiDir.Left)) { - _ = Directory.CreateDirectory(Path.Combine(Options.Repos[Options.BaseRepo].LocalPath, Options.BrowsePath, path)); + LogBrowserFailure("Copying", path, RepoBrowsing.Copy(path, compareRoot, baseRoot)); } if (ImGui.IsItemHovered()) @@ -2058,11 +2060,11 @@ private void ShowCompareBrowser() { if (existsInA && ImGui.Button("X##Remove")) { - Directory.Delete(Path.Combine(Options.Repos[Options.BaseRepo].LocalPath, Options.BrowsePath, path)); + LogBrowserFailure("Deleting", path, RepoBrowsing.Delete(path, baseRoot)); } else if (existsInB && ImGui.Button("X##Remove")) { - Directory.Delete(Path.Combine(Options.Repos[Options.CompareRepo].LocalPath, Options.BrowsePath, path)); + LogBrowserFailure("Deleting", path, RepoBrowsing.Delete(path, compareRoot)); } if (ImGui.IsItemHovered()) @@ -2092,15 +2094,11 @@ private void ShowCompareBrowser() { if (existsInA && ImGui.ArrowButton("##Copy", ImGuiDir.Right)) { - string srcPath = Path.Combine(Options.Repos[Options.BaseRepo].LocalPath, Options.BrowsePath, path); - string dstPath = Path.Combine(Options.Repos[Options.CompareRepo].LocalPath, Options.BrowsePath, path); - File.Copy(srcPath, dstPath); + LogBrowserFailure("Copying", path, RepoBrowsing.Copy(path, baseRoot, compareRoot)); } else if (existsInB && ImGui.ArrowButton("##Copy", ImGuiDir.Left)) { - string srcPath = Path.Combine(Options.Repos[Options.CompareRepo].LocalPath, Options.BrowsePath, path); - string dstPath = Path.Combine(Options.Repos[Options.BaseRepo].LocalPath, Options.BrowsePath, path); - File.Copy(srcPath, dstPath); + LogBrowserFailure("Copying", path, RepoBrowsing.Copy(path, compareRoot, baseRoot)); } if (ImGui.IsItemHovered()) @@ -2118,11 +2116,11 @@ private void ShowCompareBrowser() { if (existsInA && ImGui.Button("X##Remove")) { - File.Delete(Path.Combine(Options.Repos[Options.BaseRepo].LocalPath, Options.BrowsePath, path)); + LogBrowserFailure("Deleting", path, RepoBrowsing.Delete(path, baseRoot)); } else if (existsInB && ImGui.Button("X##Remove")) { - File.Delete(Path.Combine(Options.Repos[Options.CompareRepo].LocalPath, Options.BrowsePath, path)); + LogBrowserFailure("Deleting", path, RepoBrowsing.Delete(path, compareRoot)); } if (ImGui.IsItemHovered()) @@ -2147,7 +2145,7 @@ private void ShowRepoBrowser() { Collection allFilesystemEntries = BrowserContentsBase; GitRepository baseRepo = Options.Repos[Options.BaseRepo]; - Collection directories = allFilesystemEntries.Where(x => Directory.Exists(Path.Combine(baseRepo.LocalPath, Options.BrowsePath, x))).ToCollection(); + Collection directories = allFilesystemEntries.Where(x => RepoBrowsing.IsDirectory(x, baseRepo.LocalPath)).ToCollection(); Collection files = allFilesystemEntries.Except(directories).ToCollection(); bool shouldOpenPopup = false; @@ -2315,34 +2313,25 @@ private void ShowRepoBrowser() _ = PopupPropagateFile.ShowIfOpen(); } + private void LogBrowserFailure(string action, RelativePath path, string? failure) + { + if (failure is not null) + { + QueueLog($"{action} {path} failed: {failure}"); + } + } + private void SwitchCompareBrowserPath(FullyQualifiedGitHubRepoName baseRepo, FullyQualifiedGitHubRepoName compareRepo, RelativeDirectoryPath newPath) { Options.BrowsePath = newPath; GitRepository repoA = Options.Repos[baseRepo]; GitRepository repoB = Options.Repos[compareRepo]; - static RelativePath formatPath(string path, string prefix) => RelativePath.Create(path.RemovePrefix(prefix + Path.DirectorySeparatorChar) + (Directory.Exists(path) ? Path.DirectorySeparatorChar : string.Empty)); - BrowserContentsBase.Clear(); BrowserContentsCompare.Clear(); - try - { - BrowserContentsBase = Directory.EnumerateFileSystemEntries(Path.Combine(repoA.LocalPath, Options.BrowsePath)).Select(x => formatPath(x, repoA.LocalPath)).ToCollection(); - } - catch (DirectoryNotFoundException) - { - // skip this repo - } - - try - { - BrowserContentsCompare = Directory.EnumerateFileSystemEntries(Path.Combine(repoB.LocalPath, Options.BrowsePath)).Select(x => formatPath(x, repoB.LocalPath)).ToCollection(); - } - catch (DirectoryNotFoundException) - { - // skip this repo - } + BrowserContentsBase = RepoBrowsing.List(repoA.LocalPath, Options.BrowsePath); + BrowserContentsCompare = RepoBrowsing.List(repoB.LocalPath, Options.BrowsePath); QueueSaveOptions(); } @@ -2357,19 +2346,10 @@ private void SwitchRepoBrowserPath(FullyQualifiedGitHubRepoName baseRepo, Relati Options.BrowsePath = newPath; GitRepository repoA = Options.Repos[baseRepo]; - static RelativePath formatPath(string path, string prefix) => RelativePath.Create(path.RemovePrefix(prefix + Path.DirectorySeparatorChar) + (Directory.Exists(path) ? Path.DirectorySeparatorChar : string.Empty)); - BrowserContentsBase.Clear(); BrowserContentsCompare.Clear(); - try - { - BrowserContentsBase = Directory.EnumerateFileSystemEntries(Path.Combine(repoA.LocalPath, Options.BrowsePath)).Select(x => formatPath(x, repoA.LocalPath)).ToCollection(); - } - catch (DirectoryNotFoundException) - { - // skip this repo - } + BrowserContentsBase = RepoBrowsing.List(repoA.LocalPath, Options.BrowsePath); QueueSaveOptions(); } diff --git a/ProjectDirector/RepoBrowsing.cs b/ProjectDirector/RepoBrowsing.cs new file mode 100644 index 0000000..1253d9a --- /dev/null +++ b/ProjectDirector/RepoBrowsing.cs @@ -0,0 +1,112 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.ProjectDirector; + +using System.Collections.ObjectModel; +using System.IO; +using ktsu.Semantics.Paths; + +/// +/// The path handling behind the repo and compare browsers, kept apart from the drawing so it can be +/// driven against real throwaway directories. +/// +/// +/// Every entry is relative to the repository root, not to the folder being browsed, so it already +/// includes the browse path. Combining it with the browse path again is what used to send the +/// browsers to src/src/.... Whether an entry is a folder is asked of the filesystem rather +/// than read from a trailing separator, because normalises that away. +/// +internal static class RepoBrowsing +{ + /// + /// Lists one folder of a repository. + /// + /// The repository's local path. + /// The folder to list, relative to . + /// Each entry relative to , or nothing when the folder is not there. + internal static Collection List(string repoRoot, string browsePath) + { + try + { + return [.. Directory.EnumerateFileSystemEntries(Path.Join(repoRoot, browsePath)) + .Select(entry => RelativePath.Create(Path.GetRelativePath(repoRoot, entry)))]; + } + catch (DirectoryNotFoundException) + { + // The other repository may not have this folder at all. + return []; + } + } + + /// + /// Gets whether an entry is a folder in any of the given repositories. + /// + /// An entry from . + /// The repositories it may have come from. + /// when it is a folder in at least one of them. + internal static bool IsDirectory(RelativePath entry, params string[] repoRoots) => + repoRoots.Any(root => Directory.Exists(Path.Join(root, entry))); + + /// + /// Copies an entry that exists in one repository into the same place in another. + /// + /// An entry from . + /// The repository that has it. + /// The repository that does not. + /// Why the copy failed, or null when it succeeded. + /// A folder is created empty, as the browser always has, rather than copied with its contents. + internal static string? Copy(RelativePath entry, string fromRoot, string toRoot) + { + string source = Path.Join(fromRoot, entry); + string destination = Path.Join(toRoot, entry); + + try + { + if (Directory.Exists(source)) + { + _ = Directory.CreateDirectory(destination); + } + else + { + _ = Directory.CreateDirectory(Path.GetDirectoryName(destination)!); + File.Copy(source, destination); + } + + return null; + } + catch (Exception failure) when (failure is IOException or UnauthorizedAccessException) + { + return failure.Message; + } + } + + /// + /// Deletes an entry from one repository. + /// + /// An entry from . + /// The repository to delete it from. + /// Why the delete failed, or null when it succeeded. + /// A folder is only deleted when it is empty, so one click cannot take a tree with it. + internal static string? Delete(RelativePath entry, string repoRoot) + { + string path = Path.Join(repoRoot, entry); + + try + { + if (Directory.Exists(path)) + { + Directory.Delete(path); + } + else + { + File.Delete(path); + } + + return null; + } + catch (Exception failure) when (failure is IOException or UnauthorizedAccessException) + { + return failure.Message; + } + } +}