diff --git a/CLAUDE.md b/CLAUDE.md index f950d76..abc29b8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,7 +26,7 @@ Tests live in `ProjectDirector.Test` (MSTest, via `MSTest.Sdk` + `ktsu.Sdk`). Th `tests/ProjectDirector.UITests` drives the real application headlessly through `ktsu.ImGui.App.Testing` to render the app gallery (`Gallery/`), and exists for that purpose. It reaches the app through `ProjectDirector.BuildConfig()` and the internal options constructor. It replaces the settings file and the secret store with in-memory ones, and points the app at three repositories it seeds offline under `/tmp/projectdirector-gallery`. That path is fixed because it appears in the pictures. Every picture is cropped above the log panel, because the log prints wall-clock timestamps. `PROJECTDIRECTOR_GALLERY_OUT` names the output directory; `.github/workflows/app-gallery.yml` regenerates `docs/gallery/` after CI passes on `main`. A new view or dialog worth showing gets an entry in `Gallery/GalleryCatalog.cs`. -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 6c0d724..5c400ef 100644 --- a/ProjectDirector/ProjectDirector.cs +++ b/ProjectDirector/ProjectDirector.cs @@ -2074,7 +2074,9 @@ private static void ShowDiffSummaryText(int linesDeleted, int linesAdded) private void ShowCompareBrowser() { IEnumerable allFilesystemEntries = BrowserContentsBase.Union(BrowserContentsCompare); - Collection directories = ListBrowserDirectories(allFilesystemEntries, Options.Repos[Options.BaseRepo].LocalPath, Options.Repos[Options.CompareRepo].LocalPath); + string baseRoot = Options.Repos[Options.BaseRepo].LocalPath; + string compareRoot = Options.Repos[Options.CompareRepo].LocalPath; + Collection directories = ListBrowserDirectories(allFilesystemEntries, baseRoot, compareRoot); Collection files = allFilesystemEntries.Except(directories).ToCollection(); if (ImGui.BeginTable("CompareBrowser", 3, ImGuiTableFlags.Borders)) @@ -2120,7 +2122,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 @@ -2134,11 +2136,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()) @@ -2156,11 +2158,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()) @@ -2190,15 +2192,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()) @@ -2216,11 +2214,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()) @@ -2416,17 +2414,13 @@ private void ShowRepoBrowser() _ = PopupPropagateFile.ShowIfOpen(); } - /// - /// Lists a directory for the repository browsers, in name order. - /// - /// The directory to list. - /// The full paths of its files and directories, ordered by name ignoring case. - /// - /// The file system's own order is whatever that file system keeps: alphabetical on NTFS, by hash - /// on ext4. Sorting makes the browsers read the same on every machine. - /// - internal static IEnumerable ListBrowserEntries(string directory) => - Directory.EnumerateFileSystemEntries(directory).Order(StringComparer.OrdinalIgnoreCase); + private void LogBrowserFailure(string action, RelativePath path, string? failure) + { + if (failure is not null) + { + QueueLog($"{action} {path} failed: {failure}"); + } + } /// /// Picks out the entries of a browser listing that are directories in any of the given repositories. @@ -2440,7 +2434,7 @@ internal static IEnumerable ListBrowserEntries(string directory) => /// compare browser used to list every directory as a file that could not be opened. /// internal static Collection ListBrowserDirectories(IEnumerable entries, params string[] repoRoots) => - entries.Where(entry => repoRoots.Any(root => Directory.Exists(Path.Join(root, entry)))).ToCollection(); + entries.Where(entry => RepoBrowsing.IsDirectory(entry, repoRoots)).ToCollection(); private void SwitchCompareBrowserPath(FullyQualifiedGitHubRepoName baseRepo, FullyQualifiedGitHubRepoName compareRepo, RelativeDirectoryPath newPath) { @@ -2448,28 +2442,11 @@ private void SwitchCompareBrowserPath(FullyQualifiedGitHubRepoName baseRepo, Ful 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 = ListBrowserEntries(Path.Join(repoA.LocalPath, Options.BrowsePath)).Select(x => formatPath(x, repoA.LocalPath)).ToCollection(); - } - catch (DirectoryNotFoundException) - { - // skip this repo - } - - try - { - BrowserContentsCompare = ListBrowserEntries(Path.Join(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(); } @@ -2484,19 +2461,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 = ListBrowserEntries(Path.Join(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..967c5f1 --- /dev/null +++ b/ProjectDirector/RepoBrowsing.cs @@ -0,0 +1,117 @@ +// 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 , in name order ignoring case, or nothing when the folder is not there. + /// + /// The file system's own order is whatever that file system keeps: alphabetical on NTFS, by hash + /// on ext4. Sorting makes the browsers read the same on every machine. + /// + internal static Collection List(string repoRoot, string browsePath) + { + try + { + return [.. Directory.EnumerateFileSystemEntries(Path.Join(repoRoot, browsePath)) + .Order(StringComparer.OrdinalIgnoreCase) + .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; + } + } +}