diff --git a/src/Stampeded.Core/Diff/NoTextualDiff.cs b/src/Stampeded.Core/Diff/NoTextualDiff.cs new file mode 100644 index 0000000..f7b807e --- /dev/null +++ b/src/Stampeded.Core/Diff/NoTextualDiff.cs @@ -0,0 +1,86 @@ +using System.Globalization; + +namespace Stampeded.Core.Diff; + +/// +/// What to say about a file the diff has nothing to show for. +/// +/// Two kinds reach a review with no lines in them: a binary file, which git compares byte by +/// byte and reports only as differing, and a file that changed without its content changing - +/// a rename, or a permission bit. Both used to open as an empty document, which reads as a +/// tool that failed rather than as a file with nothing to read, and the file list showed them +/// with no counts at all beside files that genuinely changed nothing. +/// +public static class NoTextualDiff +{ + /// Whether this file has no lines to show. A generated file is never one of + /// these: its two sides are read from disk and compared here, so it either has hunks or + /// was left out of the change altogether. + public static bool Applies(FileDiff file) => file.IsBinary || file.Hunks.Count == 0; + + /// The badge the file list carries for such a file, or empty for one with lines. + /// Short, because it stands where the line counts would. + public static string Badge(FileDiff file) + => !Applies(file) ? "" : file.IsBinary ? "binary" : "no lines"; + + /// + /// The page shown in place of the diff. Sizes are what a reader actually wants to know + /// about a binary change - whether the image got bigger - and are null for the side that + /// does not have the file. + /// + public static string Describe(FileDiff file, long? baseSize, long? headSize) + { + var text = new System.Text.StringBuilder(); + text.Append(file.Path).Append("\n\n"); + text.Append(file.IsBinary + ? "Binary file: git compares the bytes and reports only that they differ, so there\n" + + "is no textual diff to read.\n\n" + : "No lines changed. A file reaches a change without any when it was renamed, or\n" + + "when only its permissions moved.\n\n"); + text.Append(Kind(file)); + if (file.Kind == FileChangeKind.Renamed) + text.Append(" from ").Append(file.OldPath); + text.Append('.'); + if (Sizes(baseSize, headSize) is { Length: > 0 } sizes) + text.Append(" ").Append(sizes); + return text.Append('\n').ToString(); + } + + static string Kind(FileDiff file) => file.Kind switch { + FileChangeKind.Added => "Added", + FileChangeKind.Deleted => "Deleted", + FileChangeKind.Renamed => "Renamed", + _ => "Modified", + }; + + /// + /// How the two sides compare, said the way a reader would: one size for a file that only + /// one revision has, and the difference spelled out for one both have - "grew by 2.5 KB" + /// is the fact, where two absolute numbers are the arithmetic to get there. + /// + static string Sizes(long? baseSize, long? headSize) => (baseSize, headSize) switch { + (null, { } head) => Size(head), + ({ } start, null) => "was " + Size(start), + ({ } start, { } end) when start == end => Size(end) + ", unchanged in size", + ({ } start, { } end) => $"{Size(start)} -> {Size(end)} ({(end > start ? "+" : "-")}{Size(Math.Abs(end - start))})", + _ => "", + }; + + /// A byte count as a person reads one. Binary in the sense the file managers use, + /// because that is what the reader will see if they check. + public static string Size(long bytes) + { + string[] units = ["bytes", "KB", "MB", "GB"]; + double size = bytes; + int unit = 0; + while (size >= 1024 && unit < units.Length - 1) + { + size /= 1024; + unit++; + } + // Whole bytes: a file of 512.0 bytes is 512 bytes, and the decimal is noise. + return unit == 0 + ? string.Create(CultureInfo.InvariantCulture, $"{bytes} {units[0]}") + : string.Create(CultureInfo.InvariantCulture, $"{size:0.#} {units[unit]}"); + } +} diff --git a/src/Stampeded.Core/Git/GitService.cs b/src/Stampeded.Core/Git/GitService.cs index 0e688a3..8a9a431 100644 --- a/src/Stampeded.Core/Git/GitService.cs +++ b/src/Stampeded.Core/Git/GitService.cs @@ -328,6 +328,29 @@ public async Task> DiffWorkingTreeAsync( public Task ShowFileAsync(string rev, string path, CancellationToken ct = default) => RunAsync(ct, "show", $"{rev}:{path}"); + /// + /// How many bytes a revision's copy of a file has, or null when that revision does not + /// have it - which is the ordinary answer for the base side of an addition and the head + /// side of a deletion, not a failure. + /// + /// Asked of the object database rather than read, because the file this is wanted for is + /// the one that cannot be read as text: how much a binary file grew is the whole of what + /// a review can say about it. + /// + public async Task BlobSizeAsync(string rev, string path, CancellationToken ct = default) + { + try + { + return long.TryParse((await RunAsync(ct, "cat-file", "-s", $"{rev}:{path}")).Trim(), out long size) + ? size + : null; + } + catch (ToolFailedException) + { + return null; + } + } + /// A whole commit as a patch - its message and its diff, the way git prints it. public Task ShowCommitAsync(string rev, CancellationToken ct = default) => RunAsync(ct, "show", rev); diff --git a/src/Stampeded/Documents/MarkdownDocumentView.axaml b/src/Stampeded/Documents/MarkdownDocumentView.axaml new file mode 100644 index 0000000..d3cc803 --- /dev/null +++ b/src/Stampeded/Documents/MarkdownDocumentView.axaml @@ -0,0 +1,10 @@ + + + + diff --git a/src/Stampeded/Documents/MarkdownDocumentView.axaml.cs b/src/Stampeded/Documents/MarkdownDocumentView.axaml.cs new file mode 100644 index 0000000..22db5db --- /dev/null +++ b/src/Stampeded/Documents/MarkdownDocumentView.axaml.cs @@ -0,0 +1,30 @@ +using Avalonia.Controls; + +namespace Stampeded.Documents; + +public partial class MarkdownDocumentView : UserControl +{ + public MarkdownDocumentView() + { + InitializeComponent(); + // A document under review is full of links - to its own sections, to issues, to the + // sites it cites - and without an engine that runs them a link renders as a link and + // does nothing when pressed. + Rendered.Engine = Editor.MarkdownLinks.NewEngine(); + // A preview is read to be quoted from, in a comment about the paragraph it shows. The + // renderer paints a selection but carries nothing that copies one, and the blocks it + // draws never take focus, so the gesture is handled by the page. + Controls.MarkdownSelection.Enable(this); + // The renderer draws the markers of an emphasis reaching around a link or a code span + // instead of the emphasis itself; they are paired again from the pieces it left, once + // it has drawn. Posted, because the rendered blocks exist only after the layout pass. + Rendered.PropertyChanged += (_, e) => { + if (e.Property == global::Markdown.Avalonia.MarkdownScrollViewer.MarkdownProperty) + { + Avalonia.Threading.Dispatcher.UIThread.Post( + () => Controls.MarkdownEmphasis.Repair(Rendered), + Avalonia.Threading.DispatcherPriority.Loaded); + } + }; + } +} diff --git a/src/Stampeded/Documents/MarkdownDocumentViewModel.cs b/src/Stampeded/Documents/MarkdownDocumentViewModel.cs new file mode 100644 index 0000000..d1b4bba --- /dev/null +++ b/src/Stampeded/Documents/MarkdownDocumentViewModel.cs @@ -0,0 +1,17 @@ +using Dock.Model.Mvvm.Controls; + +namespace Stampeded.Documents; + +/// +/// A markdown file as it renders, in a tab of its own beside the diff of its source. A change +/// to a README or a document is read for what it will look like as much as for what it says, +/// and the diff can only show the source. +/// +public class MarkdownDocumentViewModel(string text) : Document +{ + public string Text { get; } = text; + + /// What the tab strip says about this tab on hover. The title is a file name and + /// several documents can share one; the path is what tells them apart. + public string TabTooltip { get; init; } = ""; +} diff --git a/src/Stampeded/Documents/ReviewGestures.cs b/src/Stampeded/Documents/ReviewGestures.cs index ebc83bb..2728387 100644 --- a/src/Stampeded/Documents/ReviewGestures.cs +++ b/src/Stampeded/Documents/ReviewGestures.cs @@ -64,6 +64,11 @@ public static bool Handle(KeyEventArgs e, IReviewDocumentView view) case (Key.C, KeyModifiers.None): view.CommentAtCaretCommand(); return true; + // The workspace rather than the view: a preview is a document of its own and needs + // nothing from the caret, so both layouts offer it without implementing anything. + case (Key.M, KeyModifiers.None): + App.Workspace?.OpenMarkdownPreviewAsync().HandleExceptions(); + return true; case (Key.Left, KeyModifiers.Alt): workspace?.GoBackAsync().HandleExceptions(); return true; diff --git a/src/Stampeded/KeyboardShortcuts.cs b/src/Stampeded/KeyboardShortcuts.cs index de6e4c3..626e95c 100644 --- a/src/Stampeded/KeyboardShortcuts.cs +++ b/src/Stampeded/KeyboardShortcuts.cs @@ -31,6 +31,7 @@ F12 go to definition Alt+Left back Alt+Right forward b blame margin on or off + m render the markdown file in front, in a tab of its own Esc clear highlighted occurrences Saying something diff --git a/src/Stampeded/MainWindow.axaml b/src/Stampeded/MainWindow.axaml index ee423fb..096335f 100644 --- a/src/Stampeded/MainWindow.axaml +++ b/src/Stampeded/MainWindow.axaml @@ -162,6 +162,12 @@ ToggleType="CheckBox" ToolTip="Read every change as two panes instead of one interleaved document" /> + + diff --git a/src/Stampeded/MainWindow.axaml.cs b/src/Stampeded/MainWindow.axaml.cs index 2b75cba..dc37394 100644 --- a/src/Stampeded/MainWindow.axaml.cs +++ b/src/Stampeded/MainWindow.axaml.cs @@ -16,7 +16,7 @@ public partial class MainWindow : Window readonly NativeMenuItem recentMenu, buildSolutionMenu, exitItem, nextHunkItem, prevHunkItem, nextUncoveredItem, historyOfSelectionItem, backItem, forwardItem, - sideBySideItem, blameItem, multiRowTabsItem, pointerCrossHairItem, debugHereItem, + sideBySideItem, blameItem, markdownPreviewItem, multiRowTabsItem, pointerCrossHairItem, debugHereItem, lightThemeItem, darkThemeItem; public MainWindow() @@ -36,6 +36,7 @@ NativeMenuItem Named(string key) => FindMenuItem(menu, i => key.Equals(i.Command forwardItem = Named("ForwardItem"); sideBySideItem = Named("SideBySideItem"); blameItem = Named("BlameItem"); + markdownPreviewItem = Named("MarkdownPreviewItem"); multiRowTabsItem = Named("MultiRowTabsItem"); pointerCrossHairItem = Named("PointerCrossHairItem"); debugHereItem = Named("DebugHereItem"); @@ -193,6 +194,9 @@ protected override void OnKeyDown(KeyEventArgs e) case (Key.C, KeyModifiers.None): View?.CommentAtCaretCommand(); break; + case (Key.M, KeyModifiers.None): + App.Workspace?.OpenMarkdownPreviewAsync().HandleExceptions(); + break; case (Key.F12, KeyModifiers.None): View?.GoToDefinitionCommand(); break; @@ -289,6 +293,7 @@ async Task PromptUrlAsync() void OnToggleViewed(object? s, EventArgs e) => App.Workspace?.ToggleViewedAndAdvanceAsync().HandleExceptions(); void OnToggleOverview(object? s, EventArgs e) => App.Workspace?.ToggleOverviewAsync().HandleExceptions(); void OnToggleBlame(object? s, EventArgs e) => View?.ToggleBlameCommand(); + void OnMarkdownPreview(object? s, EventArgs e) => App.Workspace?.OpenMarkdownPreviewAsync().HandleExceptions(); void OnCommentAtCaret(object? s, EventArgs e) => View?.CommentAtCaretCommand(); void OnGoToDefinition(object? s, EventArgs e) => View?.GoToDefinitionCommand(); void OnFindReferences(object? s, EventArgs e) => View?.FindReferencesCommand(); @@ -508,6 +513,9 @@ internal void RefreshMenus() sideBySideItem.IsChecked = DiffLayoutPreference.SideBySide; blameItem.IsEnabled = Has(ReviewCommands.ToggleBlame); blameItem.IsChecked = view?.BlameVisible ?? false; + // Only where there is something to render. Offered on a .cs file it would do nothing, + // which reads as a broken command rather than one that does not apply here. + markdownPreviewItem.IsEnabled = file is not null && ReviewWorkspace.IsMarkdown(file.Path); debugHereItem.IsEnabled = Has(ReviewCommands.DebugHere); historyOfSelectionItem.IsEnabled = Has(ReviewCommands.HistoryOfSelection); } diff --git a/src/Stampeded/Panes/PrFilesPaneView.axaml b/src/Stampeded/Panes/PrFilesPaneView.axaml index 8a9a41a..09b6d1b 100644 --- a/src/Stampeded/Panes/PrFilesPaneView.axaml +++ b/src/Stampeded/Panes/PrFilesPaneView.axaml @@ -42,6 +42,11 @@ reader picks the next file by, in the list they pick it from. --> + + diff --git a/src/Stampeded/Panes/PrFilesPaneViewModel.cs b/src/Stampeded/Panes/PrFilesPaneViewModel.cs index 10e9985..2b0cac3 100644 --- a/src/Stampeded/Panes/PrFilesPaneViewModel.cs +++ b/src/Stampeded/Panes/PrFilesPaneViewModel.cs @@ -47,6 +47,11 @@ public string CoverageBadge { /// "new!" when the latest push touched this file after it was last reviewed. public string SinceBadge { get; init; } = ""; + /// "binary" or "no lines" for a file the diff has nothing to show for. It stands + /// where the line counts would, which is otherwise empty for such a file and reads as a + /// change of no size rather than one with nothing to read. + public string NoDiffBadge { get; init; } = ""; + /// How much of the file changed, the way the diff counts it. The size of a file's /// change is what a reader picks the next one by, and the list was the one place that said /// only which files, not how much of them. @@ -194,6 +199,7 @@ void Rebuild() CommentBadge = badge, CommentsSettled = settled, SinceBadge = markTouched && workspace.IsTouchedSinceLastPass(file.Path) ? "new!" : "", + NoDiffBadge = Core.Diff.NoTextualDiff.Badge(file), }; entry.PropertyChanged += OnEntryChanged; Files.Add(entry); diff --git a/src/Stampeded/ReviewWorkspace.cs b/src/Stampeded/ReviewWorkspace.cs index cc641e5..f5b9efe 100644 --- a/src/Stampeded/ReviewWorkspace.cs +++ b/src/Stampeded/ReviewWorkspace.cs @@ -771,6 +771,11 @@ void SetGeneratedStatus(string status, bool done) return ShowDiffDocument(file, ReadOrEmpty(generated.BaseFile), ReadOrEmpty(generated.HeadFile)); if (BaseSha is null || HeadSha is null) return null; + // A binary file, or one that changed without its lines changing, has nothing to draw. + // Built as a diff of two empty sides it opened as a blank document, which reads as a + // tool that failed rather than as a file with nothing in it to read. + if (NoTextualDiff.Applies(file)) + return await ShowNoTextualDiffAsync(file, record); string oldText = ""; if (file.Kind != FileChangeKind.Added && !file.IsBinary) { @@ -801,6 +806,84 @@ void SetGeneratedStatus(string status, bool done) static string ReadOrEmpty(string? path) => path is null ? "" : File.ReadAllText(path); } + /// + /// The page a file with no lines opens as: what kind of nothing it is, and for a binary one + /// how the two sides compare in size, which is the only thing a review can say about it. + /// + /// It takes the file's own tab and carries the file's own path, so it is a file of the + /// review like any other: it can be marked viewed, stepped past, and commented on where the + /// host allows one. + /// + async Task ShowNoTextualDiffAsync(FileDiff file, bool record) + { + long? baseSize = file.Kind == FileChangeKind.Added || BaseSha is not { } baseSha + ? null + : await Git.BlobSizeAsync(baseSha, file.OldPath); + long? headSize = file.Kind == FileChangeKind.Deleted || HeadSha is not { } headSha + ? null + : await Git.BlobSizeAsync(headSha, file.NewPath); + string described = NoTextualDiff.Describe(file, baseSize, headSize); + CliLog.Write("review", $"{file.Path}: {NoTextualDiff.Badge(file)}, nothing to diff"); + var document = ShowDocument("diff:" + file.Path, () => { + var page = Stampeded.Documents.DiffDocumentViewModel.ForSource(file.Path, described); + page.Title = Path.GetFileName(file.Path); + page.TabTooltipOverride = file.Path + " - " + NoTextualDiff.Badge(file); + return page; + }); + if (record && document is not null) + RecordArrival("diff:" + file.Path); + return document; + } + + /// Whether a path is markdown, and so has a rendering worth looking at beside its + /// source. + public static bool IsMarkdown(string path) => MarkdownExtensions.Contains(Path.GetExtension(path)); + + static readonly IReadOnlySet MarkdownExtensions = + new HashSet(StringComparer.OrdinalIgnoreCase) { ".md", ".markdown" }; + + /// + /// Opens the rendered form of a markdown file beside its diff. A change to a README or a + /// document is read for what it will look like as much as for what it says, and the diff + /// shows the source - so the preview is a tab of its own rather than a mode of the diff, + /// and both can be open at once. + /// + /// The head side, because that is what the change produces; the base side for a file the + /// change deletes, which has no head side to render. + /// + public async Task OpenMarkdownPreviewAsync(FileDiff? file = null) + { + if ((file ?? CurrentFile) is not { } target) + { + StatusMessage?.Invoke("No file is open to preview."); + return; + } + if (!IsMarkdown(target.Path)) + { + StatusMessage?.Invoke($"{Path.GetFileName(target.Path)} is not markdown, so there is nothing to render."); + return; + } + bool oldSide = target.Kind == FileChangeKind.Deleted; + string? text = oldSide + ? BaseSha is { } baseSha ? await Blobs.ReadAsync(baseSha, target.OldPath) : null + : await ReadHeadFileAsync(target.NewPath); + if (text is null) + { + StatusMessage?.Invoke($"{target.Path} is not in the {(oldSide ? "base" : "head")} revision."); + return; + } + string id = "md:" + target.Path; + // Rendered from the revision on screen, so a preview left open while the scope moves + // under it would be showing another revision's text under this one's name. + if (Factory is not null && Documents?.VisibleDockables?.FirstOrDefault(d => d.Id == id) is { } stale) + Factory.CloseDockable(stale); + ShowDocument(id, () => new Stampeded.Documents.MarkdownDocumentViewModel(text) { + Title = Path.GetFileName(target.Path) + (oldSide ? " (preview @ base)" : " (preview)"), + TabTooltip = target.Path + " rendered", + }); + CliLog.Write("action", $"markdown preview {target.Path}{(oldSide ? " (base)" : "")}"); + } + /// The head side of a file whose earlier version cannot be read, as a source view: /// there is nothing to compare it with, and saying so beats drawing a diff that is not /// one. It takes the file's own tab, so reopening the file replaces it once the base is diff --git a/src/Stampeded/ViewLocator.cs b/src/Stampeded/ViewLocator.cs index ff0b65f..321996c 100644 --- a/src/Stampeded/ViewLocator.cs +++ b/src/Stampeded/ViewLocator.cs @@ -17,6 +17,7 @@ public class ViewLocator : IDataTemplate [typeof(DiffDocumentViewModel)] = () => new Documents.DiffDocumentView(), [typeof(SideBySideDocumentViewModel)] = () => new Documents.SideBySideDocumentView(), [typeof(TextDocumentViewModel)] = () => new Documents.TextDocumentView(), + [typeof(MarkdownDocumentViewModel)] = () => new Documents.MarkdownDocumentView(), [typeof(StartDocumentViewModel)] = () => new Documents.StartDocumentView(), [typeof(OverviewDocumentViewModel)] = () => new Documents.OverviewDocumentView(), [typeof(ReviewDocumentViewModel)] = () => new Documents.ReviewDocumentView(), diff --git a/tests/Stampeded.Core.Tests/NoTextualDiffTests.cs b/tests/Stampeded.Core.Tests/NoTextualDiffTests.cs new file mode 100644 index 0000000..bea67a4 --- /dev/null +++ b/tests/Stampeded.Core.Tests/NoTextualDiffTests.cs @@ -0,0 +1,84 @@ +using NUnit.Framework; + +using Stampeded.Core.Diff; + +namespace Stampeded.Core.Tests; + +/// +/// What a file with nothing to read says for itself. Both kinds used to open as an empty +/// document, which a reader cannot tell from a tool that failed. +/// +public class NoTextualDiffTests +{ + static FileDiff Binary(FileChangeKind kind = FileChangeKind.Modified, string path = "docs/logo.png") + => new(path, path, kind, IsBinary: true, []); + + static FileDiff Textual(FileChangeKind kind, string oldPath, string newPath) + => new(oldPath, newPath, kind, IsBinary: false, []); + + [Test] + public void AppliesToABinaryFileAndToOneWithNoHunks() + { + Assert.That(NoTextualDiff.Applies(Binary()), Is.True); + Assert.That(NoTextualDiff.Applies(Textual(FileChangeKind.Renamed, "a.md", "b.md")), Is.True); + Assert.That(NoTextualDiff.Applies( + new("a.cs", "a.cs", FileChangeKind.Modified, false, [new DiffHunk(1, 1, 1, 1, "", [])])), + Is.False, "a file with hunks has something to read"); + } + + [Test] + public void SaysWhichKindOfNothingItIs() + { + Assert.That(NoTextualDiff.Badge(Binary()), Is.EqualTo("binary")); + Assert.That(NoTextualDiff.Badge(Textual(FileChangeKind.Renamed, "a.md", "b.md")), Is.EqualTo("no lines")); + Assert.That(NoTextualDiff.Badge( + new("a.cs", "a.cs", FileChangeKind.Modified, false, [new DiffHunk(1, 1, 1, 1, "", [])])), + Is.Empty); + } + + [Test] + public void ComparesTheTwoSidesOfABinaryChange() + { + string described = NoTextualDiff.Describe(Binary(), baseSize: 12 * 1024, headSize: 14 * 1024); + + Assert.That(described, Does.Contain("Binary file")); + Assert.That(described, Does.Contain("Modified.")); + Assert.That(described, Does.Contain("12 KB -> 14 KB (+2 KB)"), + "the direction and the difference, not two numbers to subtract"); + } + + [Test] + public void NamesOnlyTheSideThatHasTheFile() + { + Assert.That(NoTextualDiff.Describe(Binary(FileChangeKind.Added), null, 2048), + Does.Contain("Added. 2 KB")); + Assert.That(NoTextualDiff.Describe(Binary(FileChangeKind.Deleted), 2048, null), + Does.Contain("Deleted. was 2 KB")); + } + + [Test] + public void SaysWhereARenameCameFrom() + { + string described = NoTextualDiff.Describe( + Textual(FileChangeKind.Renamed, "docs/old-guide.md", "docs/guide.md"), null, null); + + Assert.That(described, Does.Contain("No lines changed")); + Assert.That(described, Does.Contain("Renamed from docs/old-guide.md.")); + } + + [Test] + public void ASizeThatDidNotMoveSaysSo() + { + // A binary file whose bytes differ at the same length - a rebuilt asset, a re-encoded + // image - would otherwise read as "4 KB -> 4 KB" and look like a mistake. + Assert.That(NoTextualDiff.Describe(Binary(), 4096, 4096), Does.Contain("4 KB, unchanged in size")); + } + + [TestCase(0, "0 bytes")] + [TestCase(512, "512 bytes")] + [TestCase(1024, "1 KB")] + [TestCase(1536, "1.5 KB")] + [TestCase(1024 * 1024, "1 MB")] + public void ReadsASizeTheWayAPersonWould(long bytes, string expected) + => Assert.That(NoTextualDiff.Size(bytes), Is.EqualTo(expected)); +}