diff --git a/claude.md b/claude.md index df75dbab..fd3c2228 100644 --- a/claude.md +++ b/claude.md @@ -190,6 +190,16 @@ apart. enrichment on top. Copying is `IViewerWindow.SetClipboard` rather than a `ViewerActions` member, because a clipboard belongs to a toolkit the way a window does, and it is answered before the owner link: the text is already in this process. +- An entry opens at its first change, not line 1: every path that changes what is being read goes + through `ViewerSession.Open`, so none resets to row 0 on its own. The minimal view ("Changes + only", `SessionState.Minimal`) is a second `DiffView` built with each entry - changes plus + `DiffView.Context` rows either side, longer unchanged runs folded into one `RowKind.Folded` row. + Scrolling, the scrollbar and navigation count rows of the view on screen; a selection stays in + rows of the entry (`ViewerSession.Drag` unfolds the head's rows), so it survives switching views + and a fold inside it copies what it stands for. Navigation is defined over where it lands a + change - `Context` rows under the top - which is what makes previous undo next. The fold kind and + the `m` key are additive ABI values with no `DEVIEW_VERSION` bump, as `DEVIEW_QUEUE_HEADER` was: + a stale library draws a fold as a plain row. - Queue tooltips are composed once in `QueueProjection`, not per head, and are **null when they would only repeat the row**. Labels are already the shortest distinguishing form, so the tip is what the label left off — path, test, frameworks, failure text. `QueueTooltipTests` snapshots the diff --git a/docs/mdsource/viewer.source.md b/docs/mdsource/viewer.source.md index 0caee6f5..be0ebbc1 100644 --- a/docs/mdsource/viewer.source.md +++ b/docs/mdsource/viewer.source.md @@ -70,7 +70,8 @@ Nothing is written to disk for inline review. The patch travels over stdin, or o | Key | Action | | --- | --- | | `Up` `Down` `PgUp` `PgDn` `Home` `End` | Scroll | -| `n` `p` | Next and previous change | +| `n` `p` | Next and previous change (also the **Next change** and **Prev change** buttons) | +| `m` | Show only the changes, or every line (also the **Changes only** button) | | `Tab` `Shift+Tab` | Next and previous pending item | | `a` | Accept | | `Shift+A` | Accept all | @@ -81,6 +82,15 @@ Nothing is written to disk for inline review. The patch travels over stdin, or o | `q` `Esc` | Close | +## Moving between changes + +A comparison opens scrolled to its first change, with three lines of context above it, rather than at line 1. **Next change** and **Prev change** move from one change to the next, putting each in the same place under the top of the pane, and each is disabled when no change is left in its direction. + +**Changes only** switches to a minimal view: each change with the three lines either side of it, and every longer run of unchanged lines folded into one row saying how many lines it stands for. The button then reads **All lines**, which switches back. Switching keeps the line being read where it is on screen, and the choice holds while moving through the queue. Images are never folded, because their rows are their properties and each is worth reading. + +A fold is only a view. A selection that spans one copies the lines it stands for, since those are what lies between the selection's two ends, and the status line names the stretch of the file on screen rather than a count of rows: sixteen rows in the minimal view can read `lines 1-30 of 40`. + + ## Selecting and copying Drag across either pane to select text, and `Ctrl+C` to copy it. `Ctrl+A` selects one whole pane: the one something is already selected in, or the received side when nothing is. On macOS the Edit menu carries both, so `Cmd+C` and `Cmd+A` work there too. diff --git a/docs/viewer.md b/docs/viewer.md index 7806896f..eb3c5c98 100644 --- a/docs/viewer.md +++ b/docs/viewer.md @@ -77,7 +77,8 @@ Nothing is written to disk for inline review. The patch travels over stdin, or o | Key | Action | | --- | --- | | `Up` `Down` `PgUp` `PgDn` `Home` `End` | Scroll | -| `n` `p` | Next and previous change | +| `n` `p` | Next and previous change (also the **Next change** and **Prev change** buttons) | +| `m` | Show only the changes, or every line (also the **Changes only** button) | | `Tab` `Shift+Tab` | Next and previous pending item | | `a` | Accept | | `Shift+A` | Accept all | @@ -88,6 +89,15 @@ Nothing is written to disk for inline review. The patch travels over stdin, or o | `q` `Esc` | Close | +## Moving between changes + +A comparison opens scrolled to its first change, with three lines of context above it, rather than at line 1. **Next change** and **Prev change** move from one change to the next, putting each in the same place under the top of the pane, and each is disabled when no change is left in its direction. + +**Changes only** switches to a minimal view: each change with the three lines either side of it, and every longer run of unchanged lines folded into one row saying how many lines it stands for. The button then reads **All lines**, which switches back. Switching keeps the line being read where it is on screen, and the choice holds while moving through the queue. Images are never folded, because their rows are their properties and each is worth reading. + +A fold is only a view. A selection that spans one copies the lines it stands for, since those are what lies between the selection's two ends, and the status line names the stretch of the file on screen rather than a count of rows: sixteen rows in the minimal view can read `lines 1-30 of 40`. + + ## Selecting and copying Drag across either pane to select text, and `Ctrl+C` to copy it. `Ctrl+A` selects one whole pane: the one something is already selected in, or the received side when nothing is. On macOS the Edit menu carries both, so `Cmd+C` and `Cmd+A` work there too. diff --git a/native/include/deview.h b/native/include/deview.h index 4fa0963c..884625c4 100644 --- a/native/include/deview.h +++ b/native/include/deview.h @@ -31,7 +31,13 @@ enum DeviewRowKind { DEVIEW_ROW_ADDED = 1, DEVIEW_ROW_REMOVED = 2, DEVIEW_ROW_MODIFIED = 3, - DEVIEW_ROW_FILLER = 4 + DEVIEW_ROW_FILLER = 4, + /* + * A run of unchanged lines the minimal view left out, as one row: its text says how many, and + * it has no line number. Drawn dimmed on a band of its own. A library built before this kind + * existed draws it as a plain unchanged row, which still reads. + */ + DEVIEW_ROW_FOLDED = 5 }; enum DeviewButtonFlags { @@ -47,7 +53,7 @@ enum DeviewQueueFlags { typedef struct DeviewRow { int32_t kind; - /* -1 when the row is filler and has no line number. */ + /* -1 when the row is filler or folded and has no line number. */ int32_t lineNumber; int32_t textOffset; int32_t textLength; @@ -184,7 +190,9 @@ enum DeviewKey { /* Ctrl+C, and Cmd+C on macOS. */ DEVIEW_KEY_COPY = 16, /* Ctrl+A, which is why plain A must be reported as accept only when no modifier is held. */ - DEVIEW_KEY_SELECT_ALL = 17 + DEVIEW_KEY_SELECT_ALL = 17, + /* M: every line, or only the changes and the lines around them. */ + DEVIEW_KEY_TOGGLE_MINIMAL = 18 }; typedef struct DeviewInput { diff --git a/native/src/deview.cpp b/native/src/deview.cpp index 2db13fbe..2b9b1734 100644 --- a/native/src/deview.cpp +++ b/native/src/deview.cpp @@ -241,6 +241,9 @@ ImU32 RowColour(int kind) return IM_COL32(233, 129, 129, 255); case DEVIEW_ROW_MODIFIED: return IM_COL32(231, 197, 113, 255); + /* Dimmed like the gutter, since what it says is about the file rather than from it. */ + case DEVIEW_ROW_FOLDED: + return IM_COL32(130, 130, 130, 255); default: return IM_COL32(212, 212, 212, 255); } @@ -256,6 +259,10 @@ ImU32 RowBackground(int kind) return IM_COL32(84, 40, 40, 255); case DEVIEW_ROW_MODIFIED: return IM_COL32(74, 64, 32, 255); + /* A shade lighter than filler, so a fold reads as a break in the file rather than as a + * line of it or as padding. */ + case DEVIEW_ROW_FOLDED: + return IM_COL32(34, 34, 34, 255); default: return 0; } @@ -540,6 +547,7 @@ int ReadKey() if (IsKeyPressed(KEY_END)) return DEVIEW_KEY_END; if (IsKeyPressed(KEY_N)) return DEVIEW_KEY_NEXT_CHANGE; if (IsKeyPressed(KEY_P)) return DEVIEW_KEY_PREVIOUS_CHANGE; + if (IsKeyPressed(KEY_M)) return DEVIEW_KEY_TOGGLE_MINIMAL; if (IsKeyPressed(KEY_TAB)) return IsKeyDown(KEY_LEFT_SHIFT) || IsKeyDown(KEY_RIGHT_SHIFT) ? DEVIEW_KEY_PREVIOUS_ITEM : DEVIEW_KEY_NEXT_ITEM; diff --git a/native/swift/Sources/Deview/MainMenu.swift b/native/swift/Sources/Deview/MainMenu.swift index 89826f11..11044945 100644 --- a/native/swift/Sources/Deview/MainMenu.swift +++ b/native/swift/Sources/Deview/MainMenu.swift @@ -105,6 +105,7 @@ enum MainMenu { .separator(), command("Next Change (n)", DEVIEW_KEY_NEXT_CHANGE, target), command("Previous Change (p)", DEVIEW_KEY_PREVIOUS_CHANGE, target), + command("Toggle Changes Only (m)", DEVIEW_KEY_TOGGLE_MINIMAL, target), .separator(), command("Next Pending (⇥)", DEVIEW_KEY_NEXT_ITEM, target), command("Previous Pending (⇧⇥)", DEVIEW_KEY_PREVIOUS_ITEM, target) diff --git a/native/swift/Sources/Deview/Palette.swift b/native/swift/Sources/Deview/Palette.swift index 410ed5d2..bfa6f954 100644 --- a/native/swift/Sources/Deview/Palette.swift +++ b/native/swift/Sources/Deview/Palette.swift @@ -33,6 +33,10 @@ enum Palette { static let rule = grey(70) + /// Behind a folded row, a shade lighter than filler, so the runs the minimal view leaves out + /// read as breaks in the file rather than as a line of it or as padding. + static let folded = grey(34) + /// ImGui draws a selected item as its accent at 31% over the window background. This is that /// composite, so the queue highlight matches without carrying an alpha channel around. static let selected = rgb(38, 64, 90) @@ -60,6 +64,9 @@ enum Palette { return rgb(233, 129, 129) case DEVIEW_ROW_MODIFIED.value: return rgb(231, 197, 113) + // Dimmed like the gutter, since what it says is about the file rather than from it. + case DEVIEW_ROW_FOLDED.value: + return dim default: return text } @@ -76,6 +83,8 @@ enum Palette { return rgb(74, 64, 32) case DEVIEW_ROW_FILLER.value: return filler + case DEVIEW_ROW_FOLDED.value: + return folded default: return nil } diff --git a/native/swift/Sources/Deview/Renderer.swift b/native/swift/Sources/Deview/Renderer.swift index 9aa7e3d5..3546d3f1 100644 --- a/native/swift/Sources/Deview/Renderer.swift +++ b/native/swift/Sources/Deview/Renderer.swift @@ -323,7 +323,8 @@ final class Renderer { return } - let number = String(row.lineNumber) + // A folded row has no number, and printing the -1 standing in for one put it in the gutter. + let number = row.lineNumber < 0 ? "" : String(row.lineNumber) let gutter = "\(Palette.marker(row.kind)) \(String(repeating: " ", count: max(0, 4 - number.count)))\(number)" let width = Renderer.gutterCells * cell.width diff --git a/native/swift/Sources/Deview/ViewerView.swift b/native/swift/Sources/Deview/ViewerView.swift index 96ab3a80..72b4208f 100644 --- a/native/swift/Sources/Deview/ViewerView.swift +++ b/native/swift/Sources/Deview/ViewerView.swift @@ -315,6 +315,8 @@ final class ViewerView: NSView, NSViewToolTipOwner { return DEVIEW_KEY_NEXT_CHANGE.value case "p": return DEVIEW_KEY_PREVIOUS_CHANGE.value + case "m": + return DEVIEW_KEY_TOGGLE_MINIMAL.value case "a": return shift ? DEVIEW_KEY_ACCEPT_ALL.value : DEVIEW_KEY_ACCEPT.value case "d": diff --git a/src/DiffEngine.Tests/ViewerClientUnownedTests.cs b/src/DiffEngine.Tests/ViewerClientUnownedTests.cs index e7f48cff..de65a449 100644 --- a/src/DiffEngine.Tests/ViewerClientUnownedTests.cs +++ b/src/DiffEngine.Tests/ViewerClientUnownedTests.cs @@ -13,8 +13,14 @@ public class ViewerClientUnownedTests static readonly ViewerMessage settle = new(ViewerVerb.Settle, InlineKey.For("Tests.cs", 1)); // Read before any test can have changed it, so the restore below puts back the real default - // rather than a copy of it kept here - static readonly TimeSpan recheckUnownedAfter = ViewerClient.RecheckUnownedAfter; + // rather than a copy of it kept here. A hook rather than a static field initializer: with no + // static constructor that runs on first touching a static field, and TheMemoryExpires sets the + // value before it touches one, so running first it captured Zero for every test after it. + static TimeSpan recheckUnownedAfter; + + [Before(Class)] + public static void Remember() => + recheckUnownedAfter = ViewerClient.RecheckUnownedAfter; [Before(Test)] public void Forget() => diff --git a/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so b/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so index e7e62c4d..79c8d649 100644 Binary files a/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so and b/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so differ diff --git a/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so b/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so index 6bcb719d..183f04e5 100644 Binary files a/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so and b/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so differ diff --git a/src/DiffEngineViewer.Mac/runtimes/osx-arm64/native/libdiffengine_viewer.dylib b/src/DiffEngineViewer.Mac/runtimes/osx-arm64/native/libdiffengine_viewer.dylib index 11d27714..5d30f898 100644 Binary files a/src/DiffEngineViewer.Mac/runtimes/osx-arm64/native/libdiffengine_viewer.dylib and b/src/DiffEngineViewer.Mac/runtimes/osx-arm64/native/libdiffengine_viewer.dylib differ diff --git a/src/DiffEngineViewer.Mac/runtimes/osx-x64/native/libdiffengine_viewer.dylib b/src/DiffEngineViewer.Mac/runtimes/osx-x64/native/libdiffengine_viewer.dylib index 11d27714..5d30f898 100644 Binary files a/src/DiffEngineViewer.Mac/runtimes/osx-x64/native/libdiffengine_viewer.dylib and b/src/DiffEngineViewer.Mac/runtimes/osx-x64/native/libdiffengine_viewer.dylib differ diff --git a/src/DiffEngineViewer.Tests/AcceptAllProgressTests.cs b/src/DiffEngineViewer.Tests/AcceptAllProgressTests.cs index 9d6837ad..ca83743d 100644 --- a/src/DiffEngineViewer.Tests/AcceptAllProgressTests.cs +++ b/src/DiffEngineViewer.Tests/AcceptAllProgressTests.cs @@ -176,10 +176,10 @@ public async Task TheWindowOffersNothingThatChangesTheQueueWhileItRuns() var screen = ScreenBuilder.Build(state); await Assert.That(screen.Status).IsEqualTo("Accepting 1 of 3"); - await Assert.That(screen.Buttons.Where(_ => _.Enabled)).IsEmpty(); + await Assert.That(EnabledQueueButtons(screen)).IsEmpty(); var window = new Window(); - foreach (var key in new[] { CommandKind.Accept, CommandKind.Discard, CommandKind.AcceptAll }) + foreach (var key in queueCommands) { var pressed = ViewerProgram.Apply(state, Input(key), null, window); await Assert.That(pressed.Queue).IsSameReferenceAs(state.Queue); @@ -206,7 +206,7 @@ public async Task AnAttachedWindowShowsTheOwnersProgress() var screen = ScreenBuilder.Build(state); await Assert.That(screen.Status).IsEqualTo("Accepting 5 of 9"); - await Assert.That(screen.Buttons.Where(_ => _.Enabled)).IsEmpty(); + await Assert.That(EnabledQueueButtons(screen)).IsEmpty(); // And the listing after the batch is what gives the window back var after = ViewerSession.Sync(state, Fixtures.Pending(Fixtures.Patch()), [], "Accepted 8", null); @@ -359,6 +359,15 @@ static SessionState Pending() => Fixtures.Patch("SampleTests.cs", 88, "\"one\"", "two"), Fixtures.Patch("OtherTests.cs", 12, null, "brand new")); + /// + /// What a batch refuses. Moving between changes and switching views only change what is being + /// read, so those buttons stay live while one runs. + /// + static readonly CommandKind[] queueCommands = [CommandKind.Accept, CommandKind.Discard, CommandKind.AcceptAll]; + + static IEnumerable