From ba3463bba4de27f61bd249a8bb427286c1e3423c Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sun, 4 Oct 2026 21:54:45 +1100 Subject: [PATCH] Fold the files derived from a document beneath it in the viewer A snapshot library that splits a document into files (a png and the text of each page, a csv per sheet) can say so: DiffRunner.LaunchDerived and AddDerivedDelete name the received file of the pending source a file was derived from. When the viewer is drawing that source as a document, the derived files open no tool, cost nothing against MaxInstancesToLaunch, and are shown beneath the document's row, which accepts or discards them with it. In every other case each file is launched exactly as before, so other diff tools are unaffected. The marker is additive on all three wires: a source line on move, diff and delete requests, a derived line on listings, and an optional Source property on the tray's Move and Delete payloads. An older tray, viewer or library yields ordinary rows, never a lost file. --- claude.md | 41 ++ docs/diff-tool.md | 14 +- docs/mdsource/diff-tool.source.md | 2 + docs/mdsource/tray.source.md | 11 + docs/mdsource/viewer.source.md | 33 +- docs/tray.md | 35 +- docs/viewer.md | 33 +- readme.md | 27 +- readme.source.md | 11 + .../BinaryCompatibilityTests.cs | 89 +++ src/DiffEngine.Tests/DiffRunnerTests.cs | 22 + .../InlineQueueClientTests.cs | 4 +- src/DiffEngine.Tests/PendingFilesDiffTests.cs | 270 ++++++++- src/DiffEngine.Tests/ViewerProtocolTests.cs | 195 ++++++- src/DiffEngine.Tests/diffTools.include.md | 12 +- src/DiffEngine/DiffRunner.cs | 239 +++++++- src/DiffEngine/Protocol/IQueueOwner.cs | 9 +- src/DiffEngine/Protocol/ViewerMessage.cs | 29 +- .../Protocol/ViewerMessageHandler.cs | 22 +- src/DiffEngine/Protocol/ViewerResponse.cs | 72 ++- src/DiffEngine/Tray/PendingFiles.cs | 145 ++++- src/DiffEngine/Tray/PiperClient.cs | 78 ++- src/DiffEngine/Viewer/ViewerDocuments.cs | 14 + .../DebugReportTests.Derived.verified.txt | 42 ++ src/DiffEngineTray.Tests/DebugReportTests.cs | 37 ++ .../OwnedInlineHostTest.cs | 19 +- .../PiperTest.DeleteJson.verified.txt | 11 +- ...iperTest.DeleteWithSourceJson.verified.txt | 5 + .../PiperTest.MoveWithSourceJson.verified.txt | 10 + src/DiffEngineTray.Tests/PiperTest.cs | 85 ++- src/DiffEngineTray.Tests/SerializerTests.cs | 70 +++ .../TrackerMoveOntoDeleteTest.cs | 2 +- src/DiffEngineTray.Tests/TrackerSourceTest.cs | 206 +++++++ .../TrackerTrackedFilesTest.cs | 6 +- src/DiffEngineTray/DebugReport.cs | 18 + src/DiffEngineTray/ITrackedFiles.cs | 10 +- src/DiffEngineTray/OwnedInlineHost.cs | 8 +- src/DiffEngineTray/Payloads/DeletePayload.cs | 5 +- src/DiffEngineTray/Payloads/MovePayload.cs | 9 +- src/DiffEngineTray/Program.cs | 16 +- src/DiffEngineTray/TrackedDelete.cs | 13 +- src/DiffEngineTray/TrackedMove.cs | 12 +- src/DiffEngineTray/Tracker.cs | 121 +++- .../DerivedAcceptTests.cs | 474 +++++++++++++++ .../DerivedFilesTests.cs | 551 ++++++++++++++++++ .../DerivedScreenTests.Folded.verified.txt | 24 + ...ScreenTests.MenuOnTheDocument.verified.txt | 24 + ...dScreenTests.ReadingOneOfThem.verified.txt | 24 + .../DerivedScreenTests.Tooltips.verified.txt | 21 + .../DerivedScreenTests.Unfolded.verified.txt | 24 + .../DerivedScreenTests.cs | 58 ++ src/DiffEngineViewer.Tests/Fixtures.cs | 119 +++- .../ImageScreenTests.cs | 2 + src/DiffEngineViewer.Tests/PixelTests.cs | 1 + src/DiffEngineViewer.Tests/ReEnqueueTests.cs | 2 + .../TrackedLabelTests.cs | 2 + .../TrackedWatchTests.cs | 6 +- src/DiffEngineViewer/AcceptBatch.cs | 11 + src/DiffEngineViewer/CommandKind.cs | 7 + .../Documents/DocumentWatch.cs | 36 +- src/DiffEngineViewer/Ipc/MessageHandler.cs | 22 +- src/DiffEngineViewer/Ipc/OwnerLink.cs | 21 +- src/DiffEngineViewer/MenuState.cs | 35 +- src/DiffEngineViewer/QueueEntry.cs | 27 +- src/DiffEngineViewer/QueueProjection.cs | 371 +++++++++++- src/DiffEngineViewer/ScreenBuilder.cs | 7 +- src/DiffEngineViewer/SessionState.cs | 17 + src/DiffEngineViewer/TrackedEntry.cs | 62 +- src/DiffEngineViewer/ViewerProgram.cs | 91 ++- src/DiffEngineViewer/ViewerSession.cs | 306 +++++++++- todo.md | 4 + 71 files changed, 4249 insertions(+), 212 deletions(-) create mode 100644 src/DiffEngineTray.Tests/DebugReportTests.Derived.verified.txt create mode 100644 src/DiffEngineTray.Tests/PiperTest.DeleteWithSourceJson.verified.txt create mode 100644 src/DiffEngineTray.Tests/PiperTest.MoveWithSourceJson.verified.txt create mode 100644 src/DiffEngineTray.Tests/TrackerSourceTest.cs create mode 100644 src/DiffEngineViewer.Tests/DerivedAcceptTests.cs create mode 100644 src/DiffEngineViewer.Tests/DerivedFilesTests.cs create mode 100644 src/DiffEngineViewer.Tests/DerivedScreenTests.Folded.verified.txt create mode 100644 src/DiffEngineViewer.Tests/DerivedScreenTests.MenuOnTheDocument.verified.txt create mode 100644 src/DiffEngineViewer.Tests/DerivedScreenTests.ReadingOneOfThem.verified.txt create mode 100644 src/DiffEngineViewer.Tests/DerivedScreenTests.Tooltips.verified.txt create mode 100644 src/DiffEngineViewer.Tests/DerivedScreenTests.Unfolded.verified.txt create mode 100644 src/DiffEngineViewer.Tests/DerivedScreenTests.cs diff --git a/claude.md b/claude.md index 4e31e128e..762343e05 100644 --- a/claude.md +++ b/claude.md @@ -737,6 +737,47 @@ the comment there about not caching "nothing staged" asks for. `Application.Run()`. - Allows accepting/discarding diffs from system tray +**Source and derived files (`DiffRunner.LaunchDerived`, `QueueProjection.Derivation`):** +- A snapshot library that splits a document into files (Verify's converters: a png and the text + of each page, a csv per sheet) says so. A derived pending file names its **source** by the + source's received path: one level, only while the source is itself pending, and the caller + launches the source first. Not "group", which here already means solution. +- The marker is additive on all three wires and is only ever relayed. `source:` on a `move`, + `diff` or `delete` request (`ViewerMessage.Source`); a `derived: key|source key` line of its + own on a listing (`ViewerResponseMove.SourceKey`), as `held:` is and for its reason; and a + trailing `"Source"` on the piper's Move and Delete payloads, which are byte for byte what they + were without it. A new payload `Type` would be dropped whole by an older tray, where a property + it does not know is skipped and the file still tracked. `IQueueOwner.TrackMove` and + `TrackDelete` take the source as a required argument, so no owner can leave it out. +- The policy is the library's and has one condition, `PendingFiles.Draws`: the tool resolved for + the source is the viewer, the source is a document, and that copy reads documents + (`ViewerDocuments.ReadBy`). Then `InnerLaunch` tracks the derived pair and answers + `AlreadyRunningAndSupportsRefresh` before a tool is resolved for it: no window, nothing against + `MaxInstance`, and no empty target written for a tool that would have required one. In every + other case the launch is the one it always was, with the source said to whatever tracks the + pair. That is what keeps the other diff tools working, and why the tray decides none of it. + `PendingFiles.AddDerived` gives a tray the viewer's exe and `RelaunchFor` arguments, so the + file counts as open and "Accept all open" takes it. Nobody to take it falls through to the + ordinary launch. +- The tray stores and relays (`TrackedMove.Source`, `TrackedDelete.Source`), and its menu is + unchanged. A source changing makes a new tracked object, since `ITrackedFiles.Version` is their + identity. `Tracker.UntrackDerivedFrom` drops the derived moves of a settled document whose + received files have gone, which would otherwise be ordinary rows until the next scan. +- The fold is the viewer's and is a view (`QueueProjection.Derivation`): D sits beneath S when + `D.SourceKey == S.Key`, S is a move naming no source of its own, `S.IsDocument`, and the two + share a solution. Anything else is an ordinary row, which is the degrade for an older owner, a + document accepted on its own, and a viewer with no documents folder. `Order` puts attached + entries straight after their source, `Walk` gives them a slot only while the source's key is in + `SessionState.Unfolded`, and the selection never rests on an entry with no row (`Seen`). Labels + only, so no head and no ABI changed: the menu's Expand and Collapse, and a click on the row of + the document already selected (`ViewerProgram.ClickEntry`). +- A window's accept or discard of a document with files beneath it is a batch over them all + (`ViewerSession.BeginAcceptWithDerived`, `AcceptBatch.Cascade`), the document last + (`FilesInBatchOrder`): taken first it left its files behind as ordinary rows, one frame each. + The batch's rules stand, so a file that fails is kept and counted. An accept by key over the + wire is still one entry, as asked; an attached viewer sends the derived files as a group accept + and then the document (`ViewerProgram.DispatchWithDerived`). + **Packaging.Tests (`src/Packaging.Tests/`):** - Opens each `.nupkg` a Release build drops in `nugets` and snapshots its entry list, plus a few invariants a snapshot states poorly: an apphost with no assembly beside it, a viewer file in the diff --git a/docs/diff-tool.md b/docs/diff-tool.md index 86cc74e14..75ef36c2c 100644 --- a/docs/diff-tool.md +++ b/docs/diff-tool.md @@ -54,6 +54,8 @@ This value can be changed using an environment variable or by explicitly specify The count includes [DiffEngineViewer](/docs/viewer.md), but only when a viewer has to be started. Handing a pair to one already on screen opens no window and so spends nothing. +Neither does a file [derived from a document](/docs/viewer.md#files-derived-from-a-document) the viewer is drawing: it is shown beneath the document rather than in a tool of its own. A document split into a file per page would otherwise spend the whole allowance on one test. + ### Using an environment variable @@ -319,9 +321,9 @@ DiffTools.UseOrder(DiffTool.DiffEngineViewer); * Scanned paths: * `%USERPROFILE%\.dotnet\tools\DiffEngineViewer.exe` * `%USERPROFILE%\.dotnet\tools\.store\diffenginetray\*\diffenginetray\*\tools\*\any\viewer\win-x64\DiffEngineViewer.exe` - * `%NUGET_PACKAGES%\diffengine\20.6.0\tools\viewer\win-x64\DiffEngineViewer.exe` + * `%NUGET_PACKAGES%\diffengine\20.7.0-beta.1\tools\viewer\win-x64\DiffEngineViewer.exe` * `%NUGET_PACKAGES%\diffengine\*\tools\viewer\win-x64\DiffEngineViewer.exe` - * `%USERPROFILE%\.nuget\packages\diffengine\20.6.0\tools\viewer\win-x64\DiffEngineViewer.exe` + * `%USERPROFILE%\.nuget\packages\diffengine\20.7.0-beta.1\tools\viewer\win-x64\DiffEngineViewer.exe` * `%USERPROFILE%\.nuget\packages\diffengine\*\tools\viewer\win-x64\DiffEngineViewer.exe` * `%PATH%DiffEngineViewer.exe` @@ -337,9 +339,9 @@ DiffTools.UseOrder(DiffTool.DiffEngineViewer); ``` * Scanned paths: * `%HOME%/.dotnet/tools/DiffEngineViewer` - * `%NUGET_PACKAGES%/diffengine/20.6.0/tools/viewer/osx-x64/DiffEngineViewer` + * `%NUGET_PACKAGES%/diffengine/20.7.0-beta.1/tools/viewer/osx-x64/DiffEngineViewer` * `%NUGET_PACKAGES%/diffengine/*/tools/viewer/osx-x64/DiffEngineViewer` - * `%HOME%/.nuget/packages/diffengine/20.6.0/tools/viewer/osx-x64/DiffEngineViewer` + * `%HOME%/.nuget/packages/diffengine/20.7.0-beta.1/tools/viewer/osx-x64/DiffEngineViewer` * `%HOME%/.nuget/packages/diffengine/*/tools/viewer/osx-x64/DiffEngineViewer` * `%PATH%DiffEngineViewer` @@ -355,9 +357,9 @@ DiffTools.UseOrder(DiffTool.DiffEngineViewer); ``` * Scanned paths: * `%HOME%/.dotnet/tools/DiffEngineViewer` - * `%NUGET_PACKAGES%/diffengine/20.6.0/tools/viewer/linux-x64/DiffEngineViewer` + * `%NUGET_PACKAGES%/diffengine/20.7.0-beta.1/tools/viewer/linux-x64/DiffEngineViewer` * `%NUGET_PACKAGES%/diffengine/*/tools/viewer/linux-x64/DiffEngineViewer` - * `%HOME%/.nuget/packages/diffengine/20.6.0/tools/viewer/linux-x64/DiffEngineViewer` + * `%HOME%/.nuget/packages/diffengine/20.7.0-beta.1/tools/viewer/linux-x64/DiffEngineViewer` * `%HOME%/.nuget/packages/diffengine/*/tools/viewer/linux-x64/DiffEngineViewer` * `%PATH%DiffEngineViewer` diff --git a/docs/mdsource/diff-tool.source.md b/docs/mdsource/diff-tool.source.md index 99f4fc032..84acbf4fe 100644 --- a/docs/mdsource/diff-tool.source.md +++ b/docs/mdsource/diff-tool.source.md @@ -47,6 +47,8 @@ This value can be changed using an environment variable or by explicitly specify The count includes [DiffEngineViewer](/docs/viewer.md), but only when a viewer has to be started. Handing a pair to one already on screen opens no window and so spends nothing. +Neither does a file [derived from a document](/docs/viewer.md#files-derived-from-a-document) the viewer is drawing: it is shown beneath the document rather than in a tool of its own. A document split into a file per page would otherwise spend the whole allowance on one test. + ### Using an environment variable diff --git a/docs/mdsource/tray.source.md b/docs/mdsource/tray.source.md index ccf4ba07b..4f8b08191 100644 --- a/docs/mdsource/tray.source.md +++ b/docs/mdsource/tray.source.md @@ -190,6 +190,17 @@ snippet: PiperTest.MoveJson.verified.txt snippet: PiperTest.DeleteJson.verified.txt +### Derived from another file + +A move or a delete of a file that was [derived from a document](/docs/viewer.md#files-derived-from-a-document) names the temp file of the pending move it was derived from, in a `Source` property that is absent otherwise: + +snippet: PiperTest.MoveWithSourceJson.verified.txt + +snippet: PiperTest.DeleteWithSourceJson.verified.txt + +The tray tracks such a file as it tracks any other, so it is listed in the menu and taken by an accept-all. The property is only passed on to the viewer, which is what folds the file beneath its document. A tray older than the property ignores it. + + ## Logging Directory Beside the installed tool, so it moves with the target framework the tray is built for: diff --git a/docs/mdsource/viewer.source.md b/docs/mdsource/viewer.source.md index 88d290edc..27f27c163 100644 --- a/docs/mdsource/viewer.source.md +++ b/docs/mdsource/viewer.source.md @@ -202,6 +202,7 @@ Every row of the pending column answers a right-click: * An inline snapshot offers **Accept**, **Discard** and **Open source file**, plus **Show next variant** when frameworks disagree about it. * A move offers **Accept move**, **Discard** and **Open target directory**; a delete offers **Accept delete**, **Discard** and **Open directory**. + * A document with [files derived from it](#files-derived-from-a-document) counts them in the first two, **Accept move +5** and **Discard +5**, and adds **Expand** or **Collapse**. * A solution header offers **Accept all in ...** and **Discard all in ...** for that solution only, and a test sub-header the same for that test's changes. Bulk accepts skip conflicted snapshots, the way accept-all does. * Every entry also offers **Copy selection** when there is one, and a **Copy** item per pane, named after that pane, which copies the whole side. A side with nothing in it — the expected side of a brand new snapshot, or what is left after a delete — gets no item rather than one that copies nothing. @@ -236,7 +237,7 @@ When [DiffEngineTray](/docs/tray.md) owns the queue, the viewer also lists the t **Accept all** on a tray-owned queue sweeps everything the window shows: deletes, moves and snapshots, with conflicted snapshots skipped and anything locked kept pending and counted. The deletes are the ones pending when it began, and are held back when a snapshot was not written. One for a file that a move in the same sweep has written is left pending. -A viewer that owns the queue itself never shows moves or deletes, because DiffEngine only sends them to a running tray. +A viewer that owns the queue itself is sent them directly when [no tray is running](#with-no-tray), and shows them the same way. ## Images @@ -307,6 +308,36 @@ An SVG is drawn with scripts, external images and external elements turned off. DiffEngine offers the viewer for `.pdf`, `.docx`, `.xlsx`, `.pptx` and the map extensions only when the copy it resolved carries the folder. The viewer is last in the default tool order, so Word, Excel, Beyond Compare or DeltaWalker are still preferred where installed. +### Files derived from a document + +A snapshot library often verifies more than the document itself: a png of each page, the text read out of it, a csv per sheet. Each is a received file of its own, and each would be a row of its own, asking for one change to be accepted again once per file, when the document's row has already shown its pages and its text. + +So a caller can say that a file was derived from a document, with `DiffRunner.LaunchDerived` in place of `DiffRunner.Launch`. [Verify](https://github.com/VerifyTests/Verify) does, for what its converters split out of a document. While the viewer is drawing that document, a derived file opens no tool of its own, spends nothing against [MaxInstancesToLaunch](/docs/diff-tool.md#maxinstancestolaunch), and has no row. It is counted on the document's: + +``` ++ Sample.Test (pdf) (5) +``` + +Accepting the document accepts the files beneath it as well, the document last, and the button and the menu say how many: **Accept move +5**. **Discard +5** discards them the same way. A file that could not be written stays pending and the closing message counts it, as an accept-all does. A page the document no longer has is a pending delete beneath it, carried out when the document is accepted. **Accept all** counts and takes every file, folded or not. + +The row's marker is the one a header has. **Expand** in the row's right-click menu, or a click on the row once it is the one selected, gives each derived file a row under the document, named by what it adds to the document's name: + +``` +- Sample.Test (pdf) (5) + (txt) + #page_0001 (png) + #page_0001 (txt) + #page_0002 (png) + #page_0003.verified.png +``` + +Selected, a derived file is the ordinary pair it also is, and its **Accept move** takes that file alone. `Tab` steps over the files of a folded document, a folded document's row carries the `!` of a failure beneath it, and anything that selects a derived file from outside the window unfolds its document. + +Folding needs the document on screen as a document. A derived file is an ordinary row when its document is not pending, when the copy of the viewer running has no `documents` folder, or when the document was accepted on its own from the tray's menu. When the document went to another tool, such as Word or Beyond Compare, each derived file is opened in its own tool as it always has been. The viewer is last in the default [tool order](/docs/diff-tool.order.md), so on a machine with one of those installed it has to be ordered first for a document to reach it. + +What is accepted unseen is accepted on the strength of the viewer's own drawing of the document, which is not necessarily the renderer that produced the page files. Expanding the row shows them. + + ### Maps Maps are read and drawn with [GeoConvert](https://github.com/Papyrine/GeoConvert). Each is one picture, as an SVG is, so there are no pages to turn, and the status line says whether the two draw the same. diff --git a/docs/tray.md b/docs/tray.md index b0ac0f292..71dbb43e6 100644 --- a/docs/tray.md +++ b/docs/tray.md @@ -212,18 +212,49 @@ The case it exists for is a test suite that needs the launch to happen but does ```txt { +"Type":"Delete", +"File":"theFilePath" +} +``` +snippet source | anchor + + + +### Derived from another file + +A move or a delete of a file that was [derived from a document](/docs/viewer.md#files-derived-from-a-document) names the temp file of the pending move it was derived from, in a `Source` property that is absent otherwise: + + + +```txt +{ "Type":"Move", "Temp":"theTempFilePath", "Target":"theTargetFilePath", "CanKill":true, "Exe":"theExePath", "Arguments":"TheArguments", -"ProcessId":1000 +"ProcessId":1000, +"Source":"theSourceTempFilePath" +} +``` +snippet source | anchor + + + + +```txt +{ +"Type":"Delete", +"File":"theFilePath", +"Source":"theSourceTempFilePath" } ``` -snippet source | anchor +snippet source | anchor +The tray tracks such a file as it tracks any other, so it is listed in the menu and taken by an accept-all. The property is only passed on to the viewer, which is what folds the file beneath its document. A tray older than the property ignores it. + ## Logging Directory diff --git a/docs/viewer.md b/docs/viewer.md index d9b4037b7..e50605517 100644 --- a/docs/viewer.md +++ b/docs/viewer.md @@ -209,6 +209,7 @@ Every row of the pending column answers a right-click: * An inline snapshot offers **Accept**, **Discard** and **Open source file**, plus **Show next variant** when frameworks disagree about it. * A move offers **Accept move**, **Discard** and **Open target directory**; a delete offers **Accept delete**, **Discard** and **Open directory**. + * A document with [files derived from it](#files-derived-from-a-document) counts them in the first two, **Accept move +5** and **Discard +5**, and adds **Expand** or **Collapse**. * A solution header offers **Accept all in ...** and **Discard all in ...** for that solution only, and a test sub-header the same for that test's changes. Bulk accepts skip conflicted snapshots, the way accept-all does. * Every entry also offers **Copy selection** when there is one, and a **Copy** item per pane, named after that pane, which copies the whole side. A side with nothing in it — the expected side of a brand new snapshot, or what is left after a delete — gets no item rather than one that copies nothing. @@ -243,7 +244,7 @@ When [DiffEngineTray](/docs/tray.md) owns the queue, the viewer also lists the t **Accept all** on a tray-owned queue sweeps everything the window shows: deletes, moves and snapshots, with conflicted snapshots skipped and anything locked kept pending and counted. The deletes are the ones pending when it began, and are held back when a snapshot was not written. One for a file that a move in the same sweep has written is left pending. -A viewer that owns the queue itself never shows moves or deletes, because DiffEngine only sends them to a running tray. +A viewer that owns the queue itself is sent them directly when [no tray is running](#with-no-tray), and shows them the same way. ## Images @@ -314,6 +315,36 @@ An SVG is drawn with scripts, external images and external elements turned off. DiffEngine offers the viewer for `.pdf`, `.docx`, `.xlsx`, `.pptx` and the map extensions only when the copy it resolved carries the folder. The viewer is last in the default tool order, so Word, Excel, Beyond Compare or DeltaWalker are still preferred where installed. +### Files derived from a document + +A snapshot library often verifies more than the document itself: a png of each page, the text read out of it, a csv per sheet. Each is a received file of its own, and each would be a row of its own, asking for one change to be accepted again once per file, when the document's row has already shown its pages and its text. + +So a caller can say that a file was derived from a document, with `DiffRunner.LaunchDerived` in place of `DiffRunner.Launch`. [Verify](https://github.com/VerifyTests/Verify) does, for what its converters split out of a document. While the viewer is drawing that document, a derived file opens no tool of its own, spends nothing against [MaxInstancesToLaunch](/docs/diff-tool.md#maxinstancestolaunch), and has no row. It is counted on the document's: + +``` ++ Sample.Test (pdf) (5) +``` + +Accepting the document accepts the files beneath it as well, the document last, and the button and the menu say how many: **Accept move +5**. **Discard +5** discards them the same way. A file that could not be written stays pending and the closing message counts it, as an accept-all does. A page the document no longer has is a pending delete beneath it, carried out when the document is accepted. **Accept all** counts and takes every file, folded or not. + +The row's marker is the one a header has. **Expand** in the row's right-click menu, or a click on the row once it is the one selected, gives each derived file a row under the document, named by what it adds to the document's name: + +``` +- Sample.Test (pdf) (5) + (txt) + #page_0001 (png) + #page_0001 (txt) + #page_0002 (png) + #page_0003.verified.png +``` + +Selected, a derived file is the ordinary pair it also is, and its **Accept move** takes that file alone. `Tab` steps over the files of a folded document, a folded document's row carries the `!` of a failure beneath it, and anything that selects a derived file from outside the window unfolds its document. + +Folding needs the document on screen as a document. A derived file is an ordinary row when its document is not pending, when the copy of the viewer running has no `documents` folder, or when the document was accepted on its own from the tray's menu. When the document went to another tool, such as Word or Beyond Compare, each derived file is opened in its own tool as it always has been. The viewer is last in the default [tool order](/docs/diff-tool.order.md), so on a machine with one of those installed it has to be ordered first for a document to reach it. + +What is accepted unseen is accepted on the strength of the viewer's own drawing of the document, which is not necessarily the renderer that produced the page files. Expanding the row shows them. + + ### Maps Maps are read and drawn with [GeoConvert](https://github.com/Papyrine/GeoConvert). Each is one picture, as an SVG is, so there are no pages to turn, and the status line says whether the two draw the same. diff --git a/readme.md b/readme.md index 4ce5e685d..e355c757f 100644 --- a/readme.md +++ b/readme.md @@ -40,6 +40,7 @@ DiffEngine manages launching and cleanup of diff tools. It is designed to be use * [NuGet](#nuget) * [Supported Tools](#supported-tools) * [Launching a tool](#launching-a-tool) + * [Files derived from another](#files-derived-from-another) * [Closing a tool](#closing-a-tool) * [File type detection](#file-type-detection) * [BuildServerDetector](#buildserverdetector) @@ -109,6 +110,30 @@ await DiffRunner.LaunchAsync(tempFile, targetFile); Note that this method will respect the above [difference behavior](/docs/diff-tool.md#detected-difference-behavior) in terms of Auto refresh and MDI behaviors. +### Files derived from another + +A snapshot of a document is often several files: the document, and what was computed from it, such as a png of each page, its text, or a csv per sheet. A file of the second kind can be launched as derived from the first: + + + +```cs +// The document first: what is derived from it names it +await DiffRunner.LaunchAsync(documentTempFile, documentTargetFile); + +// A file computed from the document, named by the document's temp file +await DiffRunner.LaunchDerivedAsync(pageTempFile, pageTargetFile, documentTempFile, null); + +// A file the document no longer produces +await DiffRunner.AddDerivedDeleteAsync(stalePageFile, documentTempFile); +``` +snippet source | anchor + + +The source is named by its temp file, is launched first, and is named only while it is itself pending. + +This changes nothing for most tools: each derived file is launched exactly as `Launch` would launch it. The exception is [DiffEngineViewer](/docs/viewer.md#files-derived-from-a-document) when it is drawing the source as a document. It already shows the pages and the text, so the derived files open no tool, do not count towards [MaxInstancesToLaunch](/docs/diff-tool.md#maxinstancestolaunch), and are accepted or discarded together with the document. + + ## Closing a tool A tool can be closed using the following: @@ -118,7 +143,7 @@ A tool can be closed using the following: ```cs DiffRunner.Kill(file1, file2); ``` -snippet source | anchor +snippet source | anchor Note that this method will respect the above [difference behavior](/docs/diff-tool.md#detected-difference-behavior) in terms of MDI behavior. diff --git a/readme.source.md b/readme.source.md index 00e355706..23206ecf6 100644 --- a/readme.source.md +++ b/readme.source.md @@ -42,6 +42,17 @@ snippet: DiffRunnerLaunch Note that this method will respect the above [difference behavior](/docs/diff-tool.md#detected-difference-behavior) in terms of Auto refresh and MDI behaviors. +### Files derived from another + +A snapshot of a document is often several files: the document, and what was computed from it, such as a png of each page, its text, or a csv per sheet. A file of the second kind can be launched as derived from the first: + +snippet: DiffRunnerLaunchDerived + +The source is named by its temp file, is launched first, and is named only while it is itself pending. + +This changes nothing for most tools: each derived file is launched exactly as `Launch` would launch it. The exception is [DiffEngineViewer](/docs/viewer.md#files-derived-from-a-document) when it is drawing the source as a document. It already shows the pages and the text, so the derived files open no tool, do not count towards [MaxInstancesToLaunch](/docs/diff-tool.md#maxinstancestolaunch), and are accepted or discarded together with the document. + + ## Closing a tool A tool can be closed using the following: diff --git a/src/DiffEngine.Tests/BinaryCompatibilityTests.cs b/src/DiffEngine.Tests/BinaryCompatibilityTests.cs index 0199f9f8a..28d79002d 100644 --- a/src/DiffEngine.Tests/BinaryCompatibilityTests.cs +++ b/src/DiffEngine.Tests/BinaryCompatibilityTests.cs @@ -35,4 +35,93 @@ public async Task InlineStagingClearFrom20_4() [typeof(string), typeof(int), typeof(string), typeof(string), typeof(string)]); await Assert.That(method).IsNotNull(); } + + /// + /// What every consumer launches and tracks a pending file through. The launches for a file + /// derived from another were added beside these, under names of their own, precisely so that + /// none of these gained a parameter. + /// + [Test] + [Arguments(nameof(DiffRunner.Launch))] + [Arguments(nameof(DiffRunner.LaunchAsync))] + [Arguments(nameof(DiffRunner.LaunchForText))] + [Arguments(nameof(DiffRunner.LaunchForTextAsync))] + public async Task LaunchByPath(string name) + { + var method = typeof(DiffRunner).GetMethod( + name, + [typeof(string), typeof(string), typeof(Encoding)]); + await Assert.That(method).IsNotNull(); + } + + [Test] + [Arguments(nameof(DiffRunner.AddDelete))] + [Arguments(nameof(DiffRunner.AddDeleteAsync))] + [Arguments(nameof(DiffRunner.SettleDelete))] + public async Task ByFile(string name) + { + var method = typeof(DiffRunner).GetMethod( + name, + [typeof(string)]); + await Assert.That(method).IsNotNull(); + } + + [Test] + public async Task Kill() + { + var method = typeof(DiffRunner).GetMethod( + nameof(DiffRunner.Kill), + [typeof(string), typeof(string)]); + await Assert.That(method).IsNotNull(); + } + + /// + /// The launches a snapshot library makes for a file derived from another, from the release + /// they shipped in. With no optional parameter among them, so a parameter one of them needs + /// later is an overload beside it and not a change to it. + /// + [Test] + [Arguments(nameof(DiffRunner.LaunchDerived))] + [Arguments(nameof(DiffRunner.LaunchDerivedAsync))] + [Arguments(nameof(DiffRunner.LaunchDerivedForText))] + [Arguments(nameof(DiffRunner.LaunchDerivedForTextAsync))] + public async Task LaunchDerived(string name) + { + var method = typeof(DiffRunner).GetMethod( + name, + [typeof(string), typeof(string), typeof(string), typeof(Encoding)]); + await Assert.That(method).IsNotNull(); + await Assert.That(method!.GetParameters().Any(_ => _.IsOptional)).IsFalse(); + } + + [Test] + [Arguments(nameof(DiffRunner.AddDerivedDelete))] + [Arguments(nameof(DiffRunner.AddDerivedDeleteAsync))] + public async Task AddDerivedDelete(string name) + { + var method = typeof(DiffRunner).GetMethod( + name, + [typeof(string), typeof(string)]); + await Assert.That(method).IsNotNull(); + await Assert.That(method!.GetParameters().Any(_ => _.IsOptional)).IsFalse(); + } + + /// + /// The obsolete shim is public too, and compiled against by whatever predates + /// tracking for itself. + /// + [Test] +#pragma warning disable CS0618 // Type or member is obsolete + public async Task TheTrayShim() + { + var addMove = typeof(DiffEngineTray).GetMethod( + nameof(DiffEngineTray.AddMove), + [typeof(string), typeof(string), typeof(string), typeof(string), typeof(bool), typeof(int?)]); + var addDelete = typeof(DiffEngineTray).GetMethod( + nameof(DiffEngineTray.AddDelete), + [typeof(string)]); +#pragma warning restore CS0618 + await Assert.That(addMove).IsNotNull(); + await Assert.That(addDelete).IsNotNull(); + } } diff --git a/src/DiffEngine.Tests/DiffRunnerTests.cs b/src/DiffEngine.Tests/DiffRunnerTests.cs index f29ddde6d..bc426a47a 100644 --- a/src/DiffEngine.Tests/DiffRunnerTests.cs +++ b/src/DiffEngine.Tests/DiffRunnerTests.cs @@ -98,6 +98,28 @@ static async Task Launch() #endregion } + static async Task LaunchDerived() + { + var documentTargetFile = ""; + var documentTempFile = ""; + var pageTargetFile = ""; + var pageTempFile = ""; + var stalePageFile = ""; + + #region DiffRunnerLaunchDerived + + // The document first: what is derived from it names it + await DiffRunner.LaunchAsync(documentTempFile, documentTargetFile); + + // A file computed from the document, named by the document's temp file + await DiffRunner.LaunchDerivedAsync(pageTempFile, pageTargetFile, documentTempFile, null); + + // A file the document no longer produces + await DiffRunner.AddDerivedDeleteAsync(stalePageFile, documentTempFile); + + #endregion + } + [Test] [Skip("Explicit")] public async Task KillAsync() diff --git a/src/DiffEngine.Tests/InlineQueueClientTests.cs b/src/DiffEngine.Tests/InlineQueueClientTests.cs index 4957c02fe..76c8af4c8 100644 --- a/src/DiffEngine.Tests/InlineQueueClientTests.cs +++ b/src/DiffEngine.Tests/InlineQueueClientTests.cs @@ -377,11 +377,11 @@ void IQueueOwner.Settle(string key, string? origin, string? member, string? valu } } - void IQueueOwner.TrackMove(string temp, string target) + void IQueueOwner.TrackMove(string temp, string target, string? source) { } - void IQueueOwner.TrackDelete(string file) + void IQueueOwner.TrackDelete(string file, string? source) { } diff --git a/src/DiffEngine.Tests/PendingFilesDiffTests.cs b/src/DiffEngine.Tests/PendingFilesDiffTests.cs index d0459a29a..a02491a63 100644 --- a/src/DiffEngine.Tests/PendingFilesDiffTests.cs +++ b/src/DiffEngine.Tests/PendingFilesDiffTests.cs @@ -2,6 +2,8 @@ // lives, and these tests have to hold it down. #pragma warning disable CS0618 +using System.Diagnostics.CodeAnalysis; + /// /// The route a pair takes when the diff tool resolved for it is the viewer itself: queued with /// whoever owns the queue rather than given a process and a window of its own. @@ -214,6 +216,261 @@ public async Task AnMdiToolIsNotKillableEither() await Assert.That(canKill).IsFalse(); } + /// + /// A page of a document the viewer is drawing is on screen already, in that document, so it is + /// tracked with what it was derived from and nothing is opened for it. Not even resolved: the + /// tool handed in here throws if it is asked for, and a cap of zero would refuse a launch. + /// + [Test] + public async Task AFileDerivedFromADocumentTheViewerDrawsIsTrackedAndNothingIsOpened() + { + using var owner = new Recording(); + using var enabled = new Enabled(); + DiffRunner.MaxInstancesToLaunch(0); + MaxInstance.ResetCount(); + try + { + var result = await DiffRunner.InnerLaunchAsync( + NeverResolved, + Page, + PageTarget, + null, + Document, + Resolves(Viewer(documents: true))); + + await Assert.That(result).IsEqualTo(LaunchResult.AlreadyRunningAndSupportsRefresh); + } + finally + { + MaxInstance.ResetAppDomainValue(); + MaxInstance.ResetCount(); + } + + await Assert.That(owner.Heard).IsEquivalentTo([$"{ViewerVerb.Move}:{Page}:{PageTarget} from {Document}"]); + } + + [Test] + public async Task ASyncDerivedFileIsTrackedTheSameWay() + { + using var owner = new Recording(); + using var enabled = new Enabled(); + + var result = DiffRunner.InnerLaunch( + NeverResolved, + Page, + PageTarget, + null, + Document, + Resolves(Viewer(documents: true))); + + await Assert.That(result).IsEqualTo(LaunchResult.AlreadyRunningAndSupportsRefresh); + await Assert.That(owner.Heard).IsEquivalentTo([$"{ViewerVerb.Move}:{Page}:{PageTarget} from {Document}"]); + } + + /// + /// Nobody answering means the document is in no viewer either, since its own send went first + /// and waited for one. So the page is opened as any pair is, which here finds no tool for it. + /// What matters is that it was asked: tracked beneath a document nobody is showing, it would + /// be a pending file with no window and no row. + /// + [Test] + public async Task WithNobodyShowingTheDocumentADerivedFileIsOpenedAsAnyOther() + { + using var absent = new NoOwner(); + using var enabled = new Enabled(); + + var result = await DiffRunner.InnerLaunchAsync( + NoTool, + Page, + PageTarget, + null, + Document, + Resolves(Viewer(documents: true))); + + await Assert.That(result).IsEqualTo(LaunchResult.NoDiffToolFound); + } + + /// + /// A document that went to Word, or Beyond Compare, or anything that is not a viewer drawing + /// it, leaves its pages to be opened as they always were. What they were derived from is + /// still said, to whoever tracks them. + /// + [Test] + public async Task ADocumentInAnotherToolLeavesItsDerivedFilesToTheirOwnTools() + { + using var owner = new Recording(); + using var enabled = new Enabled(); + + var result = await DiffRunner.InnerLaunchAsync( + NoTool, + Page, + PageTarget, + null, + Document, + Resolves(Other(isMdi: false))); + + await Assert.That(result).IsEqualTo(LaunchResult.NoDiffToolFound); + await Assert.That(owner.Heard).IsEquivalentTo([$"{ViewerVerb.Move}:{Page}:{PageTarget} from {Document}"]); + } + + /// + /// And so does a viewer with no documents folder, which shows the document as nothing it can + /// read: the copy bundled in the package, for one. Where the page's own tool is that viewer, + /// the page is shown, as a pair of its own, and says what it was derived from. + /// + [Test] + public async Task AViewerThatCannotDrawTheDocumentShowsItsDerivedFilesAsPairs() + { + using var owner = new Recording(); + using var enabled = new Enabled(); + var viewer = Viewer(documents: false); + + var result = await DiffRunner.InnerLaunchAsync( + Resolves(viewer), + Page, + PageTarget, + null, + Document, + Resolves(viewer)); + + await Assert.That(result).IsEqualTo(LaunchResult.AlreadyRunningAndSupportsRefresh); + await Assert.That(owner.Heard).IsEquivalentTo([$"{ViewerVerb.Diff}:{Page}:{PageTarget} from {Document}"]); + } + + /// + /// Launching turned off turns off the launch, not the tracking, as for every pair. Nothing + /// was opened for the document either, so its page is tracked as any pair is, source said. + /// + [Test] + public async Task WhileDisabledADerivedFileIsStillTracked() + { + using var owner = new Recording(); + var previousDisabled = DiffRunner.Disabled; + DiffRunner.Disabled = true; + try + { + var result = await DiffRunner.InnerLaunchAsync( + NeverResolved, + Page, + PageTarget, + null, + Document, + Resolves(Viewer(documents: true))); + + await Assert.That(result).IsEqualTo(LaunchResult.Disabled); + } + finally + { + DiffRunner.Disabled = previousDisabled; + } + + await Assert.That(owner.Heard).IsEquivalentTo([$"{ViewerVerb.Move}:{Page}:{PageTarget} from {Document}"]); + } + + /// + /// What the viewer draws is decided by the copy that resolved and by the file: a copy with its + /// documents folder, and a file that folder reads. An SVG is the one it reaches as the text + /// tool, so the extension that resolved the viewer for it says nothing either way. + /// + [Test] + [Arguments(@"c:\temp\a.received.pdf", true, true)] + [Arguments(@"c:\temp\a.received.docx", true, true)] + [Arguments(@"c:\temp\a.received.svg", true, true)] + [Arguments(@"c:\temp\a.received.pdf", false, false)] + [Arguments(@"c:\temp\a.received.svg", false, false)] + [Arguments(@"c:\temp\a.received.html", true, false)] + [Arguments(@"c:\temp\a.received.png", true, false)] + public async Task AViewerDrawsWhatItsDocumentsFolderReads(string file, bool documents, bool expected) => + await Assert.That(PendingFiles.Draws(Viewer(documents), file)).IsEqualTo(expected); + + [Test] + public async Task AnotherToolDrawsNothing() => + await Assert.That(PendingFiles.Draws(Other(isMdi: false), Document)).IsFalse(); + + /// + /// A page a document no longer has: the delete of its verified file, saying which document. + /// + [Test] + public async Task ADerivedDeleteReachesTheOwnerWithItsSource() + { + using var owner = new Recording(); + using var enabled = new Enabled(); + + DiffRunner.AddDerivedDelete(Stale, Document); + await DiffRunner.AddDerivedDeleteAsync(Stale, Document); + + await Assert.That(owner.Heard).IsEquivalentTo( + [ + $"{ViewerVerb.Delete}:{Stale}: from {Document}", + $"{ViewerVerb.Delete}:{Stale}: from {Document}" + ]); + } + + /// + /// And an ordinary delete says nothing of one, as it never has. + /// + [Test] + public async Task ADeleteWithNoSourceSaysNothingOfOne() + { + using var owner = new Recording(); + using var enabled = new Enabled(); + + DiffRunner.AddDelete(Stale); + + await Assert.That(owner.Heard).IsEquivalentTo([$"{ViewerVerb.Delete}:{Stale}:"]); + } + + /// + /// A file is not derived from itself, and one that says it is would be hidden beneath an + /// entry that is not there. Said by the library, so no owner has to guard against it. + /// + [Test] + public async Task AFileNamingItselfAsItsSourceHasNone() + { + using var owner = new Recording(); + using var enabled = new Enabled(); + + DiffRunner.AddDerivedDelete(Stale, Stale); + + await Assert.That(owner.Heard).IsEquivalentTo([$"{ViewerVerb.Delete}:{Stale}:"]); + } + + const string Document = @"c:\temp\Sample.Test.received.pdf"; + const string Page = @"c:\temp\Sample.Test#page_0001.received.png"; + const string PageTarget = @"c:\code\Sample.Test#page_0001.verified.png"; + + static DiffRunner.TryResolveTool Resolves(ResolvedTool tool) => + ([NotNullWhen(true)] out ResolvedTool? resolved) => + { + resolved = tool; + return true; + }; + + static bool NoTool([NotNullWhen(true)] out ResolvedTool? resolved) + { + resolved = null; + return false; + } + + static bool NeverResolved([NotNullWhen(true)] out ResolvedTool? resolved) => + throw new("A file shown beneath its source has no tool to resolve."); + + /// + /// Launching switched on for a test, and put back after it. DisabledChecker turns it off for + /// build servers and AI CLIs, and these drive the real launch path. + /// + sealed class Enabled : + IDisposable + { + readonly bool previous = DiffRunner.Disabled; + + public Enabled() => + DiffRunner.Disabled = false; + + public void Dispose() => + DiffRunner.Disabled = previous; + } + static ResolvedTool Other(bool isMdi) => new( name: "Fake", @@ -235,7 +492,11 @@ static ResolvedTool Other(bool isMdi) => /// /// Carries the identity the route branches on. Never started: an owner answers every time. /// - static ResolvedTool Viewer() => + /// + /// Whether it is a copy with its documents folder, which is said by the extensions it was + /// resolved with: a copy that has one is given the routed ones. + /// + static ResolvedTool Viewer(bool documents = false) => new( name: nameof(DiffTool.DiffEngineViewer), tool: DiffTool.DiffEngineViewer, @@ -245,7 +506,7 @@ static ResolvedTool Viewer() => Right: (temp, target) => $"\"{temp}\" \"{target}\""), isMdi: false, autoRefresh: false, - binaryExtensions: [], + binaryExtensions: documents ? DocumentExtensions.Routed : [], requiresTarget: false, supportsText: true, useShellExecute: false); @@ -288,7 +549,10 @@ public Recording() { lock (Heard) { - Heard.Add($"{message.Verb}:{message.Key}:{message.Body}"); + // Nothing where there is none, so what a file with no source is heard as + // stays what the tests from before there were sources assert + var from = message.Source is null ? "" : $" from {message.Source}"; + Heard.Add($"{message.Verb}:{message.Key}:{message.Body}{from}"); } if (message.Verb == Refuse) diff --git a/src/DiffEngine.Tests/ViewerProtocolTests.cs b/src/DiffEngine.Tests/ViewerProtocolTests.cs index 4a40c9b56..a0b9cb4b3 100644 --- a/src/DiffEngine.Tests/ViewerProtocolTests.cs +++ b/src/DiffEngine.Tests/ViewerProtocolTests.cs @@ -413,6 +413,113 @@ public async Task AHoldIsOptionalAndMatchedByKey() await Assert.That(ViewerResponse.TryParse(text.Replace(holdLine, "held: only-one-field"), out _)).IsFalse(); } + /// + /// What a move or a delete was derived from is a line of its own, as a hold is and for its + /// reason: the lines it belongs to keep their five and four fields, so a reader that predates + /// it reads the listing as it always has. One line name for both, since a key says which of + /// the two it is. + /// + [Test] + public async Task ADerivedFileNamesItsSourceOnALineOfItsOwn() + { + var listing = ViewerResponse.Listing( + [], + moves: + [ + new(@"move:c:\temp\a.received.pdf", "a (pdf)", null, @"c:\temp\a.received.pdf", @"c:\code\a.verified.pdf"), + new(@"move:c:\temp\a#page_0001.received.png", "a#page_0001 (png)", null, @"c:\temp\a#page_0001.received.png", @"c:\code\a#page_0001.verified.png") + { + SourceKey = @"move:c:\temp\a.received.pdf" + } + ], + deletes: + [ + new(@"delete:c:\code\a#page_0002.verified.png", "a#page_0002.verified.png", null, @"c:\code\a#page_0002.verified.png") + { + SourceKey = @"move:c:\temp\a.received.pdf" + } + ]); + + var text = listing.Build(); + await Assert.That(Fields(text, "move: ").Select(_ => _.Length)).IsEquivalentTo([5, 5]); + await Assert.That(Fields(text, "delete: ").Single().Length).IsEqualTo(4); + await Assert.That(Fields(text, "derived: ").Select(_ => _.Length)).IsEquivalentTo([2, 2]); + await Assert.That(ViewerResponse.TryParse(text, out var parsed)).IsTrue(); + await Assert.That(parsed!.Moves.Select(_ => _.SourceKey)).IsEquivalentTo([null, @"move:c:\temp\a.received.pdf"]); + await Assert.That(parsed.Deletes.Single().SourceKey).IsEqualTo(@"move:c:\temp\a.received.pdf"); + } + + /// + /// Both directions of an older peer, as for a hold. An owner that predates the line sends + /// none, which reads as every file standing alone. And a line is matched to its file by key + /// once every line is in, so it need not follow it, and one for a file the listing does not + /// carry is dropped rather than refused. + /// + /// A source that is not in the listing is kept as it was said. Whether the source is pending + /// is the reader's to ask of the queue it has, and it changes from one listing to the next. + /// + /// + [Test] + public async Task ADerivedLineIsOptionalAndMatchedByKey() + { + var derived = new ViewerResponseMove("move:page", "page", null, "page", "target") + { + SourceKey = "move:gone" + }; + var text = ViewerResponse.Listing([], moves: [derived]).Build(); + var derivedLine = text.Split('\n').Single(_ => _.StartsWith("derived: ", StringComparison.Ordinal)); + + await Assert.That(ViewerResponse.TryParse(text, out var whole)).IsTrue(); + await Assert.That(whole!.Moves.Single().SourceKey).IsEqualTo("move:gone"); + + await Assert.That(ViewerResponse.TryParse(text.Replace($"{derivedLine}\n", ""), out var older)).IsTrue(); + await Assert.That(older!.Moves.Single().SourceKey).IsNull(); + + // Ahead of its move + var moveLine = text.Split('\n').Single(_ => _.StartsWith("move: ", StringComparison.Ordinal)); + var reordered = text + .Replace($"{derivedLine}\n", "") + .Replace($"{moveLine}\n", $"{derivedLine}\n{moveLine}\n"); + await Assert.That(ViewerResponse.TryParse(reordered, out var early)).IsTrue(); + await Assert.That(early!.Moves.Single().SourceKey).IsEqualTo("move:gone"); + + var orphan = ViewerResponse.Listing([]).Build() + derivedLine + "\n"; + await Assert.That(ViewerResponse.TryParse(orphan, out var none)).IsTrue(); + await Assert.That(none!.Moves).IsEmpty(); + } + + [Test] + public async Task AMalformedDerivedLineRejectsTheResponse() + { + var text = ViewerResponse.Listing( + [], + moves: + [ + new("move:page", "page", null, "page", "target") + { + SourceKey = "move:document" + } + ]).Build(); + var derivedLine = text.Split('\n').Single(_ => _.StartsWith("derived: ", StringComparison.Ordinal)); + + await Assert.That(ViewerResponse.TryParse(text.Replace(derivedLine, "derived: only-one-field"), out _)).IsFalse(); + } + + /// + /// A listing with nothing derived in it is the listing it has always been, to the byte: the + /// line is only there for a file that has a source. + /// + [Test] + public async Task AListingWithNothingDerivedSaysNothingOfIt() + { + var text = ViewerResponse.Listing( + [], + moves: [new("move:x", "x", null, "x", "y")], + deletes: [new("delete:z", "z", null, "z")]).Build(); + + await Assert.That(text).DoesNotContain("derived:"); + } + [Test] public async Task AListingWithoutTrackedItemsParsesEmpty() { @@ -602,6 +709,74 @@ await Assert.That(owner.Tracked).IsEquivalentTo( ]); } + /// + /// What a pending file was derived from rides the same three verbs, as a field of its own, and + /// reaches the owner beside the paths: a page of a document whose document is pending too. + /// + [Test] + public async Task ADerivedFileReachesTheOwnerWithItsSource() + { + var owner = new FakeOwner((true, null)); + const string source = @"c:\temp\a.received.pdf"; + + Send(owner, new(ViewerVerb.Move, @"c:\temp\a#page_0001.received.png", @"c:\code\a#page_0001.verified.png") + { + Source = source + }); + Send(owner, new(ViewerVerb.Diff, @"c:\temp\a#page_0002.received.png", @"c:\code\a#page_0002.verified.png") + { + Source = source + }); + Send(owner, new(ViewerVerb.Delete, @"c:\code\a#page_0003.verified.png") + { + Source = source + }); + + await Assert.That(owner.Tracked).IsEquivalentTo( + [ + @"move c:\temp\a#page_0001.received.png > c:\code\a#page_0001.verified.png from c:\temp\a.received.pdf", + @"move c:\temp\a#page_0002.received.png > c:\code\a#page_0002.verified.png from c:\temp\a.received.pdf", + @"delete c:\code\a#page_0003.verified.png from c:\temp\a.received.pdf" + ]); + + // Over the wire, as an owner in another process is sent it, rather than handed the record + static void Send(FakeOwner owner, ViewerMessage message) + { + if (!ViewerMessage.TryParse(message.Build(), out var parsed)) + { + throw new("The message did not survive its own format."); + } + + ViewerMessageHandler.Handle(owner, parsed); + } + } + + /// + /// A file with no source is sent as it always was, to the byte, so an owner from before the + /// field is sent nothing it has not always been sent. And one from before it reads past it: + /// see , which is that owner's half. + /// + [Test] + public async Task AMessageWithoutASourceBuildsAsBefore() + { + var text = new ViewerMessage(ViewerVerb.Move, "temp", "target").Build(); + + await Assert.That(text).IsEqualTo($"version: 1\nverb: move\nkey: {ViewerPayload.Encode("temp")}\nbody: {ViewerPayload.Encode("target")}\n"); + await Assert.That(ViewerMessage.TryParse(text, out var parsed)).IsTrue(); + await Assert.That(parsed!.Source).IsNull(); + } + + /// + /// An empty source names nothing, so it is none, rather than a file derived from the file + /// with no name. + /// + [Test] + public async Task AnEmptySourceIsNone() + { + await Assert.That(ViewerMessage.TryParse("version: 1\nverb: move\nsource: \n", out var parsed)).IsTrue(); + await Assert.That(parsed!.Source).IsNull(); + } + /// /// The pair whose diff tool is the viewer itself: tracked exactly as a move, and then raised, /// which is the whole difference between the two verbs. The focus names no entry, so the pair @@ -720,11 +895,23 @@ public void Settle(string key, string? origin, string? member, string? value) public List Tracked { get; } = []; - public void TrackMove(string temp, string target) => - Tracked.Add($"move {temp} > {target}"); + public void TrackMove(string temp, string target, string? source) => + Tracked.Add($"move {temp} > {target}{From(source)}"); + + public void TrackDelete(string file, string? source) => + Tracked.Add($"delete {file}{From(source)}"); - public void TrackDelete(string file) => - Tracked.Add($"delete {file}"); + // Nothing where there is none, so what a file with no source is recorded as stays what + // the tests from before there were sources assert + static string From(string? source) + { + if (source is null) + { + return ""; + } + + return $" from {source}"; + } public ViewerResponse Listing(bool withPatches) => ViewerResponse.Listing([]); diff --git a/src/DiffEngine.Tests/diffTools.include.md b/src/DiffEngine.Tests/diffTools.include.md index 93b248876..20b208844 100644 --- a/src/DiffEngine.Tests/diffTools.include.md +++ b/src/DiffEngine.Tests/diffTools.include.md @@ -178,9 +178,9 @@ DiffTools.UseOrder(DiffTool.DiffEngineViewer); * Scanned paths: * `%USERPROFILE%\.dotnet\tools\DiffEngineViewer.exe` * `%USERPROFILE%\.dotnet\tools\.store\diffenginetray\*\diffenginetray\*\tools\*\any\viewer\win-x64\DiffEngineViewer.exe` - * `%NUGET_PACKAGES%\diffengine\20.6.0\tools\viewer\win-x64\DiffEngineViewer.exe` + * `%NUGET_PACKAGES%\diffengine\20.7.0-beta.1\tools\viewer\win-x64\DiffEngineViewer.exe` * `%NUGET_PACKAGES%\diffengine\*\tools\viewer\win-x64\DiffEngineViewer.exe` - * `%USERPROFILE%\.nuget\packages\diffengine\20.6.0\tools\viewer\win-x64\DiffEngineViewer.exe` + * `%USERPROFILE%\.nuget\packages\diffengine\20.7.0-beta.1\tools\viewer\win-x64\DiffEngineViewer.exe` * `%USERPROFILE%\.nuget\packages\diffengine\*\tools\viewer\win-x64\DiffEngineViewer.exe` * `%PATH%DiffEngineViewer.exe` @@ -196,9 +196,9 @@ DiffTools.UseOrder(DiffTool.DiffEngineViewer); ``` * Scanned paths: * `%HOME%/.dotnet/tools/DiffEngineViewer` - * `%NUGET_PACKAGES%/diffengine/20.6.0/tools/viewer/osx-x64/DiffEngineViewer` + * `%NUGET_PACKAGES%/diffengine/20.7.0-beta.1/tools/viewer/osx-x64/DiffEngineViewer` * `%NUGET_PACKAGES%/diffengine/*/tools/viewer/osx-x64/DiffEngineViewer` - * `%HOME%/.nuget/packages/diffengine/20.6.0/tools/viewer/osx-x64/DiffEngineViewer` + * `%HOME%/.nuget/packages/diffengine/20.7.0-beta.1/tools/viewer/osx-x64/DiffEngineViewer` * `%HOME%/.nuget/packages/diffengine/*/tools/viewer/osx-x64/DiffEngineViewer` * `%PATH%DiffEngineViewer` @@ -214,9 +214,9 @@ DiffTools.UseOrder(DiffTool.DiffEngineViewer); ``` * Scanned paths: * `%HOME%/.dotnet/tools/DiffEngineViewer` - * `%NUGET_PACKAGES%/diffengine/20.6.0/tools/viewer/linux-x64/DiffEngineViewer` + * `%NUGET_PACKAGES%/diffengine/20.7.0-beta.1/tools/viewer/linux-x64/DiffEngineViewer` * `%NUGET_PACKAGES%/diffengine/*/tools/viewer/linux-x64/DiffEngineViewer` - * `%HOME%/.nuget/packages/diffengine/20.6.0/tools/viewer/linux-x64/DiffEngineViewer` + * `%HOME%/.nuget/packages/diffengine/20.7.0-beta.1/tools/viewer/linux-x64/DiffEngineViewer` * `%HOME%/.nuget/packages/diffengine/*/tools/viewer/linux-x64/DiffEngineViewer` * `%PATH%DiffEngineViewer` diff --git a/src/DiffEngine/DiffRunner.cs b/src/DiffEngine/DiffRunner.cs index 1b9634155..1c2918be0 100644 --- a/src/DiffEngine/DiffRunner.cs +++ b/src/DiffEngine/DiffRunner.cs @@ -172,6 +172,106 @@ public static LaunchResult Launch(ResolvedTool tool, string tempFile, string tar encoding); } + /// + /// Launch a diff tool for a file that was derived from another: a page of a document, the text + /// read out of one, anything a snapshot library computed from a source it is also verifying. + /// + /// is the received file of that source, exactly as it was + /// given as tempFile to the launch for it. Pass it only while the source is itself + /// pending, and launch the source first. + /// + /// + /// When the source went to DiffEngineViewer, and the viewer is drawing it as a document, no + /// tool is opened for this file: it is tracked, and the viewer shows it beneath the document + /// and accepts the two together. That is a pair handed to something already on screen, so it + /// is reported as , and costs + /// nothing against . In every other case - the source is + /// in Word or Beyond Compare, or in no tool at all - this is + /// , with the source said to whoever tracks + /// the pair. + /// + /// + /// A name of its own rather than an overload: three strings in a row beside the overloads + /// that take a tool read as one of those. + /// + /// + public static LaunchResult LaunchDerived(string tempFile, string targetFile, string sourceTempFile, Encoding? encoding) + { + GuardFiles(tempFile, targetFile); + Guard.AgainstEmpty(sourceTempFile, nameof(sourceTempFile)); + + return InnerLaunch( + ([NotNullWhen(true)] out tool) => + DiffTools.TryFindForInputFilePath(tempFile, out tool), + tempFile, + targetFile, + encoding, + SourceOf(tempFile, sourceTempFile)); + } + + /// + public static Task LaunchDerivedAsync(string tempFile, string targetFile, string sourceTempFile, Encoding? encoding) + { + GuardFiles(tempFile, targetFile); + Guard.AgainstEmpty(sourceTempFile, nameof(sourceTempFile)); + + return InnerLaunchAsync( + ([NotNullWhen(true)] out tool) => + DiffTools.TryFindForInputFilePath(tempFile, out tool), + tempFile, + targetFile, + encoding, + SourceOf(tempFile, sourceTempFile)); + } + + /// + /// for a file the caller knows + /// to be text whatever its extension says, as is to + /// . + /// + public static LaunchResult LaunchDerivedForText(string tempFile, string targetFile, string sourceTempFile, Encoding? encoding) + { + GuardFiles(tempFile, targetFile); + Guard.AgainstEmpty(sourceTempFile, nameof(sourceTempFile)); + + return InnerLaunch( + ([NotNullWhen(true)] out tool) => + DiffTools.TryFindForText(out tool), + tempFile, + targetFile, + encoding, + SourceOf(tempFile, sourceTempFile)); + } + + /// + public static Task LaunchDerivedForTextAsync(string tempFile, string targetFile, string sourceTempFile, Encoding? encoding) + { + GuardFiles(tempFile, targetFile); + Guard.AgainstEmpty(sourceTempFile, nameof(sourceTempFile)); + + return InnerLaunchAsync( + ([NotNullWhen(true)] out tool) => + DiffTools.TryFindForText(out tool), + tempFile, + targetFile, + encoding, + SourceOf(tempFile, sourceTempFile)); + } + + /// + /// A file is not derived from itself. Said here rather than left to whoever tracks it, where + /// an entry naming itself as its source would be hidden beneath an entry that is not there. + /// + static string? SourceOf(string file, string source) + { + if (InlineKey.SamePath(file, source)) + { + return null; + } + + return source; + } + public static void AddDelete(string file) { if (Disabled) @@ -179,7 +279,7 @@ public static void AddDelete(string file) return; } - DiffEngineTray.AddDelete(file); + PendingFiles.AddDelete(file); } public static Task AddDeleteAsync(string file) @@ -189,7 +289,40 @@ public static Task AddDeleteAsync(string file) return Task.CompletedTask; } - return DiffEngineTray.AddDeleteAsync(file); + return PendingFiles.AddDeleteAsync(file, Cancel.None); + } + + /// + /// for a file that was derived from another, which that other no + /// longer produces: a page a document has lost. is as on + /// , and the delete is raised + /// after the launch for the source, never before it. + /// + /// A viewer drawing the source shows the delete beneath it and carries it out when the source + /// is accepted. Anything else holds it as the ordinary delete it also is. + /// + /// + public static void AddDerivedDelete(string file, string sourceTempFile) + { + Guard.AgainstEmpty(sourceTempFile, nameof(sourceTempFile)); + if (Disabled) + { + return; + } + + PendingFiles.AddDelete(file, SourceOf(file, sourceTempFile)); + } + + /// + public static Task AddDerivedDeleteAsync(string file, string sourceTempFile) + { + Guard.AgainstEmpty(sourceTempFile, nameof(sourceTempFile)); + if (Disabled) + { + return Task.CompletedTask; + } + + return PendingFiles.AddDeleteAsync(file, Cancel.None, SourceOf(file, sourceTempFile)); } /// @@ -227,11 +360,39 @@ public static Task LaunchAsync(ResolvedTool tool, string tempFile, encoding); } - static LaunchResult InnerLaunch(TryResolveTool tryResolveTool, string tempFile, string targetFile, Encoding? encoding) + /// The tool for the pair. + /// The received file. + /// The file it belongs at. + /// For an empty target a tool needs written first. + /// + /// The received file of the pending move this pair was derived from, or null for a pair that + /// stands alone, which is every pair any caller but the derived launches sends. + /// + /// + /// The tool for . Null resolves it from its path, the way the launch + /// for the source itself did. Handed in by the tests, which have a stand-in for a viewer. + /// + internal static LaunchResult InnerLaunch( + TryResolveTool tryResolveTool, + string tempFile, + string targetFile, + Encoding? encoding, + string? source = null, + TryResolveTool? tryResolveSource = null) { + // Before the pair's own tool is resolved. A file shown beneath its source needs none, and + // resolving one writes an empty target for a tool that requires it, which the viewer + // would then draw the page against. + if (source is not null && + DrawnWithSource(source, tryResolveSource, out var viewer) && + PendingFiles.AddDerived(viewer, tempFile, targetFile, source)) + { + return LaunchResult.AlreadyRunningAndSupportsRefresh; + } + if (ShouldExitLaunch(tryResolveTool, targetFile, encoding, out var tool, out var result)) { - DiffEngineTray.AddMove(tempFile, targetFile, null, null, false, null); + PendingFiles.AddMove(tempFile, targetFile, null, null, false, null, source); return result.Value; } @@ -243,7 +404,7 @@ static LaunchResult InnerLaunch(TryResolveTool tryResolveTool, string tempFile, // this method. if (PendingFiles.IsViewer(tool)) { - return PendingFiles.AddDiff(tool, tempFile, targetFile); + return PendingFiles.AddDiff(tool, tempFile, targetFile, source); } tool.CommandAndArguments(tempFile, targetFile, out var arguments, out var command); @@ -254,7 +415,7 @@ static LaunchResult InnerLaunch(TryResolveTool tryResolveTool, string tempFile, { if (tool.AutoRefresh) { - DiffEngineTray.AddMove(tempFile, targetFile, tool.ExePath, arguments, canKill, processCommand.Process); + PendingFiles.AddMove(tempFile, targetFile, tool.ExePath, arguments, canKill, processCommand.Process, source); return LaunchResult.AlreadyRunningAndSupportsRefresh; } @@ -267,30 +428,45 @@ static LaunchResult InnerLaunch(TryResolveTool tryResolveTool, string tempFile, if (!replacing && MaxInstance.Reached()) { - DiffEngineTray.AddMove(tempFile, targetFile, tool.ExePath, arguments, canKill, null); + PendingFiles.AddMove(tempFile, targetFile, tool.ExePath, arguments, canKill, null, source); return LaunchResult.TooManyRunningDiffTools; } var processId = LaunchProcess(tool, arguments); ProcessCleanup.Track(command, processId); - DiffEngineTray.AddMove(tempFile, targetFile, tool.ExePath, arguments, canKill, processId); + PendingFiles.AddMove(tempFile, targetFile, tool.ExePath, arguments, canKill, processId, source); return LaunchResult.StartedNewInstance; } - static async Task InnerLaunchAsync(TryResolveTool tryResolveTool, string tempFile, string targetFile, Encoding? encoding) + /// + internal static async Task InnerLaunchAsync( + TryResolveTool tryResolveTool, + string tempFile, + string targetFile, + Encoding? encoding, + string? source = null, + TryResolveTool? tryResolveSource = null) { + // As above: a file shown beneath its source has no tool to resolve + if (source is not null && + DrawnWithSource(source, tryResolveSource, out var viewer) && + await PendingFiles.AddDerivedAsync(viewer, tempFile, targetFile, source, Cancel.None)) + { + return LaunchResult.AlreadyRunningAndSupportsRefresh; + } + if (ShouldExitLaunch(tryResolveTool, targetFile, encoding, out var tool, out var result)) { - await DiffEngineTray.AddMoveAsync(tempFile, targetFile, null, null, false, null); + await PendingFiles.AddMoveAsync(tempFile, targetFile, null, null, false, null, Cancel.None, source); return result.Value; } // As above: the viewer has no window of its own for this pair to reason about. if (PendingFiles.IsViewer(tool)) { - return await PendingFiles.AddDiffAsync(tool, tempFile, targetFile, Cancel.None); + return await PendingFiles.AddDiffAsync(tool, tempFile, targetFile, Cancel.None, source); } tool.CommandAndArguments(tempFile, targetFile, out var arguments, out var command); @@ -301,7 +477,7 @@ static async Task InnerLaunchAsync(TryResolveTool tryResolveTool, { if (tool.AutoRefresh) { - await DiffEngineTray.AddMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, processCommand.Process); + await PendingFiles.AddMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, processCommand.Process, Cancel.None, source); return LaunchResult.AlreadyRunningAndSupportsRefresh; } @@ -312,18 +488,51 @@ static async Task InnerLaunchAsync(TryResolveTool tryResolveTool, if (!replacing && MaxInstance.Reached()) { - await DiffEngineTray.AddMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, null); + await PendingFiles.AddMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, null, Cancel.None, source); return LaunchResult.TooManyRunningDiffTools; } var processId = LaunchProcess(tool, arguments); ProcessCleanup.Track(command, processId); - await DiffEngineTray.AddMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, processId); + await PendingFiles.AddMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, processId, Cancel.None, source); return LaunchResult.StartedNewInstance; } + /// + /// Whether the tool the source went to is a viewer drawing it as a document, and so already + /// showing what was derived from it: see . + /// + /// Resolved from the source's path, as its own launch resolved it, so the two agree about + /// where the source is. Not while launching is turned off: nothing was opened for the source + /// then, and the pair is tracked as every pair is. + /// + /// + static bool DrawnWithSource(string source, TryResolveTool? tryResolveSource, [NotNullWhen(true)] out ResolvedTool? viewer) + { + viewer = null; + if (Disabled || + !TryResolveSource(source, tryResolveSource, out var tool) || + !PendingFiles.Draws(tool, source)) + { + return false; + } + + viewer = tool; + return true; + } + + static bool TryResolveSource(string source, TryResolveTool? tryResolveSource, [NotNullWhen(true)] out ResolvedTool? tool) + { + if (tryResolveSource is null) + { + return DiffTools.TryFindForInputFilePath(source, out tool); + } + + return tryResolveSource(out tool); + } + static bool ShouldExitLaunch( TryResolveTool tryResolveTool, string targetFile, @@ -439,5 +648,5 @@ static void GuardFiles(string tempFile, string targetFile) Guard.AgainstEmpty(targetFile, nameof(targetFile)); } - delegate bool TryResolveTool([NotNullWhen(true)] out ResolvedTool? resolved); + internal delegate bool TryResolveTool([NotNullWhen(true)] out ResolvedTool? resolved); } diff --git a/src/DiffEngine/Protocol/IQueueOwner.cs b/src/DiffEngine/Protocol/IQueueOwner.cs index 85ee70f0f..35230010a 100644 --- a/src/DiffEngine/Protocol/IQueueOwner.cs +++ b/src/DiffEngine/Protocol/IQueueOwner.cs @@ -29,11 +29,16 @@ interface IQueueOwner /// port fills, which is what a tray started after the test process needs: that process's /// tray check is cached, so its moves come here for the rest of its life. /// + /// + /// is the received file of the pending move this one was derived + /// from, or null: see . A parameter with no default, so an + /// owner cannot track a file and quietly drop what it was derived from. + /// /// - void TrackMove(string temp, string target); + void TrackMove(string temp, string target, string? source); /// - void TrackDelete(string file); + void TrackDelete(string file, string? source); /// /// The whole listing response rather than just its items, because an owner answers with its diff --git a/src/DiffEngine/Protocol/ViewerMessage.cs b/src/DiffEngine/Protocol/ViewerMessage.cs index ec2b9b0e6..2d47ef273 100644 --- a/src/DiffEngine/Protocol/ViewerMessage.cs +++ b/src/DiffEngine/Protocol/ViewerMessage.cs @@ -29,6 +29,20 @@ record ViewerMessage(ViewerVerb Verb, string? Key = null, string? Body = null, s /// public const string Arrived = "arrived"; + /// + /// The received file of the pending move this file was derived from, on a + /// , a or a + /// : a page of a document, say, whose document is itself + /// pending. The path rather than a key, since it is what the sender has, and the owner keys + /// it the way it keys that move. + /// + /// An init property rather than a sixth positional one, so nothing that builds a message by + /// position has to say it does not have one. Optional, and read past by an owner that predates + /// it, which then tracks the file as it tracks any other. + /// + /// + public string? Source { get; init; } + public string Build() { var builder = new StringBuilder($"version: {ViewerPayload.Version}\n"); @@ -37,6 +51,7 @@ public string Build() ViewerPayload.Append(builder, "body", Body); ViewerPayload.Append(builder, "member", Member); ViewerPayload.Append(builder, "value", Value); + ViewerPayload.Append(builder, "source", Source); return builder.ToString(); } @@ -54,6 +69,7 @@ public static bool TryParse(string text, [NotNullWhen(true)] out ViewerMessage? string? body = null; string? member = null; string? settledBy = null; + string? source = null; foreach (var (name, value) in lines) { switch (name) @@ -95,6 +111,13 @@ public static bool TryParse(string text, [NotNullWhen(true)] out ViewerMessage? return false; } + continue; + case "source": + if (!ViewerPayload.TryDecode(value, out source)) + { + return false; + } + continue; default: // Unknown fields are ignored so a newer client can add one without breaking @@ -108,7 +131,11 @@ public static bool TryParse(string text, [NotNullWhen(true)] out ViewerMessage? return false; } - message = new(verb.Value, key, body, member, settledBy); + message = new(verb.Value, key, body, member, settledBy) + { + // An empty one names nothing, so it is none + Source = string.IsNullOrEmpty(source) ? null : source + }; return true; } } diff --git a/src/DiffEngine/Protocol/ViewerMessageHandler.cs b/src/DiffEngine/Protocol/ViewerMessageHandler.cs index 4bfb9a0fb..f1ca2ae89 100644 --- a/src/DiffEngine/Protocol/ViewerMessageHandler.cs +++ b/src/DiffEngine/Protocol/ViewerMessageHandler.cs @@ -18,11 +18,11 @@ public static ViewerResponse Handle(IQueueOwner owner, ViewerMessage message) case ViewerVerb.Settle: return Settle(owner, message.Key, message.Body, message.Member, message.Value); case ViewerVerb.Move: - return Move(owner, message.Key, message.Body); + return Move(owner, message.Key, message.Body, message.Source); case ViewerVerb.Diff: - return Diff(owner, message.Key, message.Body); + return Diff(owner, message.Key, message.Body, message.Source); case ViewerVerb.Delete: - return Delete(owner, message.Key); + return Delete(owner, message.Key, message.Source); case ViewerVerb.List: return owner.Listing(false); case ViewerVerb.ListFull: @@ -92,8 +92,12 @@ static ViewerResponse Settle(IQueueOwner owner, string? key, string? origin, str /// move is. What DiffEngine knows beside them — the diff tool it launched and that tool's /// process id — is the tray's kill machinery and means nothing to an owner that does not have /// any, so it is not sent. + /// + /// What it was derived from is sent, since that is about the file rather than about a tool: + /// see . + /// /// - static ViewerResponse Move(IQueueOwner owner, string? temp, string? target) + static ViewerResponse Move(IQueueOwner owner, string? temp, string? target, string? source) { if (temp is null || target is null) @@ -101,7 +105,7 @@ static ViewerResponse Move(IQueueOwner owner, string? temp, string? target) return ViewerResponse.Error("Move requires a key and a body"); } - owner.TrackMove(temp, target); + owner.TrackMove(temp, target, source); return ViewerResponse.Success(); } @@ -116,7 +120,7 @@ static ViewerResponse Move(IQueueOwner owner, string? temp, string? target) /// without a window - a tray - starts a viewer onto its queue. /// /// - static ViewerResponse Diff(IQueueOwner owner, string? temp, string? target) + static ViewerResponse Diff(IQueueOwner owner, string? temp, string? target, string? source) { if (temp is null || target is null) @@ -124,19 +128,19 @@ static ViewerResponse Diff(IQueueOwner owner, string? temp, string? target) return ViewerResponse.Error("Diff requires a key and a body"); } - owner.TrackMove(temp, target); + owner.TrackMove(temp, target, source); owner.Window(WindowCommand.Focus, null); return ViewerResponse.Success(); } - static ViewerResponse Delete(IQueueOwner owner, string? file) + static ViewerResponse Delete(IQueueOwner owner, string? file, string? source) { if (file is null) { return ViewerResponse.Error("Delete requires a key"); } - owner.TrackDelete(file); + owner.TrackDelete(file, source); return ViewerResponse.Success(); } diff --git a/src/DiffEngine/Protocol/ViewerResponse.cs b/src/DiffEngine/Protocol/ViewerResponse.cs index e26edbfd8..33102b91a 100644 --- a/src/DiffEngine/Protocol/ViewerResponse.cs +++ b/src/DiffEngine/Protocol/ViewerResponse.cs @@ -29,13 +29,30 @@ record ViewerResponseItem(string Key, string Name, string? Status, string? Patch /// A tracked file move riding a full listing, so a viewer displaying the tray's queue can render /// it from the two local paths and accept or discard it by key. /// -record ViewerResponseMove(string Key, string Name, string? Group, string Temp, string Target); +record ViewerResponseMove(string Key, string Name, string? Group, string Temp, string Target) +{ + /// + /// The key of the pending move this one was derived from, or null when it stands alone: a + /// page of a document whose document is pending too. A reader that draws the source as a + /// document shows this beneath it and accepts the two together. The key rather than the + /// path, so a reader matches it against and never has to build one. + /// + /// On a line of its own (derived) rather than a sixth field of move, which a + /// reader that predates it would refuse the whole listing over. That reader skips the line, + /// and an owner that predates it sends none, which reads as standing alone. + /// + /// + public string? SourceKey { get; init; } +} /// /// A tracked pending delete riding a full listing. /// record ViewerResponseDelete(string Key, string Name, string? Group, string File) { + /// + public string? SourceKey { get; init; } + /// /// Why the owner's accept-all would leave this delete pending, in words for whoever is looking /// at it, or null when it would carry it out: a move was accepted onto the file since the @@ -73,8 +90,8 @@ record ViewerResponse( string? WindowKey = null) { /// - /// The tray's tracked moves, on a full listing from a tray owner. A viewer that owns the - /// queue never has any: DiffEngine only sends moves and deletes to a running tray. + /// The owner's tracked moves, on a full listing: a tray's, or the ones a viewer that owns the + /// queue holds itself, which is where DiffEngine sends them when no tray is running. /// public IReadOnlyList Moves { get; init; } = []; @@ -189,6 +206,7 @@ public string Build() { var group = move.Group is null ? "" : ViewerPayload.Encode(move.Group); builder.Append($"move: {ViewerPayload.Encode(move.Key)}|{ViewerPayload.Encode(move.Name)}|{group}|{ViewerPayload.Encode(move.Temp)}|{ViewerPayload.Encode(move.Target)}\n"); + AppendDerived(builder, move.Key, move.SourceKey); } foreach (var delete in Deletes) @@ -199,11 +217,24 @@ public string Build() { builder.Append($"held: {ViewerPayload.Encode(delete.Key)}|{ViewerPayload.Encode(delete.Held)}\n"); } + + AppendDerived(builder, delete.Key, delete.SourceKey); } return builder.ToString(); } + // One line name for moves and deletes alike: their keys are prefixed, so a key says which it is + static void AppendDerived(StringBuilder builder, string key, string? sourceKey) + { + if (sourceKey is null) + { + return; + } + + builder.Append($"derived: {ViewerPayload.Encode(key)}|{ViewerPayload.Encode(sourceKey)}\n"); + } + public static bool TryParse(string text, [NotNullWhen(true)] out ViewerResponse? response) { response = null; @@ -226,6 +257,7 @@ public static bool TryParse(string text, [NotNullWhen(true)] out ViewerResponse? var deletes = new List(); Dictionary>? variants = null; Dictionary? holds = null; + Dictionary? sources = null; foreach (var (name, value) in lines) { switch (name) @@ -330,6 +362,16 @@ public static bool TryParse(string text, [NotNullWhen(true)] out ViewerResponse? holds ??= new(StringComparer.Ordinal); holds[heldKey] = reason; continue; + case "derived": + // The same two fields a hold is, so the same parse + if (!TryParseHeld(value, out var derivedKey, out var sourceKey)) + { + return false; + } + + sources ??= new(StringComparer.Ordinal); + sources[derivedKey] = sourceKey; + continue; default: continue; } @@ -366,6 +408,30 @@ public static bool TryParse(string text, [NotNullWhen(true)] out ViewerResponse? } } + // And as the holds are: by key, whichever of the two lists the key is in. A source that + // is not listed is kept, since whether it is pending is the reader's question to ask of + // the queue it has, and an empty one is none. + if (sources is not null) + { + for (var index = 0; index < moves.Count; index++) + { + if (sources.TryGetValue(moves[index].Key, out var sourceKey) && + sourceKey.Length > 0) + { + moves[index] = moves[index] with { SourceKey = sourceKey }; + } + } + + for (var index = 0; index < deletes.Count; index++) + { + if (sources.TryGetValue(deletes[index].Key, out var sourceKey) && + sourceKey.Length > 0) + { + deletes[index] = deletes[index] with { SourceKey = sourceKey }; + } + } + } + response = new(ok.Value, message, items, window, windowKey) { Moves = moves, diff --git a/src/DiffEngine/Tray/PendingFiles.cs b/src/DiffEngine/Tray/PendingFiles.cs index 14a01daac..c450c50f5 100644 --- a/src/DiffEngine/Tray/PendingFiles.cs +++ b/src/DiffEngine/Tray/PendingFiles.cs @@ -48,43 +48,62 @@ static class PendingFiles DiffEngineTray.IsRunning && !DiffRunner.TrayDisabled; - public static void AddDelete(string file) + /// + /// is the received file of the pending move this delete was derived + /// from, or null: a page a document no longer has, whose document is pending. It rides both + /// sends. It does not ride the launch, which has only a command line an older copy has to be + /// able to read: a delete that had to start its own viewer is held as an ordinary one. + /// + public static void AddDelete(string file, string? source = null) { if (TrayAvailable && - PiperClient.SendDelete(file)) + PiperClient.SendDelete(file, source)) { return; } - if (ViewerClient.TrySend(new(ViewerVerb.Delete, file))) + if (ViewerClient.TrySend(Delete(file, source))) { return; } ViewerLaunchGate.Launch( - () => ViewerClient.TrySend(new(ViewerVerb.Delete, file)), + () => ViewerClient.TrySend(Delete(file, source)), () => ViewerLauncher.LaunchDelete(file)); } - public static async Task AddDeleteAsync(string file, Cancel cancel) + /// + public static async Task AddDeleteAsync(string file, Cancel cancel, string? source = null) { if (TrayAvailable && - await PiperClient.SendDeleteAsync(file, cancel)) + await PiperClient.SendDeleteAsync(file, cancel, source)) { return; } - if (await ViewerClient.TrySendAsync(new(ViewerVerb.Delete, file), cancel)) + if (await ViewerClient.TrySendAsync(Delete(file, source), cancel)) { return; } await ViewerLaunchGate.LaunchAsync( - () => ViewerClient.TrySendAsync(new(ViewerVerb.Delete, file), cancel), + () => ViewerClient.TrySendAsync(Delete(file, source), cancel), () => Task.FromResult(ViewerLauncher.LaunchDelete(file)), cancel); } + static ViewerMessage Delete(string file, string? source) => + new(ViewerVerb.Delete, file) + { + Source = source + }; + + static ViewerMessage Pair(ViewerVerb verb, string tempFile, string targetFile, string? source) => + new(verb, tempFile, targetFile) + { + Source = source + }; + /// /// A failing pair whose resolved diff tool is the viewer itself. /// @@ -116,14 +135,19 @@ await ViewerLaunchGate.LaunchAsync( /// that viewer does not know the tray's files, so the focus can never find the key. The pair is /// tracked on both sides there rather than shown by neither. /// + /// + /// is what the pair was derived from, when the viewer is the tool + /// for a file whose source it is not drawing: it rides every send, and not the launch, as on + /// . + /// /// - public static LaunchResult AddDiff(ResolvedTool tool, string tempFile, string targetFile) + public static LaunchResult AddDiff(ResolvedTool tool, string tempFile, string targetFile, string? source = null) { // No process, and the arguments and CanKill from the one place that answers that, because // the tray works out the same two values for itself when a move arrives without them. var (arguments, canKill) = RelaunchFor(tool, tempFile, targetFile); if (TrayAvailable && - PiperClient.SendMove(tempFile, targetFile, tool.ExePath, arguments, canKill, null) && + PiperClient.SendMove(tempFile, targetFile, tool.ExePath, arguments, canKill, null, source) && ViewerClient.TrySend(new(ViewerVerb.Focus, TrackedKeys.ForMove(tempFile), ViewerMessage.Arrived))) { return LaunchResult.AlreadyRunningAndSupportsRefresh; @@ -131,16 +155,16 @@ public static LaunchResult AddDiff(ResolvedTool tool, string tempFile, string ta // A port recently found unowned is not asked again: the gate below probes for itself // before launching, and its probe corrects the memory when an owner has arrived since - if (ViewerClient.TrySend(new(ViewerVerb.Diff, tempFile, targetFile), out var response, skipIfUnowned: true)) + if (ViewerClient.TrySend(Pair(ViewerVerb.Diff, tempFile, targetFile, source), out var response, skipIfUnowned: true)) { return response.Ok ? LaunchResult.AlreadyRunningAndSupportsRefresh - : Refused(tempFile, targetFile); + : Refused(tempFile, targetFile, source); } return Launched( ViewerLaunchGate.Launch( - () => ViewerClient.TrySend(new(ViewerVerb.Diff, tempFile, targetFile)), + () => ViewerClient.TrySend(Pair(ViewerVerb.Diff, tempFile, targetFile, source)), () => ViewerLauncher.LaunchDiff(tempFile, targetFile))); } @@ -172,23 +196,23 @@ static LaunchResult Launched(ViewerLaunchOutcome outcome) => /// second viewer cannot change that answer and would bind nothing, so the pair goes over as a /// plain move: a row with nothing raised over it, which every owner has always understood. /// - static LaunchResult Refused(string tempFile, string targetFile) => - ViewerClient.TrySend(new(ViewerVerb.Move, tempFile, targetFile)) + static LaunchResult Refused(string tempFile, string targetFile, string? source) => + ViewerClient.TrySend(Pair(ViewerVerb.Move, tempFile, targetFile, source)) ? LaunchResult.AlreadyRunningAndSupportsRefresh : LaunchResult.NoDiffToolFound; /// - public static async Task AddDiffAsync(ResolvedTool tool, string tempFile, string targetFile, Cancel cancel) + public static async Task AddDiffAsync(ResolvedTool tool, string tempFile, string targetFile, Cancel cancel, string? source = null) { var (arguments, canKill) = RelaunchFor(tool, tempFile, targetFile); if (TrayAvailable && - await PiperClient.SendMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, null, cancel) && + await PiperClient.SendMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, null, cancel, source) && await ViewerClient.TrySendAsync(new(ViewerVerb.Focus, TrackedKeys.ForMove(tempFile), ViewerMessage.Arrived), cancel)) { return LaunchResult.AlreadyRunningAndSupportsRefresh; } - var outcome = await ViewerClient.SendAsync(new(ViewerVerb.Diff, tempFile, targetFile), cancel, skipIfUnowned: true); + var outcome = await ViewerClient.SendAsync(Pair(ViewerVerb.Diff, tempFile, targetFile, source), cancel, skipIfUnowned: true); if (outcome == SendOutcome.Accepted) { return LaunchResult.AlreadyRunningAndSupportsRefresh; @@ -196,18 +220,79 @@ await ViewerClient.TrySendAsync(new(ViewerVerb.Focus, TrackedKeys.ForMove(tempFi if (outcome == SendOutcome.Refused) { - return await ViewerClient.TrySendAsync(new(ViewerVerb.Move, tempFile, targetFile), cancel) + return await ViewerClient.TrySendAsync(Pair(ViewerVerb.Move, tempFile, targetFile, source), cancel) ? LaunchResult.AlreadyRunningAndSupportsRefresh : LaunchResult.NoDiffToolFound; } return Launched( await ViewerLaunchGate.LaunchAsync( - () => ViewerClient.TrySendAsync(new(ViewerVerb.Diff, tempFile, targetFile), cancel), + () => ViewerClient.TrySendAsync(Pair(ViewerVerb.Diff, tempFile, targetFile, source), cancel), () => Task.FromResult(ViewerLauncher.LaunchDiff(tempFile, targetFile)), cancel)); } + /// + /// Whether will show as a document: its text, + /// and its pages drawn. That is the viewer, a copy of it that has its documents folder, and a + /// file of a type that folder reads. + /// + /// It is what decides whether a file derived from needs a window of + /// its own (see ). A page of a document the viewer is drawing is + /// already on screen, beside the page it replaces. The same page beside a document Word or + /// Beyond Compare is showing, or beside a text file the viewer is showing as text, is not, and + /// is opened as it always has been. + /// + /// + public static bool Draws(ResolvedTool tool, string file) => + IsViewer(tool) && + DocumentExtensions.Is(file) && + ViewerDocuments.ReadBy(tool); + + /// + /// A pending file derived from a document the viewer is drawing: tracked, and nothing opened + /// for it. True when something took it, and false when nothing did, which leaves the caller + /// to open it as any other pair. + /// + /// Tracked as a pair the viewer shows, on both routes. To a tray that means the viewer's own + /// executable and the arguments sends, so the pair counts as open - the + /// window it is drawn in is on screen - and "accept all open" takes the pages with the + /// document rather than leaving them behind, and "open diff tool" raises the queue the row is + /// in. Without the focus follows that with: the document has the + /// window, and this row sits beneath it. + /// + /// + /// Never through the launch gate. The source went first, and its own send held the gate until + /// a viewer had the queue, so nobody answering here means the source is in no viewer either: + /// capped, failed to start, or closed since. A viewer started for a page could not be told + /// what the page was derived from, which is the only reason to start one for it. + /// + /// + public static bool AddDerived(ResolvedTool viewer, string tempFile, string targetFile, string source) + { + var (arguments, canKill) = RelaunchFor(viewer, tempFile, targetFile); + if (TrayAvailable && + PiperClient.SendMove(tempFile, targetFile, viewer.ExePath, arguments, canKill, null, source)) + { + return true; + } + + return ViewerClient.TrySend(Pair(ViewerVerb.Move, tempFile, targetFile, source)); + } + + /// + public static async Task AddDerivedAsync(ResolvedTool viewer, string tempFile, string targetFile, string source, Cancel cancel) + { + var (arguments, canKill) = RelaunchFor(viewer, tempFile, targetFile); + if (TrayAvailable && + await PiperClient.SendMoveAsync(tempFile, targetFile, viewer.ExePath, arguments, canKill, null, cancel, source)) + { + return true; + } + + return await ViewerClient.TrySendAsync(Pair(ViewerVerb.Move, tempFile, targetFile, source), cancel); + } + /// /// The other end of : the pair's test started passing, so the row it /// took goes. @@ -312,23 +397,30 @@ public static (string arguments, bool canKill) RelaunchFor(ResolvedTool tool, st return (tool.GetArguments(temp, target), !tool.IsMdi); } + /// + /// is the received file of the pending move this one was derived + /// from, or null. Here it is only said, to whoever tracks the pair: the tool that opened a + /// window for it has opened it already. + /// public static void AddMove( string tempFile, string targetFile, string? exe, string? arguments, bool canKill, - int? processId) + int? processId, + string? source = null) { if (TrayAvailable && - PiperClient.SendMove(tempFile, targetFile, exe, arguments, canKill, processId)) + PiperClient.SendMove(tempFile, targetFile, exe, arguments, canKill, processId, source)) { return; } - ViewerClient.TrySend(new(ViewerVerb.Move, tempFile, targetFile)); + ViewerClient.TrySend(Pair(ViewerVerb.Move, tempFile, targetFile, source)); } + /// public static async Task AddMoveAsync( string tempFile, string targetFile, @@ -336,14 +428,15 @@ public static async Task AddMoveAsync( string? arguments, bool canKill, int? processId, - Cancel cancel) + Cancel cancel, + string? source = null) { if (TrayAvailable && - await PiperClient.SendMoveAsync(tempFile, targetFile, exe, arguments, canKill, processId, cancel)) + await PiperClient.SendMoveAsync(tempFile, targetFile, exe, arguments, canKill, processId, cancel, source)) { return; } - await ViewerClient.TrySendAsync(new(ViewerVerb.Move, tempFile, targetFile), cancel); + await ViewerClient.TrySendAsync(Pair(ViewerVerb.Move, tempFile, targetFile, source), cancel); } } diff --git a/src/DiffEngine/Tray/PiperClient.cs b/src/DiffEngine/Tray/PiperClient.cs index 4899c680e..1ea3724ae 100644 --- a/src/DiffEngine/Tray/PiperClient.cs +++ b/src/DiffEngine/Tray/PiperClient.cs @@ -2,25 +2,45 @@ { public static int Port = 3492; - public static bool SendDelete(string file) => - Send(BuildDeletePayload(file)); + public static bool SendDelete(string file, string? source = null) => + Send(BuildDeletePayload(file, source)); public static Task SendDeleteAsync( string file, - Cancel cancel = default) + Cancel cancel = default, + string? source = null) { - var payload = BuildDeletePayload(file); + var payload = BuildDeletePayload(file, source); return SendAsync(payload, cancel); } - static string BuildDeletePayload(string file) => - $$""" - { - "Type":"Delete", - "File":"{{file.JsonEscape()}}" - } + /// + /// is the received file of the pending move this delete was derived + /// from: see , where it rides the same way. + /// + public static string BuildDeletePayload(string file, string? source = null) + { + // The payload every tray has always been sent, to the byte, when there is no source + if (source == null) + { + return $$""" + { + "Type":"Delete", + "File":"{{file.JsonEscape()}}" + } - """; + """; + } + + return $$""" + { + "Type":"Delete", + "File":"{{file.JsonEscape()}}", + "Source":"{{source.JsonEscape()}}" + } + + """; + } public static bool SendMove( string tempFile, @@ -28,8 +48,9 @@ public static bool SendMove( string? exe, string? arguments, bool canKill, - int? processId) => - Send(BuildMovePayload(tempFile, targetFile, exe, arguments, canKill, processId)); + int? processId, + string? source = null) => + Send(BuildMovePayload(tempFile, targetFile, exe, arguments, canKill, processId, source)); public static Task SendMoveAsync( string tempFile, @@ -38,13 +59,29 @@ public static Task SendMoveAsync( string? arguments, bool canKill, int? processId, - Cancel cancel = default) + Cancel cancel = default, + string? source = null) { - var payload = BuildMovePayload(tempFile, targetFile, exe, arguments, canKill, processId); + var payload = BuildMovePayload(tempFile, targetFile, exe, arguments, canKill, processId, source); return SendAsync(payload, cancel); } - public static string BuildMovePayload(string tempFile, string targetFile, string? exe, string? arguments, bool canKill, int? processId) + /// + /// is the received file of the pending move this one was derived + /// from, or null: a page of a document whose document is pending too. + /// + /// A property added to the payload a tray already reads, rather than a payload type of its + /// own, and that is the point of it. A tray from before it skips a property it has no member + /// for - as every tray skips Type, which it reads by substring - and tracks the move + /// as the ordinary one it also is. A type it did not know would be logged and dropped, and + /// this send is fire and forget: the file would be pending in nothing, with no way to tell. + /// + /// + /// Last, and absent rather than null when there is none, so the payload a move with no source + /// sends is the one it always has been. + /// + /// + public static string BuildMovePayload(string tempFile, string targetFile, string? exe, string? arguments, bool canKill, int? processId, string? source = null) { var builder = new StringBuilder( $$""" @@ -74,6 +111,15 @@ public static string BuildMovePayload(string tempFile, string targetFile, string """); } + if (source != null) + { + builder.Append( + $""" + , + "Source":"{source.JsonEscape()}" + """); + } + builder.AppendLine(); builder.Append('}'); return builder.ToString(); diff --git a/src/DiffEngine/Viewer/ViewerDocuments.cs b/src/DiffEngine/Viewer/ViewerDocuments.cs index f68b6d972..f08904ce1 100644 --- a/src/DiffEngine/Viewer/ViewerDocuments.cs +++ b/src/DiffEngine/Viewer/ViewerDocuments.cs @@ -33,4 +33,18 @@ public static bool Beside(string executable) Path.Combine(directory, ".store", "diffengineviewer.*", "*", "diffengineviewer.*", "*", "tools", "*", "any", "documents", assembly), out _); } + + /// + /// The same question of a viewer that has already been resolved, answered from what resolving + /// it concluded rather than by looking at the disk again: it was given the routed extensions + /// exactly when held. Asked once per file derived from a document, which + /// a test with a hundred pages asks a hundred times. + /// + /// It is also the only way to ask about a file the viewer reaches as the text tool. An SVG is + /// routed nowhere new, so the extension that resolved the viewer for it says nothing about + /// whether that copy draws it. + /// + /// + public static bool ReadBy(ResolvedTool viewer) => + viewer.BinaryExtensions.Contains(DocumentExtensions.Paged[0]); } diff --git a/src/DiffEngineTray.Tests/DebugReportTests.Derived.verified.txt b/src/DiffEngineTray.Tests/DebugReportTests.Derived.verified.txt new file mode 100644 index 000000000..56a8cf927 --- /dev/null +++ b/src/DiffEngineTray.Tests/DebugReportTests.Derived.verified.txt @@ -0,0 +1,42 @@ +DiffEngineTray TheVersion +Captured: 2024-10-01 13:45:30 +Inline queue: owned by another process on port {Port} +Tracking: True + +Deletes (1) +----------- +[1] Sample.Test#page_0002.verified.png + File: {Directory}\Sample.Test#page_0002.verified.png (exists) + Group: + DerivedFrom: {Directory}\Sample.Test.received.pdf (exists) + +Moves (2) +--------- +[1] Sample.Test + Temp: {Directory}\Sample.Test.received.pdf (exists) + Target: {Directory}\Sample.Test.verified.pdf (missing) + Extension: pdf + Group: + Exe: C:\tools\DiffEngineViewer.exe + Arguments: --diff + CanKill: False + KillLockingProcess: False + Process: none + IsOpen: True + +[2] Sample.Test#page_0001 + Temp: {Directory}\Sample.Test#page_0001.received.png (exists) + Target: {Directory}\Sample.Test#page_0001.verified.png (missing) + Extension: png + Group: + DerivedFrom: {Directory}\Sample.Test.received.pdf (exists) + Exe: C:\tools\DiffEngineViewer.exe + Arguments: --diff + CanKill: False + KillLockingProcess: False + Process: none + IsOpen: True + +Snapshots (0) +------------- +none diff --git a/src/DiffEngineTray.Tests/DebugReportTests.cs b/src/DiffEngineTray.Tests/DebugReportTests.cs index e6b4fd44a..19e4908cf 100644 --- a/src/DiffEngineTray.Tests/DebugReportTests.cs +++ b/src/DiffEngineTray.Tests/DebugReportTests.cs @@ -65,6 +65,43 @@ public async Task Full() await Verify(DebugReport.Build(tracker, now), settings); } + /// + /// A page of a document and a page it no longer has, each saying which pending move it was + /// derived from, and whether that file is still there. The document itself says nothing of + /// one, as every file that stands alone does, so the reports above are what they were. + /// + [Test] + public async Task Derived() + { + await using var tracker = new RecordingTracker(); + var document = Path.Combine(directory, "Sample.Test.received.pdf"); + var page = Path.Combine(directory, "Sample.Test#page_0001.received.png"); + var stale = Path.Combine(directory, "Sample.Test#page_0002.verified.png"); + // There, as pending files are: the tracker's scan drops what has gone + await File.WriteAllTextAsync(document, ""); + await File.WriteAllTextAsync(page, ""); + await File.WriteAllTextAsync(stale, ""); + const string viewer = @"C:\tools\DiffEngineViewer.exe"; + tracker.AddMove( + document, + Path.Combine(directory, "Sample.Test.verified.pdf"), + viewer, + "--diff", + canKill: false, + processId: null); + tracker.AddMove( + page, + Path.Combine(directory, "Sample.Test#page_0001.verified.png"), + viewer, + "--diff", + canKill: false, + processId: null, + source: document); + tracker.AddDelete(stale, document); + + await Verify(DebugReport.Build(tracker, now), settings); + } + /// /// The usual arrangement: the tray started first, so it holds the queue and the patches are in /// this process rather than in a viewer. diff --git a/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs b/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs index 5657b851f..19780af3e 100644 --- a/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs +++ b/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs @@ -685,11 +685,22 @@ public bool Has(string key) => public List Added { get; } = []; - public void AddMove(string temp, string target) => - Added.Add($"move {temp} > {target}"); + public void AddMove(string temp, string target, string? source) => + Added.Add($"move {temp} > {target}{From(source)}"); - public void AddDelete(string file) => - Added.Add($"delete {file}"); + public void AddDelete(string file, string? source) => + Added.Add($"delete {file}{From(source)}"); + + // Nothing where there is none, so a file with no source is recorded as it always was + static string From(string? source) + { + if (source is null) + { + return ""; + } + + return $" from {source}"; + } public List Untracked { get; } = []; diff --git a/src/DiffEngineTray.Tests/PiperTest.DeleteJson.verified.txt b/src/DiffEngineTray.Tests/PiperTest.DeleteJson.verified.txt index 5d9df6609..27b3faa79 100644 --- a/src/DiffEngineTray.Tests/PiperTest.DeleteJson.verified.txt +++ b/src/DiffEngineTray.Tests/PiperTest.DeleteJson.verified.txt @@ -1,9 +1,4 @@ { -"Type":"Move", -"Temp":"theTempFilePath", -"Target":"theTargetFilePath", -"CanKill":true, -"Exe":"theExePath", -"Arguments":"TheArguments", -"ProcessId":1000 -} \ No newline at end of file +"Type":"Delete", +"File":"theFilePath" +} diff --git a/src/DiffEngineTray.Tests/PiperTest.DeleteWithSourceJson.verified.txt b/src/DiffEngineTray.Tests/PiperTest.DeleteWithSourceJson.verified.txt new file mode 100644 index 000000000..b533c5246 --- /dev/null +++ b/src/DiffEngineTray.Tests/PiperTest.DeleteWithSourceJson.verified.txt @@ -0,0 +1,5 @@ +{ +"Type":"Delete", +"File":"theFilePath", +"Source":"theSourceTempFilePath" +} diff --git a/src/DiffEngineTray.Tests/PiperTest.MoveWithSourceJson.verified.txt b/src/DiffEngineTray.Tests/PiperTest.MoveWithSourceJson.verified.txt new file mode 100644 index 000000000..6aefad75c --- /dev/null +++ b/src/DiffEngineTray.Tests/PiperTest.MoveWithSourceJson.verified.txt @@ -0,0 +1,10 @@ +{ +"Type":"Move", +"Temp":"theTempFilePath", +"Target":"theTargetFilePath", +"CanKill":true, +"Exe":"theExePath", +"Arguments":"TheArguments", +"ProcessId":1000, +"Source":"theSourceTempFilePath" +} \ No newline at end of file diff --git a/src/DiffEngineTray.Tests/PiperTest.cs b/src/DiffEngineTray.Tests/PiperTest.cs index f98d33bf8..8bd752eb4 100644 --- a/src/DiffEngineTray.Tests/PiperTest.cs +++ b/src/DiffEngineTray.Tests/PiperTest.cs @@ -48,8 +48,18 @@ public Task MoveJson() => true, 1000)); + // The delete payload. This snapshotted a move for as long as the builder for a delete was + // private, and the documentation showing it under "Add pending delete" showed a move too [Test] public Task DeleteJson() => + Verify(PiperClient.BuildDeletePayload("theFilePath")); + + /// + /// A file derived from another names it, last, in a property a tray from before it has no + /// member for and so skips: see . + /// + [Test] + public Task MoveWithSourceJson() => Verify( PiperClient.BuildMovePayload( "theTempFilePath", @@ -57,7 +67,25 @@ public Task DeleteJson() => "theExePath", "TheArguments", true, - 1000)); + 1000, + "theSourceTempFilePath")); + + [Test] + public Task DeleteWithSourceJson() => + Verify(PiperClient.BuildDeletePayload("theFilePath", "theSourceTempFilePath")); + + /// + /// With no source the payloads are what they have always been, to the byte, so nothing about + /// a file that stands alone is new to any tray. + /// + [Test] + public async Task APayloadWithNoSourceIsUnchanged() + { + await Assert.That(PiperClient.BuildMovePayload("a", "b", "c", "d", true, 1, null)) + .IsEqualTo(PiperClient.BuildMovePayload("a", "b", "c", "d", true, 1)); + await Assert.That(PiperClient.BuildMovePayload("a", "b", "c", "d", true, 1)).DoesNotContain("Source"); + await Assert.That(PiperClient.BuildDeletePayload("a")).DoesNotContain("Source"); + } [Test] public async Task Delete() @@ -72,6 +100,61 @@ public async Task Delete() await Verify(received); } + [Test] + public async Task ADerivedMoveAndDeleteReachTheTrayWithTheirSource() + { + MovePayload? move = null; + DeletePayload? delete = null; + using var source = new CancelSource(); + var task = PiperServer.Start(_ => move = _, _ => delete = _, source.Token); + await PiperClient.SendMoveAsync("Page", "PageTarget", "theExe", "TheArguments", false, null, source.Token, "Document"); + await PiperClient.SendDeleteAsync("StalePage", source.Token, "Document"); + await Task.Delay(1000, source.Token); + await source.CancelAsync(); + await task; + + await Assert.That(move!.Temp).IsEqualTo("Page"); + await Assert.That(move.Source).IsEqualTo("Document"); + await Assert.That(delete!.File).IsEqualTo("StalePage"); + await Assert.That(delete.Source).IsEqualTo("Document"); + } + + /// + /// What a tray from before the property makes of a payload that has it. That tray's payload + /// type is this one without the member, and it reads with the same serializer and the same + /// options, so a type with the members it had stands in for it. + /// + /// This is the whole case for a property rather than a payload type of its own. A type an + /// older tray did not know would be logged and dropped (), + /// and the send is fire and forget, so the file would be pending in nothing. + /// + /// + [Test] + public async Task AnOlderTrayReadsADerivedMoveAsAnOrdinaryOne() + { + var payload = PiperClient.BuildMovePayload("Page", "PageTarget", "theExe", "TheArguments", false, null, "Document"); + + var older = Serializer.Deserialize(payload); + + await Assert.That(older.Temp).IsEqualTo("Page"); + await Assert.That(older.Target).IsEqualTo("PageTarget"); + await Assert.That(older.Exe).IsEqualTo("theExe"); + await Assert.That(older.Arguments).IsEqualTo("TheArguments"); + await Assert.That(older.CanKill).IsFalse(); + await Assert.That(older.ProcessId).IsNull(); + } + + // MovePayload as every tray had it before a move could name a source + class MoveBeforeSources + { + public string Temp { get; set; } = null!; + public string Target { get; set; } = null!; + public string? Exe { get; set; } = null!; + public string? Arguments { get; set; } = null!; + public bool CanKill { get; set; } + public int? ProcessId { get; set; } + } + [Test] public async Task Move() { diff --git a/src/DiffEngineTray.Tests/SerializerTests.cs b/src/DiffEngineTray.Tests/SerializerTests.cs index 29114d401..5094b858c 100644 --- a/src/DiffEngineTray.Tests/SerializerTests.cs +++ b/src/DiffEngineTray.Tests/SerializerTests.cs @@ -38,6 +38,76 @@ public async Task Deserialize_delete_payload() await Assert.That(result.File).IsEqualTo("theFile"); } + [Test] + public async Task Deserialize_payloads_naming_a_source() + { + var move = Serializer.Deserialize( + """ + { + "Type":"Move", + "Temp":"thePage", + "Target":"theTarget", + "CanKill":false, + "Source":"theDocument" + } + """); + var delete = Serializer.Deserialize( + """ + { + "Type":"Delete", + "File":"theFile", + "Source":"theDocument" + } + """); + + await Assert.That(move.Source).IsEqualTo("theDocument"); + await Assert.That(delete.Source).IsEqualTo("theDocument"); + } + + /// + /// A payload from a library that predates the property has none, and reads as a file that + /// stands alone. + /// + [Test] + public async Task A_payload_with_no_source_has_none() + { + var move = Serializer.Deserialize( + """ + { + "Type":"Move", + "Temp":"theTemp", + "Target":"theTarget", + "CanKill":true + } + """); + + await Assert.That(move.Source).IsNull(); + } + + /// + /// What makes a property the way to add to a payload: one this tray has no member for is + /// skipped, as Type has been in every payload this tray has ever read. So a library + /// newer than the tray can say more about a move without the tray losing the move. + /// + [Test] + public async Task A_property_this_tray_does_not_know_is_skipped() + { + var result = Serializer.Deserialize( + """ + { + "Type":"Move", + "Temp":"theTemp", + "Target":"theTarget", + "CanKill":true, + "SomethingALaterLibrarySays":"about the move" + } + """); + + await Assert.That(result.Temp).IsEqualTo("theTemp"); + await Assert.That(result.Target).IsEqualTo("theTarget"); + await Assert.That(result.CanKill).IsTrue(); + } + [Test] public async Task Deserialize_invalid_payload_throws_with_payload_in_message() { diff --git a/src/DiffEngineTray.Tests/TrackerMoveOntoDeleteTest.cs b/src/DiffEngineTray.Tests/TrackerMoveOntoDeleteTest.cs index bcc9b7ad7..60e94bbcc 100644 --- a/src/DiffEngineTray.Tests/TrackerMoveOntoDeleteTest.cs +++ b/src/DiffEngineTray.Tests/TrackerMoveOntoDeleteTest.cs @@ -37,7 +37,7 @@ public async Task AMoveArrivingOverTheViewerPortWithdrawsItToo() await using var tracker = new RecordingTracker(); tracker.AddDelete(verified); - ((ITrackedFiles) tracker).AddMove(received, verified); + ((ITrackedFiles) tracker).AddMove(received, verified, null); await Assert.That(tracker.Deletes).IsEmpty(); } diff --git a/src/DiffEngineTray.Tests/TrackerSourceTest.cs b/src/DiffEngineTray.Tests/TrackerSourceTest.cs new file mode 100644 index 000000000..fa975c274 --- /dev/null +++ b/src/DiffEngineTray.Tests/TrackerSourceTest.cs @@ -0,0 +1,206 @@ +/// +/// What a pending file was derived from, through the tracker: a page of a document whose document +/// is pending too. The tray only carries it. It lists the two as the two pending files they are, +/// and says which was derived from which to a viewer showing its queue, which is the one drawing +/// the document and so the one that puts a page beneath it. +/// +public class TrackerSourceTest : + IDisposable +{ + [Test] + public async Task AListingSaysWhatEachFileWasDerivedFrom() + { + await using var tracker = new RecordingTracker(); + ITrackedFiles tracked = tracker; + tracker.AddMove(document, documentTarget, viewerExe, "--diff", false, null); + tracker.AddMove(page, pageTarget, viewerExe, "--diff", false, null, document); + tracker.AddDelete(stale, document); + + var moves = tracked.Moves().ToDictionary(_ => _.Temp); + await Assert.That(moves[document].SourceKey).IsNull(); + // The key the document is listed under, so a reader matches one key against another + await Assert.That(moves[page].SourceKey).IsEqualTo(moves[document].Key); + await Assert.That(tracked.Deletes().Single().SourceKey).IsEqualTo(moves[document].Key); + } + + [Test] + public async Task AFileThatStandsAloneSaysNothing() + { + await using var tracker = new RecordingTracker(); + ITrackedFiles tracked = tracker; + tracker.AddMove(page, pageTarget, viewerExe, "--diff", false, null); + tracker.AddDelete(stale); + + await Assert.That(tracked.Moves().Single().SourceKey).IsNull(); + await Assert.That(tracked.Deletes().Single().SourceKey).IsNull(); + } + + /// + /// A move that names its tool is a run saying everything it knows about the pair, so what it + /// says of the source is taken, none included. A run whose document has stopped differing + /// sends its pages with no source, and they stop being that document's. + /// + [Test] + public async Task AMoveNamingItsToolSaysWhatItWasDerivedFromEachTime() + { + await using var tracker = new RecordingTracker(); + tracker.AddMove(page, pageTarget, viewerExe, "--diff", false, null, document); + await Assert.That(tracker.Moves.Single().Source).IsEqualTo(document); + + tracker.AddMove(page, pageTarget, viewerExe, "--diff", false, null); + await Assert.That(tracker.Moves.Single().Source).IsNull(); + + tracker.AddMove(page, pageTarget, viewerExe, "--diff", false, null, document); + await Assert.That(tracker.Moves.Single().Source).IsEqualTo(document); + } + + /// + /// A move over the viewer port that names no source is the pair forwarded by something that + /// was never told of one, "Open diff tool" among them, and says nothing about it either way. + /// One that names a source says so. + /// + [Test] + public async Task AMoveArrivingOverTheViewerPortKeepsItsSourceUnlessItNamesOne() + { + await using var tracker = new RecordingTracker(); + ITrackedFiles tracked = tracker; + tracker.AddMove(page, pageTarget, viewerExe, "--diff", false, null, document); + + tracked.AddMove(page, pageTarget, null); + await Assert.That(tracker.Moves.Single().Source).IsEqualTo(document); + await Assert.That(tracker.Moves.Single().IsViewer).IsTrue(); + + tracked.AddMove(page, pageTarget, other); + await Assert.That(tracker.Moves.Single().Source).IsEqualTo(other); + } + + [Test] + public async Task AMoveSeenFirstOverTheViewerPortTakesItsSource() + { + await using var tracker = new RecordingTracker(); + ITrackedFiles tracked = tracker; + + tracked.AddMove(page, pageTarget, document); + tracked.AddDelete(stale, document); + + await Assert.That(tracker.Moves.Single().Source).IsEqualTo(document); + await Assert.That(tracker.Deletes.Single().Source).IsEqualTo(document); + } + + /// + /// Everything a listing carries of a delete is fixed on the object, which is what lets the + /// objects tracked say whether a listing has changed. So a delete raised again under another + /// source is another delete, and one raised again under the same source is the one it was. + /// + [Test] + public async Task ADeleteRaisedAgainUnderAnotherSourceIsAnotherDelete() + { + await using var tracker = new RecordingTracker(); + ITrackedFiles tracked = tracker; + var first = tracker.AddDelete(stale, document); + var version = tracked.Version(); + + var again = tracker.AddDelete(stale, document); + await Assert.That(ReferenceEquals(again, first)).IsTrue(); + await Assert.That(tracked.Version()).IsEqualTo(version); + + var alone = tracker.AddDelete(stale); + await Assert.That(ReferenceEquals(alone, first)).IsFalse(); + await Assert.That(alone.Source).IsNull(); + await Assert.That(tracked.Version()).IsNotEqualTo(version); + await Assert.That(ReferenceEquals(tracker.Deletes.Single(), alone)).IsTrue(); + } + + /// + /// A run that passes deletes the received files it had left and settles what a viewer was + /// showing, which is the document. Its pages were tracked with no window to settle, so they + /// go with it here, rather than standing in a viewer as a row each until the scan finds them + /// gone. + /// + [Test] + public async Task SettlingADocumentDropsWhatWasDerivedFromItAndHasGone() + { + await using var tracker = new RecordingTracker(); + ITrackedFiles tracked = tracker; + tracker.AddMove(document, documentTarget, viewerExe, "--diff", false, null); + tracker.AddMove(page, pageTarget, viewerExe, "--diff", false, null, document); + // What the run did before it settled the document + File.Delete(page); + + await Assert.That(tracked.Untrack(TrackedKeys.ForMove(document))).IsTrue(); + + await Assert.That(tracker.Moves).IsEmpty(); + } + + /// + /// Only what has gone. A page that still differs was written again by the same run before it + /// settled the document, and is still a pending file. + /// + [Test] + public async Task SettlingADocumentLeavesWhatWasDerivedFromItAndIsStillThere() + { + await using var tracker = new RecordingTracker(); + ITrackedFiles tracked = tracker; + tracker.AddMove(document, documentTarget, viewerExe, "--diff", false, null); + tracker.AddMove(page, pageTarget, viewerExe, "--diff", false, null, document); + + await Assert.That(tracked.Untrack(TrackedKeys.ForMove(document))).IsTrue(); + + await Assert.That(tracker.Moves.Single().Temp).IsEqualTo(page); + } + + /// + /// And only what was derived from the move that went. + /// + [Test] + public async Task SettlingAMoveLeavesWhatWasDerivedFromAnother() + { + await using var tracker = new RecordingTracker(); + ITrackedFiles tracked = tracker; + tracker.AddMove(other, documentTarget, viewerExe, "--diff", false, null); + tracker.AddMove(page, pageTarget, viewerExe, "--diff", false, null, document); + File.Delete(page); + + await Assert.That(tracked.Untrack(TrackedKeys.ForMove(other))).IsTrue(); + + await Assert.That(tracker.Moves.Single().Temp).IsEqualTo(page); + } + + // The copy bundled in some other project's DiffEngine package, which is where a sender's + // viewer is and a path this process has never resolved + static readonly string viewerExe = Path.Combine( + Path.GetTempPath(), + "some-other-package", + "viewer", + "DiffEngineViewer.exe"); + + readonly string directory; + readonly string document; + readonly string documentTarget; + readonly string page; + readonly string pageTarget; + readonly string stale; + readonly string other; + + public TrackerSourceTest() + { + directory = Path.Combine(Path.GetTempPath(), $"TrackerSourceTest_{Guid.NewGuid():N}"); + Directory.CreateDirectory(directory); + document = Path.Combine(directory, "Sample.Test.received.pdf"); + documentTarget = Path.Combine(directory, "Sample.Test.verified.pdf"); + page = Path.Combine(directory, "Sample.Test#page_0001.received.png"); + pageTarget = Path.Combine(directory, "Sample.Test#page_0001.verified.png"); + stale = Path.Combine(directory, "Sample.Test#page_0002.verified.png"); + other = Path.Combine(directory, "Other.Test.received.pdf"); + // Every file a test tracks is there, as a pending file's is. The tracker's scan drops a + // move whose received file has gone and a delete whose file has, every two seconds, so a + // test that wants one gone takes it away itself + File.WriteAllText(document, "document"); + File.WriteAllText(page, "page"); + File.WriteAllText(other, "another document"); + File.WriteAllText(stale, ""); + } + + public void Dispose() => + FileEx.SafeDeleteDirectory(directory); +} diff --git a/src/DiffEngineTray.Tests/TrackerTrackedFilesTest.cs b/src/DiffEngineTray.Tests/TrackerTrackedFilesTest.cs index 843b694b5..56ead5fa8 100644 --- a/src/DiffEngineTray.Tests/TrackerTrackedFilesTest.cs +++ b/src/DiffEngineTray.Tests/TrackerTrackedFilesTest.cs @@ -267,7 +267,7 @@ public async Task AMoveArrivingAgainOverTheViewerPortKeepsItsTool() var arguments = $"--diff \"{temp}\" \"{target}\""; tracker.AddMove(temp, target, viewerExe, arguments, false, null); - ((ITrackedFiles) tracker).AddMove(temp, target); + ((ITrackedFiles) tracker).AddMove(temp, target, null); var move = tracker.Moves.Single(); await Assert.That(move.Exe).IsEqualTo(viewerExe); @@ -295,7 +295,7 @@ public async Task AMoveArrivingAgainOverTheViewerPortKeepsAnotherToolAndItsProce var exe = tool.MainModule!.FileName; tracker.AddMove(temp, target, exe, "theArguments", true, tool.Id); - ((ITrackedFiles) tracker).AddMove(temp, target); + ((ITrackedFiles) tracker).AddMove(temp, target, null); var move = tracker.Moves.Single(); await Assert.That(move.Exe).IsEqualTo(exe); @@ -322,7 +322,7 @@ public async Task AMoveArrivingAgainOverTheViewerPortTakesItsTarget() tracker.AddMove(temp, target, viewerExe, "--diff", false, null); var moved = Path.Combine(Path.GetTempPath(), $"TrackedFilesTest_{Guid.NewGuid():N}.Other.verified.bin"); - ((ITrackedFiles) tracker).AddMove(temp, moved); + ((ITrackedFiles) tracker).AddMove(temp, moved, null); var move = tracker.Moves.Single(); await Assert.That(move.Target).IsEqualTo(moved); diff --git a/src/DiffEngineTray/DebugReport.cs b/src/DiffEngineTray/DebugReport.cs index 0107d67cb..c9d758bb0 100644 --- a/src/DiffEngineTray/DebugReport.cs +++ b/src/DiffEngineTray/DebugReport.cs @@ -48,6 +48,7 @@ public static string Build(Tracker tracker, DateTime now) AppendEntry(builder, index, delete.Name); AppendField(builder, "File", WithExistence(delete.File)); AppendField(builder, "Group", delete.Group); + AppendSource(builder, delete.Source); // Only where there is one: why "Accept all" would leave this delete pending if (tracker.HeldReason(delete) is { } held) { @@ -64,6 +65,7 @@ public static string Build(Tracker tracker, DateTime now) AppendField(builder, "Target", WithExistence(move.Target)); AppendField(builder, "Extension", move.Extension); AppendField(builder, "Group", move.Group); + AppendSource(builder, move.Source); AppendField(builder, "Exe", move.Exe); AppendField(builder, "Arguments", move.Arguments); AppendField(builder, "CanKill", move.CanKill); @@ -89,6 +91,22 @@ public static string Build(Tracker tracker, DateTime now) return builder.ToString(); } + /// + /// What a move or a delete was derived from, only where it was derived from something: a page + /// of a document that is pending too. With whether that file is there, since a source that has + /// gone is one whose pages a viewer has stopped showing beneath it. + /// + /// Not labelled Source, which a snapshot below already uses for the file its literal is in. + /// + /// + static void AppendSource(StringBuilder builder, string? source) + { + if (source != null) + { + AppendField(builder, "DerivedFrom", WithExistence(source)); + } + } + /// /// The patch behind a queued snapshot, which is everything the reviewer sees derived back to /// what will be written. Only when this tray owns the queue: a viewer that owns one holds the diff --git a/src/DiffEngineTray/ITrackedFiles.cs b/src/DiffEngineTray/ITrackedFiles.cs index cfba3d35c..ae329c385 100644 --- a/src/DiffEngineTray/ITrackedFiles.cs +++ b/src/DiffEngineTray/ITrackedFiles.cs @@ -60,10 +60,16 @@ interface ITrackedFiles /// That happens when the sending process saw no tray at startup and this tray started after /// it: that check is cached for the life of the sender, so its files come the other way for /// good, and dropping them would lose them. + /// + /// is the received file of the pending move this one was derived + /// from, or null. With no default, as on , so nothing + /// between the wire and the tracker can leave it behind. + /// /// - void AddMove(string temp, string target); + void AddMove(string temp, string target, string? source); - void AddDelete(string file); + /// + void AddDelete(string file, string? source); /// /// Drop a tracked move or delete without touching the file, for a test that started passing. diff --git a/src/DiffEngineTray/OwnedInlineHost.cs b/src/DiffEngineTray/OwnedInlineHost.cs index f6f914dcd..abf37d3ad 100644 --- a/src/DiffEngineTray/OwnedInlineHost.cs +++ b/src/DiffEngineTray/OwnedInlineHost.cs @@ -282,15 +282,15 @@ void IQueueOwner.Settle(string key, string? origin, string? member, string? valu /// this tray when the sending process saw no tray as it started and so addressed the queue /// owner instead — and this tray is the queue owner. /// - void IQueueOwner.TrackMove(string temp, string target) + void IQueueOwner.TrackMove(string temp, string target, string? source) { - TrackedFiles?.AddMove(temp, target); + TrackedFiles?.AddMove(temp, target, source); Changed?.Invoke(); } - void IQueueOwner.TrackDelete(string file) + void IQueueOwner.TrackDelete(string file, string? source) { - TrackedFiles?.AddDelete(file); + TrackedFiles?.AddDelete(file, source); Changed?.Invoke(); } diff --git a/src/DiffEngineTray/Payloads/DeletePayload.cs b/src/DiffEngineTray/Payloads/DeletePayload.cs index 1a6d668dd..e3a46d197 100644 --- a/src/DiffEngineTray/Payloads/DeletePayload.cs +++ b/src/DiffEngineTray/Payloads/DeletePayload.cs @@ -1,4 +1,7 @@ -class DeletePayload +class DeletePayload { public string File { get; set; } = null!; + + /// + public string? Source { get; set; } } \ No newline at end of file diff --git a/src/DiffEngineTray/Payloads/MovePayload.cs b/src/DiffEngineTray/Payloads/MovePayload.cs index 80161ae6b..c4bd5f25e 100644 --- a/src/DiffEngineTray/Payloads/MovePayload.cs +++ b/src/DiffEngineTray/Payloads/MovePayload.cs @@ -1,4 +1,4 @@ -class MovePayload +class MovePayload { public string Temp { get; set; } = null!; public string Target { get; set; } = null!; @@ -6,4 +6,11 @@ public string? Arguments { get; set; } = null!; public bool CanKill { get; set; } public int? ProcessId { get; set; } + + /// + /// The received file of the pending move this one was derived from, from a library that says + /// so: a page of a document whose document is pending too. Null from one that does not, and + /// for a file that stands alone. + /// + public string? Source { get; set; } } \ No newline at end of file diff --git a/src/DiffEngineTray/Program.cs b/src/DiffEngineTray/Program.cs index 9ec0c2e4a..5efc8c61b 100644 --- a/src/DiffEngineTray/Program.cs +++ b/src/DiffEngineTray/Program.cs @@ -259,8 +259,20 @@ static Task StartServer(TcpListener listener, Tracker tracker, Cancel cancel) => payload.Exe, payload.Arguments, payload.CanKill, - payload.ProcessId); + payload.ProcessId, + Source(payload.Source)); }, - payload => tracker.AddDelete(payload.File), + payload => tracker.AddDelete(payload.File, Source(payload.Source)), cancel); + + // An empty one names nothing, as on the viewer port + static string? Source(string? source) + { + if (string.IsNullOrEmpty(source)) + { + return null; + } + + return source; + } } \ No newline at end of file diff --git a/src/DiffEngineTray/TrackedDelete.cs b/src/DiffEngineTray/TrackedDelete.cs index 683111145..f67a143b3 100644 --- a/src/DiffEngineTray/TrackedDelete.cs +++ b/src/DiffEngineTray/TrackedDelete.cs @@ -1,9 +1,10 @@ class TrackedDelete { - public TrackedDelete(string file, string? group) + public TrackedDelete(string file, string? group, string? source = null) { File = file; Group = group; + Source = source; Name = Path.GetFileName(file); } @@ -11,6 +12,16 @@ public TrackedDelete(string file, string? group) public string File { get; } public string? Group { get; } + /// + /// The received file of the pending move this delete was derived from, or null: a page a + /// document no longer has, whose document is pending. See . + /// + /// Fixed, as everything else a listing carries of a delete is, so a delete raised again under + /// another source is another object and sees it. + /// + /// + public string? Source { get; } + /// /// Set once a move has written this file while the delete was pending, and from then on no /// accept-all carries the delete out: see . Cleared when a diff --git a/src/DiffEngineTray/TrackedMove.cs b/src/DiffEngineTray/TrackedMove.cs index 02088e75b..0b35b115f 100644 --- a/src/DiffEngineTray/TrackedMove.cs +++ b/src/DiffEngineTray/TrackedMove.cs @@ -9,7 +9,8 @@ public TrackedMove(string temp, string? group, string extension, bool killLockingProcess = false, - bool isViewer = false) + bool isViewer = false, + string? source = null) { Temp = temp; Target = target; @@ -22,6 +23,7 @@ public TrackedMove(string temp, Group = group; KillLockingProcess = killLockingProcess; IsViewer = isViewer; + Source = source; } public string Extension { get; } @@ -35,6 +37,14 @@ public TrackedMove(string temp, public string? Group { get; } public bool KillLockingProcess { get; } + /// + /// The received file of the pending move this one was derived from, or null: a page of a + /// document whose document is pending too. Only carried here. The tray lists the two as the + /// two pending files they are, and a viewer showing the queue is what puts one beneath the + /// other, since it is the one drawing the document. + /// + public string? Source { get; } + /// /// Whether the tool showing this pair is the viewer, which is the one tool that opens no /// process of its own for it. diff --git a/src/DiffEngineTray/Tracker.cs b/src/DiffEngineTray/Tracker.cs index 682075d79..615df98aa 100644 --- a/src/DiffEngineTray/Tracker.cs +++ b/src/DiffEngineTray/Tracker.cs @@ -231,13 +231,24 @@ void ToggleActive() !deletes.IsEmpty || snapshots.Count > 0; + /// The received file. + /// The file it belongs at. + /// The tool showing the pair, or null when the sender named none. + /// What that tool was started with. + /// Whether that tool may be closed when the pair is accepted. + /// The process showing the pair, where one was started for it. + /// + /// The received file of the pending move this one was derived from, or null: see + /// . + /// public TrackedMove AddMove( string temp, string target, string? exe, string? arguments, bool canKill, - int? processId) + int? processId, + string? source = null) { var exeFile = Path.GetFileName(exe); var targetFile = Path.GetFileName(target); @@ -262,7 +273,7 @@ public TrackedMove AddMove( ProcessEx.TryGetTool(processId.Value, exe, temp, out process); } - var move = BuildTrackedMove(temp, exe, arguments, canKill, target, process); + var move = BuildTrackedMove(temp, exe, arguments, canKill, target, process, source); if (exeFile == null) { @@ -294,8 +305,8 @@ public TrackedMove AddMove( } var move = exe == null - ? Retarget(existing, target, process) - : BuildTrackedMove(temp, exe, arguments, canKill, target, process); + ? Retarget(existing, target, process, source) + : BuildTrackedMove(temp, exe, arguments, canKill, target, process, source); if (exeFile == null) { @@ -322,8 +333,12 @@ public TrackedMove AddMove( /// forwards the pair here as a Diff. The pair then read as another tool's with no window, so /// "Accept open" passed over it while it was on screen, and it had become killable. /// + /// + /// What the pair was derived from is kept the same way when this move names none. The one + /// "Open diff tool" forwards is exactly that: two paths, from a viewer that was never told. + /// /// - static TrackedMove Retarget(TrackedMove existing, string target, Process? process) => + static TrackedMove Retarget(TrackedMove existing, string target, Process? process, string? source) => new( existing.Temp, target, @@ -334,9 +349,15 @@ static TrackedMove Retarget(TrackedMove existing, string target, Process? proces SolutionDirectoryFinder.Find(target), Path.GetExtension(target).TrimStart('.'), existing.KillLockingProcess, - existing.IsViewer); + existing.IsViewer, + source ?? existing.Source); - static TrackedMove BuildTrackedMove(string temp, string? exe, string? arguments, bool? canKill, string target, Process? process) + /// + /// is taken as it arrived, null included. A move that names its + /// tool is a run saying everything it knows about the pair, and a run whose source has + /// stopped being pending says so by naming none. + /// + static TrackedMove BuildTrackedMove(string temp, string? exe, string? arguments, bool? canKill, string target, Process? process, string? source) { var solution = SolutionDirectoryFinder.Find(target); var extension = Path.GetExtension(target).TrimStart('.'); @@ -387,7 +408,8 @@ static TrackedMove BuildTrackedMove(string temp, string? exe, string? arguments, solution, extension, killLockingProcess, - PendingFiles.IsViewerExe(exe)); + PendingFiles.IsViewerExe(exe), + source); } /// @@ -648,18 +670,32 @@ public void Refresh() ToggleActive(); } - public TrackedDelete AddDelete(string file) => + /// The file a passing test no longer produces. + /// + /// The received file of the pending move the delete was derived from, or null: see + /// . + /// + public TrackedDelete AddDelete(string file, string? source = null) => deletes.AddOrUpdate( file, addValueFactory: key => { Log.Information("DeleteAdded. File:{file}", file); var solution = SolutionDirectoryFinder.Find(key); - return new(key, solution); + return new(key, solution, source); }, updateValueFactory: (_, existing) => { Log.Information("DeleteUpdated. File:{file}", file); + // A listing carries what a delete was derived from, and the objects tracked are + // what says whether a listing has changed, so one raised again under another + // source is another delete. A new one is not marked either, which is right: it + // was raised by a run that looked at the file as it is now + if (!string.Equals(existing.Source, source, StringComparison.OrdinalIgnoreCase)) + { + return new(existing.File, existing.Group, source); + } + // Raised again, so by a run that looked at the file as it is now. Whatever a move // wrote there since the delete was first raised, this is the later statement if (existing.Written) @@ -1289,9 +1325,26 @@ IReadOnlyList ITrackedFiles.Moves() => $"{_.Name} ({_.Extension})", _.Group, _.Temp, - _.Target)) + _.Target) + { + SourceKey = KeyOfSource(_.Source) + }) .ToList(); + /// + /// What a listing says a file was derived from: the key its source is listed under, so a + /// reader matches one key against another and never builds one. + /// + static string? KeyOfSource(string? source) + { + if (source == null) + { + return null; + } + + return TrackedKeys.ForMove(source); + } + IReadOnlyList ITrackedFiles.Deletes() { // The files the pending moves are onto, gathered once: HeldReason walks the moves for @@ -1310,7 +1363,8 @@ IReadOnlyList ITrackedFiles.Deletes() _.File) { // What the menu says beside it, for a viewer showing this queue to say too - Held = HeldReason(_, awaited.Contains(_.File)) + Held = HeldReason(_, awaited.Contains(_.File)), + SourceKey = KeyOfSource(_.Source) }) .ToList(); } @@ -1405,17 +1459,18 @@ bool Unchanged() return index == versioned.Count; } - void ITrackedFiles.AddMove(string temp, string target) + void ITrackedFiles.AddMove(string temp, string target, string? source) { // No exe, arguments or process: the sender's diff tool details do not cross the viewer // port, so this is resolved from the extension exactly as a piper move with no exe is. - AddMove(temp, target, null, null, false, null); + // What the pair was derived from does cross it, being about the file and not a tool. + AddMove(temp, target, null, null, false, null, source); Refresh(); } - void ITrackedFiles.AddDelete(string file) + void ITrackedFiles.AddDelete(string file, string? source) { - AddDelete(file); + AddDelete(file, source); Refresh(); } @@ -1440,6 +1495,7 @@ bool ITrackedFiles.Untrack(string key) } Release(removed); + UntrackDerivedFrom(removed); return true; } @@ -1447,6 +1503,39 @@ bool ITrackedFiles.Untrack(string key) deletes.TryRemove(file, out _); } + /// + /// The moves derived from one that has just been settled, where their own received file has + /// gone as well: dropped now, rather than by the scan up to two seconds on. + /// + /// A run that passes deletes every received file it had left, and settles what the viewer was + /// showing. That is the document. Its pages had no window to settle - they were tracked and + /// shown beneath it - so for those two seconds they stood in a viewer as a row each, with + /// nothing to show, where a moment before there had been one row for the lot. + /// + /// + /// The scan's own first rule, applied early, and so only to a file that is not there. A page + /// that still differs has been written again by the same run, and stays. + /// + /// + void UntrackDerivedFrom(TrackedMove source) + { + foreach (var pair in moves) + { + var move = pair.Value; + if (!string.Equals(move.Source, source.Temp, StringComparison.OrdinalIgnoreCase) || + File.Exists(move.Temp)) + { + continue; + } + + // By key and value, as the scan removes one, so a move staged again since is left + if (moves.TryRemove(pair)) + { + Release(move); + } + } + } + (bool ok, string? message) ITrackedFiles.Accept(string key) { if (TrackedKeys.TryStrip(key, TrackedKeys.MovePrefix, out var temp)) diff --git a/src/DiffEngineViewer.Tests/DerivedAcceptTests.cs b/src/DiffEngineViewer.Tests/DerivedAcceptTests.cs new file mode 100644 index 000000000..01327516c --- /dev/null +++ b/src/DiffEngineViewer.Tests/DerivedAcceptTests.cs @@ -0,0 +1,474 @@ +/// +/// Accepting or discarding a document takes the files derived from it too. +/// +/// A batch over the document and what is shown beneath it, the same one "Accept all in" a header +/// is: a file a step, each outside the lock, with what could not be moved left in the queue saying +/// why. A document with fifty pages is fifty-one files, and in one transition they would be moved +/// on the render thread. +/// +/// +/// Only the window's accept. An accept by key over the wire is another surface saying which file +/// it means, and is carried out as asked. +/// +/// +public class DerivedAcceptTests +{ + /// + /// What the window does with the key: begins the batch, and moves nothing. Real actions are + /// what a key is dispatched with, so anything moved here would be an attempt on files that do + /// not exist, and would show as failed entries. + /// + [Test] + public async Task AcceptingTheDocumentBeginsABatchOverItAndWhatIsBeneathIt() + { + var state = Fixtures.DocumentWithDerived(); + + var accepted = ViewerProgram.Apply(state, Key(CommandKind.Accept), link: null, new NoWindow()); + + await Assert.That(accepted.Batch).IsNotNull(); + await Assert.That(accepted.Batch!.Total).IsEqualTo(6); + await Assert.That(accepted.Batch.Cascade).IsEqualTo("Sample.Test (pdf)"); + await Assert.That(accepted.Batch.Covers(state.Queue.Single(_ => _.Name == "Other.Test (txt)").Key)).IsFalse(); + await Assert.That(accepted.Queue.Count).IsEqualTo(7); + await Assert.That(accepted.Queue.All(_ => _.Status is null)).IsTrue(); + await Assert.That(ScreenBuilder.Build(accepted).Status).IsEqualTo("Accepting 1 of 6"); + } + + /// + /// Carried out the way the loop has it carried out: what is beneath the document first, the + /// document last, and the entry of some other test left alone. + /// + [Test] + public async Task TheDocumentIsTheLastToGo() + { + var done = new List(); + var host = new SessionHost(Fixtures.DocumentWithDerived()); + host.Mutate(ViewerSession.BeginAcceptWithDerived); + + var message = new AcceptAllRunner(host, DerivedFilesTests.Recording(done)).Drive(); + + await Assert.That(done).IsEquivalentTo( + [ + "move Sample.Test.received.txt", + "move Sample.Test#page_0001.received.png", + "move Sample.Test#page_0001.received.txt", + "move Sample.Test#page_0002.received.png", + "delete Sample.Test#page_0003.verified.png", + "move Sample.Test.received.pdf" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + await Assert.That(message).IsEqualTo("Accepted Sample.Test (pdf) and 5 derived files"); + await Assert.That(host.State.Queue.Select(_ => _.Name)).IsEquivalentTo(["Other.Test (txt)"]); + await Assert.That(host.State.Batch).IsNull(); + } + + /// + /// Taken last, the document is in the queue for as long as its files are, so none of them is + /// ever a row of its own on the way out: at every step the rows are the document and what was + /// never beneath it. + /// + [Test] + public async Task NothingBeneathTheDocumentBecomesARowWhileItGoes() + { + var host = new SessionHost(Fixtures.DocumentWithDerived()); + var rows = new List(); + var actions = DerivedFilesTests.Recording([]) with + { + MoveFile = (_, _) => rows.Add(QueueProjection.Rows(host.State).Count), + DeleteFile = _ => rows.Add(QueueProjection.Rows(host.State).Count) + }; + host.Mutate(ViewerSession.BeginAcceptWithDerived); + + new AcceptAllRunner(host, actions).Drive(); + + await Assert.That(rows.All(_ => _ == 2)).IsTrue(); + } + + /// + /// A file that could not be moved stays, saying why, and the rest go. With the document gone + /// it is an ordinary row, where its failure can be read and it can be tried again. + /// + [Test] + public async Task AFileThatCouldNotBeMovedStaysAndSaysWhy() + { + var host = new SessionHost(Fixtures.DocumentWithDerived()); + var actions = DerivedFilesTests.Recording([]) with + { + MoveFile = (temp, _) => + { + if (temp.Contains("#page_0002")) + { + throw new IOException("the file is locked"); + } + } + }; + host.Mutate(ViewerSession.BeginAcceptWithDerived); + + var message = new AcceptAllRunner(host, actions).Drive(); + + await Assert.That(message).IsEqualTo("Accepted 5 of 6 files of Sample.Test (pdf) (1 kept)"); + var kept = host.State.Queue.Single(_ => _.Name == "Sample.Test#page_0002 (png)"); + await Assert.That(kept.Status).IsEqualTo("the file is locked"); + await Assert.That(QueueProjection.Rows(host.State).Select(_ => _.Label)).IsEquivalentTo( + [ + "Sample.Test#page_0002 (png)", + "Other.Test (txt)" + ]); + } + + /// + /// And when the one that could not be moved is the document, its files have gone and it is + /// what is left to try again. + /// + [Test] + public async Task ADocumentThatCouldNotBeMovedStaysWithoutItsFiles() + { + var host = new SessionHost(Fixtures.DocumentWithDerived()); + var actions = DerivedFilesTests.Recording([]) with + { + MoveFile = (temp, _) => + { + if (temp.EndsWith(".pdf", StringComparison.Ordinal)) + { + throw new IOException("open in another program"); + } + } + }; + host.Mutate(ViewerSession.BeginAcceptWithDerived); + + var message = new AcceptAllRunner(host, actions).Drive(); + + await Assert.That(message).IsEqualTo("Accepted 5 of 6 files of Sample.Test (pdf) (1 kept)"); + await Assert.That(QueueProjection.Rows(host.State).Select(_ => _.Label)).IsEquivalentTo( + [ + "Sample.Test (pdf)", + "Other.Test (txt)" + ]); + await Assert.That(host.State.Queue[0].Status).IsEqualTo("open in another program"); + } + + /// + /// Discarding throws away the document's received file and those of the files derived from + /// it. A pending delete among them is only untracked, which is what discarding one has always + /// meant: the file it would have removed stays. + /// + [Test] + public async Task DiscardingTheDocumentDiscardsWhatIsBeneathIt() + { + var done = new List(); + var state = Fixtures.DocumentWithDerived(); + + var begun = ViewerProgram.Apply(state, Key(CommandKind.Discard), link: null, new NoWindow()); + await Assert.That(begun.Batch!.Discarding).IsTrue(); + var host = new SessionHost(begun); + var message = new AcceptAllRunner(host, DerivedFilesTests.Recording(done)).Drive(); + + // Received files only, and the document's last + await Assert.That(done).IsEquivalentTo( + [ + "delete Sample.Test.received.txt", + "delete Sample.Test#page_0001.received.png", + "delete Sample.Test#page_0001.received.txt", + "delete Sample.Test#page_0002.received.png", + "delete Sample.Test.received.pdf" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + await Assert.That(message).IsEqualTo("Discarded Sample.Test (pdf) and 5 derived files"); + await Assert.That(host.State.Queue.Select(_ => _.Name)).IsEquivalentTo(["Other.Test (txt)"]); + } + + /// + /// Unfolded changes what has a row, not what goes with the document. + /// + [Test] + public async Task UnfoldedTheyStillGoWithTheDocument() + { + var unfolded = ViewerSession.Apply(Fixtures.DocumentWithDerived(), CommandKind.ToggleDerived); + + var accepted = ViewerProgram.Apply(unfolded, Key(CommandKind.Accept), link: null, new NoWindow()); + + await Assert.That(accepted.Batch!.Total).IsEqualTo(6); + } + + /// + /// One of them accepted on its own, from its own row, is that one file: it has nothing beneath + /// it, and the document and the rest are still pending. + /// + [Test] + public async Task OneOfThemAcceptedOnItsOwnIsThatOneFile() + { + var done = new List(); + var state = Fixtures.DocumentWithDerived(); + var reading = ViewerSession.SelectKey(state, state.Queue.Single(_ => _.Name == "Sample.Test#page_0002 (png)").Key); + await Assert.That(ViewerSession.HasDerived(reading)).IsFalse(); + + var accepted = ViewerSession.Apply(reading, CommandKind.Accept, DerivedFilesTests.Recording(done)); + + await Assert.That(done).IsEquivalentTo(["move Sample.Test#page_0002.received.png"]); + await Assert.That(accepted.Queue.Count).IsEqualTo(6); + await Assert.That(accepted.Batch).IsNull(); + } + + /// + /// An entry with nothing beneath it is not a batch, and the transition says so by leaving the + /// state as it was: the window accepts it the ordinary way. + /// + [Test] + public async Task AnEntryWithNothingBeneathItBeginsNoBatch() + { + var state = ViewerSession.Apply(Fixtures.DocumentWithDerived(), CommandKind.NextItem); + + await Assert.That(ViewerSession.HasDerived(state)).IsFalse(); + await Assert.That(ViewerSession.BeginAcceptWithDerived(state).Batch).IsNull(); + await Assert.That(ViewerSession.BeginDiscardWithDerived(state).Batch).IsNull(); + } + + /// + /// An accept by key over the wire is one entry, as it always was. Whoever sent it named the + /// file it meant, and what was derived from that file is left standing as rows of its own. + /// + [Test] + public async Task AnAcceptByKeyOverTheWireIsThatOneEntry() + { + var done = new List(); + var host = new SessionHost(Fixtures.DocumentWithDerived()); + IQueueOwner owner = new MessageHandler(host, DerivedFilesTests.Recording(done), _ => { }); + + var (ok, _, _) = owner.Accept(Fixtures.DocumentKey, null); + + await Assert.That(ok).IsTrue(); + await Assert.That(done).IsEquivalentTo(["move Sample.Test.received.pdf"]); + await Assert.That(host.State.Queue.Count).IsEqualTo(6); + await Assert.That(QueueProjection.Rows(host.State).Count).IsEqualTo(6); + } + + /// + /// A window showing someone else's queue applies nothing itself, so its accept of a document + /// is the same thing sent as keys: what is beneath the document, then the document. + /// + [Test] + public async Task AnAttachedWindowSendsWhatIsBeneathTheDocumentThenTheDocument() + { + using var files = new DocumentFiles(); + using var owner = new RecordingOwner(files.Listing); + using var documents = Documents(); + var host = new SessionHost(SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows)); + var link = new OwnerLink(host, owner.Port, documents); + link.Pump(); + await Assert.That(QueueProjection.Rows(host.State).Select(_ => _.Label)).IsEquivalentTo(["+ Sample.Test (pdf) (2)"]); + + host.Mutate(_ => ViewerProgram.Apply(_, Key(CommandKind.Accept), link, new NoWindow())); + link.Pump(); + + await Assert.That(owner.Heard).IsEquivalentTo( + [ + $"{ViewerVerb.Accept} {TrackedKeys.ForMove(files.Page)}", + $"{ViewerVerb.Accept} {TrackedKeys.ForDelete(files.Stale)}", + $"{ViewerVerb.Accept} {TrackedKeys.ForMove(files.Document)}" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + } + + [Test] + public async Task AnAttachedWindowDiscardsTheSameWay() + { + using var files = new DocumentFiles(); + using var owner = new RecordingOwner(files.Listing); + using var documents = Documents(); + var host = new SessionHost(SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows)); + var link = new OwnerLink(host, owner.Port, documents); + link.Pump(); + + host.Mutate(_ => ViewerProgram.Apply(_, Key(CommandKind.Discard), link, new NoWindow())); + link.Pump(); + + await Assert.That(owner.Heard).IsEquivalentTo( + [ + $"{ViewerVerb.Discard} {TrackedKeys.ForMove(files.Page)}", + $"{ViewerVerb.Discard} {TrackedKeys.ForDelete(files.Stale)}", + $"{ViewerVerb.Discard} {TrackedKeys.ForMove(files.Document)}" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + } + + /// + /// An owner from before any of this says nothing of what was derived from what. Nothing is + /// then beneath anything, so every file is a row and an accept is of the entry on screen, as + /// it was. + /// + [Test] + public async Task AnOwnerThatSaysNothingOfSourcesIsShownAsItAlwaysWas() + { + using var files = new DocumentFiles(); + using var owner = new RecordingOwner(() => files.Listing(sources: false)); + using var documents = Documents(); + var host = new SessionHost(SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows)); + var link = new OwnerLink(host, owner.Port, documents); + link.Pump(); + await Assert.That(QueueProjection.Rows(host.State).Count).IsEqualTo(3); + + host.Mutate(_ => ViewerSession.SelectKey(_, TrackedKeys.ForMove(files.Document))); + host.Mutate(_ => ViewerProgram.Apply(_, Key(CommandKind.Accept), link, new NoWindow())); + link.Pump(); + + await Assert.That(owner.Heard).IsEquivalentTo([$"{ViewerVerb.Accept} {TrackedKeys.ForMove(files.Document)}"]); + } + + /// + /// And a window with no documents folder draws no document, so it puts nothing beneath one + /// whatever its owner says: the pages are the only pictures it has to show. + /// + [Test] + public async Task AWindowThatDrawsNoDocumentsShowsEveryFile() + { + using var files = new DocumentFiles(); + using var owner = new RecordingOwner(files.Listing); + var host = new SessionHost(SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows)); + + new OwnerLink(host, owner.Port).Pump(); + + await Assert.That(QueueProjection.Rows(host.State).Count).IsEqualTo(3); + await Assert.That(host.State.Queue.Count(_ => _.SourceKey is not null)).IsEqualTo(2); + } + + static ViewerInput Key(CommandKind key) => + new(key, -1, -1, 0, false, Fixtures.Columns, Fixtures.Rows); + + /// + /// A documents folder that reads and draws nothing. Having one at all is what makes a PDF a + /// document to this process rather than text, which is all these tests need of it. + /// + static DocumentPlugin Documents() => + new( + static _ => throw new("Not read in these tests."), + static (_, _, _, _) => throw new("Not drawn in these tests.")); + + /// + /// A document, a page of it and a page it has lost, on disk, as an owner lists them. + /// + sealed class DocumentFiles : + IDisposable + { + readonly string directory = Directory.CreateTempSubdirectory("deview-derived-").FullName; + + public string Document { get; } + public string Page { get; } + public string Stale { get; } + + public DocumentFiles() + { + Document = Write("Sample.Test.received.pdf", "received document"); + Write("Sample.Test.verified.pdf", "verified document"); + Page = Write("Sample.Test#page_0001.received.png", "received page"); + Stale = Write("Sample.Test#page_0002.verified.png", "a page it no longer has"); + } + + public ViewerResponse Listing() => + Listing(sources: true); + + public ViewerResponse Listing(bool sources) + { + var sourceKey = sources ? TrackedKeys.ForMove(Document) : null; + return ViewerResponse.Listing( + [], + moves: + [ + // The page ahead of its document, as a tray's dictionary can list them + new(TrackedKeys.ForMove(Page), "Sample.Test#page_0001 (png)", null, Page, Path.Combine(directory, "Sample.Test#page_0001.verified.png")) + { + SourceKey = sourceKey + }, + new(TrackedKeys.ForMove(Document), "Sample.Test (pdf)", null, Document, Path.Combine(directory, "Sample.Test.verified.pdf")) + ], + deletes: + [ + new(TrackedKeys.ForDelete(Stale), "Sample.Test#page_0002.verified.png", null, Stale) + { + SourceKey = sourceKey + } + ]); + } + + string Write(string name, string content) + { + var path = Path.Combine(directory, name); + File.WriteAllText(path, content); + return path; + } + + public void Dispose() => + Directory.Delete(directory, true); + } + + /// + /// An owner that lists what it is given and writes down what it is asked to accept and + /// discard, so the assertions are about what a window sends. + /// + sealed class RecordingOwner : + IDisposable + { + readonly ViewerServer server; + readonly CancelSource cancel = new(); + + public ConcurrentQueue Heard { get; } = new(); + + public int Port => server.Port; + + public RecordingOwner(Func listing) + { + if (!ViewerServer.TryBind(0, out var bound)) + { + throw new("Could not bind an ephemeral port."); + } + + server = bound; + _ = server.Listen( + message => + { + if (message.Verb is ViewerVerb.Accept or ViewerVerb.Discard) + { + Heard.Enqueue($"{message.Verb} {message.Key}"); + return ViewerResponse.Success("Done"); + } + + return listing(); + }, + cancel.Token); + } + + public void Dispose() + { + cancel.Cancel(); + server.Dispose(); + cancel.Dispose(); + } + } + + internal sealed class NoWindow : IViewerWindow + { + public bool Present(Screen screen) => + true; + + public ViewerInput Poll() => + default; + + public void SetHidden(bool hidden) + { + } + + public void Focus() + { + } + + public void SetClipboard(string text) + { + } + + public bool Capture(Screen screen, int width, int height, string pngPath) => + false; + + public void Dispose() + { + } + } +} diff --git a/src/DiffEngineViewer.Tests/DerivedFilesTests.cs b/src/DiffEngineViewer.Tests/DerivedFilesTests.cs new file mode 100644 index 000000000..5f0483d8c --- /dev/null +++ b/src/DiffEngineViewer.Tests/DerivedFilesTests.cs @@ -0,0 +1,551 @@ +/// +/// The files derived from a document, shown beneath it. +/// +/// A snapshot library that splits a document into pages reports the document and every page file, +/// and a viewer drawing the document is already showing those pages. So an entry that says it was +/// derived from the document has no row of its own until asked for, and goes where the document +/// goes. Hidden is a view and never a filter, as a fold is: what is beneath a document is still +/// queued, still counted, and still taken by accept-all. +/// +/// +/// And only beneath a document this viewer is drawing, which most of what follows is about. Where +/// that does not hold, an entry that names a source is an ordinary row, which is what it always +/// was. +/// +/// +public class DerivedFilesTests +{ + [Test] + public async Task TheFilesDerivedFromADocumentHaveNoRows() + { + var state = Fixtures.DocumentWithDerived(); + + await Assert.That(Labels(state)).IsEquivalentTo( + [ + "+ Sample.Test (pdf) (5)", + "Other.Test (txt)" + ]); + // Still queued, which is what the title's count and accept-all go by + await Assert.That(state.Queue.Count).IsEqualTo(7); + await Assert.That(ScreenBuilder.Build(state).PendingCount).IsEqualTo(7); + } + + /// + /// Directly after their document, and by name, whatever order they arrived in: a tray lists + /// its files in its dictionary's order, and a page that arrives second is not the second page. + /// + [Test] + public async Task TheyFollowTheirDocumentByName() + { + var state = Fixtures.DocumentWithDerived(); + + await Assert.That(state.Queue.Select(_ => _.Name)).IsEquivalentTo( + [ + "Sample.Test (pdf)", + "Sample.Test (txt)", + "Sample.Test#page_0001 (png)", + "Sample.Test#page_0001 (txt)", + "Sample.Test#page_0002 (png)", + "Sample.Test#page_0003.verified.png", + "Other.Test (txt)" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + } + + /// + /// Every change to the queue orders it again, so ordering an ordered queue has to leave it be. + /// + [Test] + public async Task OrderingAnOrderedQueueChangesNothing() + { + var queue = Fixtures.DocumentWithDerived().Queue; + + var again = QueueProjection.Order(queue); + + await Assert.That(again.Select(_ => _.Key)).IsEquivalentTo( + queue.Select(_ => _.Key), + TUnit.Assertions.Enums.CollectionOrdering.Matching); + } + + /// + /// Unfolded, each is a row under the document saying what it adds to the document's name. + /// + [Test] + public async Task UnfoldingGivesEachARowBeneathTheDocument() + { + var unfolded = ViewerSession.Apply(Fixtures.DocumentWithDerived(), CommandKind.ToggleDerived); + + await Assert.That(Labels(unfolded)).IsEquivalentTo( + [ + "- Sample.Test (pdf) (5)", + " (txt)", + " #page_0001 (png)", + " #page_0001 (txt)", + " #page_0002 (png)", + " #page_0003.verified.png", + "Other.Test (txt)" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + } + + [Test] + public async Task UnfoldingTwiceFoldsAgain() + { + var state = Fixtures.DocumentWithDerived(); + + var round = ViewerSession.Apply(ViewerSession.Apply(state, CommandKind.ToggleDerived), CommandKind.ToggleDerived); + + await Assert.That(Labels(round)).IsEquivalentTo(Labels(state)); + } + + /// + /// The document's row carries the marker a header does, so a click on it does what a click on + /// a header does, once the document is the one on screen. The click that puts it there is a + /// selection and nothing more: reading a document never unfolds it. + /// + [Test] + public async Task AClickOnTheDocumentOnScreenUnfoldsIt() + { + var state = ViewerSession.Apply(Fixtures.DocumentWithDerived(), CommandKind.NextItem); + await Assert.That(state.Current!.Name).IsEqualTo("Other.Test (txt)"); + + var selected = Click(state, row: 0); + await Assert.That(selected.Current!.Key).IsEqualTo(Fixtures.DocumentKey); + await Assert.That(selected.Unfolded).IsEmpty(); + + var unfolded = Click(selected, row: 0); + await Assert.That(unfolded.Unfolded).Contains(Fixtures.DocumentKey); + await Assert.That(unfolded.Current!.Key).IsEqualTo(Fixtures.DocumentKey); + + var folded = Click(unfolded, row: 0); + await Assert.That(folded.Unfolded).IsEmpty(); + } + + /// + /// A click with a menu open is the click that closes it, and one on a selected row with + /// nothing beneath it is the selection it always was. + /// + [Test] + public async Task AClickThatIsNotForTheFoldLeavesItAlone() + { + var state = Fixtures.DocumentWithDerived(); + + var closed = Click(ViewerSession.OpenMenu(state, 0), row: 0); + await Assert.That(closed.Menu).IsNull(); + await Assert.That(closed.Unfolded).IsEmpty(); + + var other = ViewerSession.Apply(state, CommandKind.NextItem); + var clicked = Click(other, row: 1); + await Assert.That(clicked.Current!.Name).IsEqualTo("Other.Test (txt)"); + await Assert.That(clicked.Unfolded).IsEmpty(); + } + + static SessionState Click(SessionState state, int row) => + ViewerProgram.Apply( + state, + new(CommandKind.None, -1, row, 0, false, Fixtures.Columns, Fixtures.Rows), + link: null, + new DerivedAcceptTests.NoWindow()); + + /// + /// The document's menu says how many files its accept and its discard take, and offers the + /// opposite of how they are shown, after the items every move has. + /// + [Test] + public async Task TheDocumentsMenuCountsThemAndOffersToShowThem() + { + var state = Fixtures.DocumentWithDerived(); + + var folded = ViewerSession.OpenMenu(state, 0); + await Assert.That(folded.Menu!.Items.Take(4).Select(_ => _.Label)).IsEquivalentTo( + ["Accept move +5", "Discard +5", "Open target directory", "Expand"], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + + var unfolded = ViewerSession.OpenMenu(ViewerSession.Apply(folded, CommandKind.ToggleDerived), 0); + await Assert.That(unfolded.Menu!.Items[3].Label).IsEqualTo("Collapse"); + } + + [Test] + public async Task TheFooterCountsThemToo() + { + var buttons = ScreenBuilder.Build(Fixtures.DocumentWithDerived()).Buttons; + + await Assert.That(buttons[0].Label).IsEqualTo("Accept move +5"); + await Assert.That(buttons[1].Label).IsEqualTo("Discard +5"); + } + + /// + /// An entry with nothing beneath it has the menu and the buttons it always had. + /// + [Test] + public async Task AnEntryWithNothingBeneathItSaysNothingOfIt() + { + var state = ViewerSession.Apply(Fixtures.DocumentWithDerived(), CommandKind.NextItem); + await Assert.That(state.Current!.Name).IsEqualTo("Other.Test (txt)"); + + var opened = ViewerSession.OpenMenu(state, 1); + + await Assert.That(opened.Menu!.Items.Select(_ => _.Label)).DoesNotContain("Expand"); + await Assert.That(opened.Menu.Items[0].Label).IsEqualTo("Accept move"); + await Assert.That(ScreenBuilder.Build(state).Buttons[0].Label).IsEqualTo("Accept move"); + // And the command that shows them has nothing to show + await Assert.That(ViewerSession.Apply(state, CommandKind.ToggleDerived)).IsSameReferenceAs(state); + } + + /// + /// Stepping through the queue steps over what has no row, as it steps over a fold. + /// + [Test] + public async Task TabStepsOverThem() + { + var state = Fixtures.DocumentWithDerived(); + await Assert.That(state.Current!.Key).IsEqualTo(Fixtures.DocumentKey); + + var stepped = ViewerSession.Apply(state, CommandKind.NextItem); + + await Assert.That(stepped.Current!.Name).IsEqualTo("Other.Test (txt)"); + } + + /// + /// And into them once they have rows. + /// + [Test] + public async Task TabStepsIntoThemOnceUnfolded() + { + var unfolded = ViewerSession.Apply(Fixtures.DocumentWithDerived(), CommandKind.ToggleDerived); + + var stepped = ViewerSession.Apply(unfolded, CommandKind.NextItem); + + await Assert.That(stepped.Current!.Name).IsEqualTo("Sample.Test (txt)"); + } + + /// + /// The tray, or a second process, asking for one of them by key. A selection nobody can see is + /// not a selection, so the document is unfolded on the way. + /// + [Test] + public async Task SelectingOneFromOutsideUnfoldsItsDocument() + { + var state = Fixtures.DocumentWithDerived(); + var page = state.Queue.Single(_ => _.Name == "Sample.Test#page_0002 (png)"); + + var selected = ViewerSession.SelectKey(state, page.Key); + + await Assert.That(selected.Current!.Key).IsEqualTo(page.Key); + await Assert.That(selected.Unfolded).Contains(Fixtures.DocumentKey); + await Assert.That(QueueProjection.VisibleEntries(selected)).Contains(selected.Selected); + } + + /// + /// The same for a window attached to someone else's queue, whose listings can bring a + /// document in after one of its files: a tray lists what it holds in no order. + /// + [Test] + public async Task AListingThatAddsTheDocumentTakesTheSelectionToo() + { + var page = Fixtures.DerivedMove("#page_0001", "png"); + var state = Fixtures.Attached(InlineQueue.Empty, page); + await Assert.That(state.Current!.Key).IsEqualTo(page.Key); + + var synced = ViewerSession.Sync(state, InlineQueue.Empty, [page, Fixtures.DocumentMove()], null); + + await Assert.That(synced.Current!.Key).IsEqualTo(Fixtures.DocumentKey); + await Assert.That(Labels(synced)).IsEquivalentTo(["+ Sample.Test (pdf) (1)"]); + } + + /// + /// A page that arrives ahead of its document is the only thing in the queue, so it is what is + /// read. The document arriving puts it beneath the document, and the document is then what is + /// read: the same change, from the entry that stands for it. + /// + [Test] + public async Task ADocumentArrivingAfterItsFileTakesTheSelection() + { + var page = Fixtures.DerivedMove("#page_0001", "png"); + var state = ViewerSession.EnqueueTracked(SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows), page); + await Assert.That(state.Current!.Key).IsEqualTo(page.Key); + await Assert.That(Labels(state)).IsEquivalentTo(["Sample.Test#page_0001 (png)"]); + + var arrived = ViewerSession.EnqueueTracked(state, Fixtures.DocumentMove()); + + await Assert.That(arrived.Current!.Key).IsEqualTo(Fixtures.DocumentKey); + await Assert.That(Labels(arrived)).IsEquivalentTo(["+ Sample.Test (pdf) (1)"]); + } + + /// + /// A file arriving beneath the document on screen changes its count and nothing about what is + /// being read: not the scroll, and not the page turned to. + /// + [Test] + public async Task AFileArrivingBeneathTheDocumentOnScreenLeavesTheReaderWhereTheyWere() + { + var state = ViewerSession.EnqueueTracked( + SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows), + Fixtures.DocumentMove()); + var reading = ViewerSession.Apply(state, CommandKind.ScrollDown) with { Page = 3 }; + + var arrived = ViewerSession.EnqueueTracked(reading, Fixtures.DerivedMove("#page_0001", "png")); + + await Assert.That(arrived.Current!.Key).IsEqualTo(Fixtures.DocumentKey); + await Assert.That(arrived.ScrollTop).IsEqualTo(reading.ScrollTop); + await Assert.That(arrived.Page).IsEqualTo(3); + await Assert.That(Labels(arrived)).IsEquivalentTo(["+ Sample.Test (pdf) (1)"]); + } + + /// + /// The document gone - accepted somewhere else, or settled by a run in which it stopped + /// differing - leaves what was derived from it standing on its own, as the rows they are. + /// + [Test] + public async Task WithTheDocumentGoneTheyAreOrdinaryRows() + { + var settled = ViewerSession.Settle(Fixtures.DocumentWithDerived(), Fixtures.DocumentKey); + + await Assert.That(Labels(settled)).IsEquivalentTo( + [ + "Sample.Test (txt)", + "Sample.Test#page_0001 (png)", + "Sample.Test#page_0001 (txt)", + "Sample.Test#page_0002 (png)", + "Sample.Test#page_0003.verified.png", + "Other.Test (txt)" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + await Assert.That(QueueProjection.VisibleEntries(settled)).Contains(settled.Selected); + } + + /// + /// A source that is not a document this viewer draws. A text file with a picture taken of it, + /// say: the picture is what there is to look at, and beneath the text it would never be seen. + /// + [Test] + public async Task AFileDerivedFromSomethingThatIsNotDrawnIsAnOrdinaryRow() + { + var html = Fixtures.DerivedMove("", "html", sourceKey: null); + var screenshot = Fixtures.DerivedMove("", "png", sourceKey: html.Key); + + var state = Queue(html, screenshot); + + await Assert.That(Labels(state)).IsEquivalentTo(["Sample.Test (html)", "Sample.Test (png)"]); + await Assert.That(ScreenBuilder.Build(state).Buttons[0].Label).IsEqualTo("Accept move"); + } + + /// + /// One level, which is what a sender says: it names the outermost source that is pending. A + /// file naming one that is itself derived is not put beneath it. + /// + [Test] + public async Task AFileDerivedFromADerivedFileIsAnOrdinaryRow() + { + var page = Fixtures.DerivedMove("#page_0001", "png"); + var ofPage = Fixtures.DerivedMove("#page_0001", "txt", sourceKey: page.Key); + + var state = Queue(Fixtures.DocumentMove(), page, ofPage); + + await Assert.That(Labels(state)).IsEquivalentTo(["+ Sample.Test (pdf) (1)", "Sample.Test#page_0001 (txt)"]); + } + + /// + /// A solution's entries are one run of the queue, so a row beneath another has to be in the + /// same run. A sender never splits a document from its pages across two; this is what happens + /// if something does. + /// + [Test] + public async Task AFileInAnotherSolutionIsAnOrdinaryRow() + { + var state = Queue( + Fixtures.DocumentMove("SolutionA"), + Fixtures.DerivedMove("#page_0001", "png", "SolutionB")); + + await Assert.That(Labels(state)).IsEquivalentTo( + [ + "- SolutionA (1)", + " Sample.Test (pdf)", + "- SolutionB (1)", + " Sample.Test#page_0001 (png)" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + } + + /// + /// Beneath a solution's header the document is indented as its entries are, and its files + /// once more. + /// + [Test] + public async Task UnderASolutionHeaderTheyAreIndentedOnceMore() + { + var state = Queue( + Fixtures.DocumentMove("SolutionA"), + Fixtures.DerivedMove("#page_0001", "png", "SolutionA"), + Fixtures.Move(solution: "SolutionB")); + + var unfolded = ViewerSession.Apply(state, CommandKind.ToggleDerived); + + await Assert.That(Labels(unfolded)).IsEquivalentTo( + [ + "- SolutionA (2)", + " - Sample.Test (pdf) (1)", + " #page_0001 (png)", + "- SolutionB (1)", + " Sample.Test (txt)" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + } + + /// + /// A file that failed has no row to carry the mark while it is hidden, so the document's row + /// carries it, and its own failure first where it has one. Unfolded, each answers for itself. + /// + [Test] + public async Task ADocumentsRowAnswersForAFailureBeneathIt() + { + var state = Fixtures.DocumentWithDerived(); + var failed = state with + { + Queue = state.Queue + .Select(_ => _.Name == "Sample.Test#page_0002 (png)" ? _ with { Status = "the file is locked" } : _) + .ToList() + }; + + await Assert.That(QueueProjection.Rows(failed)[0].Status).IsEqualTo("the file is locked"); + + var unfolded = QueueProjection.Rows(ViewerSession.Apply(failed, CommandKind.ToggleDerived)); + await Assert.That(unfolded[0].Status).IsNull(); + await Assert.That(unfolded.Single(_ => _.Label == " #page_0002 (png)").Status).IsEqualTo("the file is locked"); + } + + /// + /// The one that would be silent and destructive if it were wrong: accept all takes what has + /// no row, as it takes what a fold hides. + /// + [Test] + public async Task AcceptAllTakesThem() + { + var done = new List(); + + var accepted = ViewerSession.Apply(Fixtures.DocumentWithDerived(), CommandKind.AcceptAll, Recording(done)); + + await Assert.That(accepted.Queue).IsEmpty(); + // The document after what is beneath it, so it is the last of them to leave + await Assert.That(done).IsEquivalentTo( + [ + "move Sample.Test.received.txt", + "move Sample.Test#page_0001.received.png", + "move Sample.Test#page_0001.received.txt", + "move Sample.Test#page_0002.received.png", + "delete Sample.Test#page_0003.verified.png", + "move Sample.Test.received.pdf", + "move sample.received.txt" + ], + TUnit.Assertions.Enums.CollectionOrdering.Matching); + } + + /// + /// What was said of a file is carried by every way an entry is built again: the watch reading + /// its files again, and the pair arriving again with nothing said this time. + /// + [Test] + public async Task WhatAFileWasDerivedFromSurvivesBeingReadAgain() + { + var directory = Directory.CreateTempSubdirectory("deview-derived-").FullName; + try + { + var temp = Path.Combine(directory, "Sample.Test#page_0001.received.txt"); + var target = Path.Combine(directory, "Sample.Test#page_0001.verified.txt"); + var stale = Path.Combine(directory, "Sample.Test#page_0002.verified.txt"); + var document = Path.Combine(directory, "Sample.Test.received.pdf"); + await File.WriteAllTextAsync(temp, "received"); + await File.WriteAllTextAsync(target, "verified"); + await File.WriteAllTextAsync(stale, "stale"); + var sourceKey = TrackedKeys.ForMove(document); + + var move = TrackedEntry.ForMove(temp, target, source: document); + var delete = TrackedEntry.ForDelete(stale, source: document); + await Assert.That(move.SourceKey).IsEqualTo(sourceKey); + await Assert.That(delete.SourceKey).IsEqualTo(sourceKey); + + // Unchanged, which is the entry it was with new stamps + await Assert.That(TrackedEntry.MoveAgain(move, temp, target).SourceKey).IsEqualTo(sourceKey); + await Assert.That(TrackedEntry.DeleteAgain(delete, stale).SourceKey).IsEqualTo(sourceKey); + + // Changed, which is an entry built from the files + await File.WriteAllTextAsync(temp, "what a later run received"); + await File.WriteAllTextAsync(stale, "and what it found here"); + await Assert.That(TrackedEntry.MoveAgain(move, temp, target).SourceKey).IsEqualTo(sourceKey); + await Assert.That(TrackedEntry.DeleteAgain(delete, stale).SourceKey).IsEqualTo(sourceKey); + + // And an arrival that names another says so + var other = Path.Combine(directory, "Other.Test.received.pdf"); + await Assert.That(TrackedEntry.MoveAgain(move, temp, target, source: other).SourceKey).IsEqualTo(TrackedKeys.ForMove(other)); + } + finally + { + Directory.Delete(directory, true); + } + } + + /// + /// Over the wire, into a queue this process owns: what the sender said the file was derived + /// from is on the entry, and on the listing another window is shown the queue through. + /// + [Test] + public async Task AnOwnerTakesWhatAFileWasDerivedFromOffTheWireAndListsIt() + { + var directory = Directory.CreateTempSubdirectory("deview-derived-").FullName; + try + { + var document = Path.Combine(directory, "Sample.Test.received.pdf"); + var page = Path.Combine(directory, "Sample.Test#page_0001.received.png"); + var stale = Path.Combine(directory, "Sample.Test#page_0002.verified.png"); + await File.WriteAllTextAsync(stale, "stale"); + var host = new SessionHost(SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows)); + var handler = new MessageHandler(host, Fixtures.Applied, _ => { }); + + handler.Handle(new(ViewerVerb.Move, page, Path.Combine(directory, "Sample.Test#page_0001.verified.png")) + { + Source = document + }); + handler.Handle(new(ViewerVerb.Delete, stale) + { + Source = document + }); + + var sourceKey = TrackedKeys.ForMove(document); + await Assert.That(host.State.Queue.Select(_ => _.SourceKey)).IsEquivalentTo(new string?[] {sourceKey, sourceKey}); + var listing = handler.Handle(new(ViewerVerb.ListFull)); + await Assert.That(ViewerResponse.TryParse(listing.Build(), out var parsed)).IsTrue(); + await Assert.That(parsed!.Moves.Single().SourceKey).IsEqualTo(sourceKey); + await Assert.That(parsed.Deletes.Single().SourceKey).IsEqualTo(sourceKey); + } + finally + { + Directory.Delete(directory, true); + } + } + + static SessionState Queue(params QueueEntry[] entries) + { + var state = SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows); + foreach (var entry in entries) + { + state = ViewerSession.EnqueueTracked(state, entry); + } + + return state; + } + + static List Labels(SessionState state) => + QueueProjection.Rows(state) + .Select(_ => _.Label) + .ToList(); + + /// + /// Actions that write down what they were asked to do to which file, by its name. + /// + internal static ViewerActions Recording(List done) => + Fixtures.Applied with + { + MoveFile = (temp, _) => done.Add($"move {Path.GetFileName(temp)}"), + DeleteFile = file => done.Add($"delete {Path.GetFileName(file)}") + }; +} diff --git a/src/DiffEngineViewer.Tests/DerivedScreenTests.Folded.verified.txt b/src/DiffEngineViewer.Tests/DerivedScreenTests.Folded.verified.txt new file mode 100644 index 000000000..3af3764eb --- /dev/null +++ b/src/DiffEngineViewer.Tests/DerivedScreenTests.Folded.verified.txt @@ -0,0 +1,24 @@ ++--------------------------------------------------------------------------------------------------------------------------------------+ +| Sample.Test (pdf) inline 1 of 7 | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| Pending (7) | Sample.Test.received.pdf (page 1 of 1) | Sample.Test.verified.pdf (page 1 of 1) | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| > + Sample.Test (pd> | 1 line 01 | 1 line 01 | +| Other.Test (txt) | 2 line 02 | 2 line 02 | +| | ~ 3 line 03 | ~ 3 line 03 changed | +| | 4 line 04 | 4 line 04 | +| | 5 line 05 | 5 line 05 | +| | 6 line 06 | 6 line 06 | +| | 7 line 07 | 7 line 07 | +| | 8 line 08 | 8 line 08 | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| [Accept move +5] [Discard +5] [Accept all] (Prev change) [Next change] [Changes only] [Picture only] (Prev page) (Next page) (Zoom > | ++--------------------------------------------------------------------------------------------------------------------------------------+ \ No newline at end of file diff --git a/src/DiffEngineViewer.Tests/DerivedScreenTests.MenuOnTheDocument.verified.txt b/src/DiffEngineViewer.Tests/DerivedScreenTests.MenuOnTheDocument.verified.txt new file mode 100644 index 000000000..579da5c0b --- /dev/null +++ b/src/DiffEngineViewer.Tests/DerivedScreenTests.MenuOnTheDocument.verified.txt @@ -0,0 +1,24 @@ ++--------------------------------------------------------------------------------------------------------------------------------------+ +| Sample.Test (pdf) inline 1 of 7 | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| Pending (7) | Sample.Test.received.pdf (page 1 of 1) | Sample.Test.verified.pdf (page 1 of 1) | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| > + Sample.Test (pd> | 1 line 01 | 1 line 01 | +| +-------------------------------+e 02 | 2 line 02 | +| | Accept move +5 |e 03 | ~ 3 line 03 changed | +| | Discard +5 |e 04 | 4 line 04 | +| | Open target directory |e 05 | 5 line 05 | +| | Expand |e 06 | 6 line 06 | +| | Copy Sample.Test.received.pdf |e 07 | 7 line 07 | +| | Copy Sample.Test.verified.pdf |e 08 | 8 line 08 | +| +-------------------------------+ | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| [Accept move +5] [Discard +5] [Accept all] (Prev change) [Next change] [Changes only] [Picture only] (Prev page) (Next page) (Zoom > | ++--------------------------------------------------------------------------------------------------------------------------------------+ \ No newline at end of file diff --git a/src/DiffEngineViewer.Tests/DerivedScreenTests.ReadingOneOfThem.verified.txt b/src/DiffEngineViewer.Tests/DerivedScreenTests.ReadingOneOfThem.verified.txt new file mode 100644 index 000000000..d4b5d5e3d --- /dev/null +++ b/src/DiffEngineViewer.Tests/DerivedScreenTests.ReadingOneOfThem.verified.txt @@ -0,0 +1,24 @@ ++--------------------------------------------------------------------------------------------------------------------------------------+ +| Sample.Test#page_0001 (txt) inline 4 of 7 | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| Pending (7) | Sample.Test#page_0001.received.txt | Sample.Test#page_0001.verified.txt | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| - Sample.Test (pd> | 1 the quick | 1 the quick | +| (txt) | ~ 2 brown dog | ~ 2 brown fox | +| #page_0001 (png) | 3 jumps over | 3 jumps over | +| > #page_0001 (txt) | 4 the lazy | 4 the lazy | +| #page_0002 (png) | 5 dog | 5 dog | +| #page_0003.veri> | | | +| Other.Test (txt) | | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| [Accept move] [Discard] [Accept all] (Prev change) (Next change) [Changes only] lines 1-5 of 5 | ++--------------------------------------------------------------------------------------------------------------------------------------+ \ No newline at end of file diff --git a/src/DiffEngineViewer.Tests/DerivedScreenTests.Tooltips.verified.txt b/src/DiffEngineViewer.Tests/DerivedScreenTests.Tooltips.verified.txt new file mode 100644 index 000000000..0be976ea0 --- /dev/null +++ b/src/DiffEngineViewer.Tests/DerivedScreenTests.Tooltips.verified.txt @@ -0,0 +1,21 @@ +[- Sample.Test (pdf) (5)] + temp/Sample.Test.received.pdf + to code/Sample.Test.verified.pdf + 5 files derived from it are accepted or discarded with it +[ (txt)] + temp/Sample.Test.received.txt + to code/Sample.Test.verified.txt +[ #page_0001 (png)] + temp/Sample.Test#page_0001.received.png + to code/Sample.Test#page_0001.verified.png +[ #page_0001 (txt)] + temp/Sample.Test#page_0001.received.txt + to code/Sample.Test#page_0001.verified.txt +[ #page_0002 (png)] + temp/Sample.Test#page_0002.received.png + to code/Sample.Test#page_0002.verified.png +[ #page_0003.verified.png] + code/Sample.Test#page_0003.verified.png +[Other.Test (txt)] + temp/sample.received.txt + to code/sample.verified.txt diff --git a/src/DiffEngineViewer.Tests/DerivedScreenTests.Unfolded.verified.txt b/src/DiffEngineViewer.Tests/DerivedScreenTests.Unfolded.verified.txt new file mode 100644 index 000000000..f9ce5a4c6 --- /dev/null +++ b/src/DiffEngineViewer.Tests/DerivedScreenTests.Unfolded.verified.txt @@ -0,0 +1,24 @@ ++--------------------------------------------------------------------------------------------------------------------------------------+ +| Sample.Test (pdf) inline 1 of 7 | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| Pending (7) | Sample.Test.received.pdf (page 1 of 1) | Sample.Test.verified.pdf (page 1 of 1) | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| > - Sample.Test (pd> | 1 line 01 | 1 line 01 | +| (txt) | 2 line 02 | 2 line 02 | +| #page_0001 (png) | ~ 3 line 03 | ~ 3 line 03 changed | +| #page_0001 (txt) | 4 line 04 | 4 line 04 | +| #page_0002 (png) | 5 line 05 | 5 line 05 | +| #page_0003.veri> | 6 line 06 | 6 line 06 | +| Other.Test (txt) | 7 line 07 | 7 line 07 | +| | 8 line 08 | 8 line 08 | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | +| | | | ++----------------------+-------------------------------------------------------+-------------------------------------------------------+ +| [Accept move +5] [Discard +5] [Accept all] (Prev change) [Next change] [Changes only] [Picture only] (Prev page) (Next page) (Zoom > | ++--------------------------------------------------------------------------------------------------------------------------------------+ \ No newline at end of file diff --git a/src/DiffEngineViewer.Tests/DerivedScreenTests.cs b/src/DiffEngineViewer.Tests/DerivedScreenTests.cs new file mode 100644 index 000000000..b64510514 --- /dev/null +++ b/src/DiffEngineViewer.Tests/DerivedScreenTests.cs @@ -0,0 +1,58 @@ +/// +/// What a document with files derived from it looks like, as every head draws it: the rows, the +/// footer and the menu are all text the heads are handed, so these are the coverage for all three. +/// +public class DerivedScreenTests +{ + /// + /// One row for the document, counted, and the entry of some other test after it. The files + /// beneath it are in the title's count and nowhere else, and the footer says how many the + /// accept takes. + /// + [Test] + public Task Folded() => + Verify(Fixtures.Render(Fixtures.DocumentWithDerived())); + + /// + /// Asked for, each is a row under the document, named by what it adds to the document's name. + /// + [Test] + public Task Unfolded() => + Verify(Fixtures.Render(ViewerSession.Apply(Fixtures.DocumentWithDerived(), CommandKind.ToggleDerived))); + + /// + /// One of them being read, which is an ordinary pair with nothing beneath it: its accept is + /// of that file alone. + /// + [Test] + public Task ReadingOneOfThem() + { + var state = Fixtures.DocumentWithDerived(); + var page = state.Queue.Single(_ => _.Name == "Sample.Test#page_0001 (txt)"); + return Verify(Fixtures.Render(ViewerSession.SelectKey(state, page.Key))); + } + + [Test] + public Task MenuOnTheDocument() => + Verify(Fixtures.Render(ViewerSession.OpenMenu(Fixtures.DocumentWithDerived(), 0))); + + /// + /// What each row says beyond its label. The document's says its count is of files that go + /// with it, since a number in brackets after a name does not. + /// + [Test] + public Task Tooltips() + { + var builder = new StringBuilder(); + foreach (var row in QueueProjection.Rows(ViewerSession.Apply(Fixtures.DocumentWithDerived(), CommandKind.ToggleDerived))) + { + builder.AppendLine($"[{row.Label}]"); + builder.AppendLine( + row.Tooltip is null + ? " (no tip)" + : string.Join("\n", row.Tooltip.Split('\n').Select(_ => $" {_}"))); + } + + return Verify(builder.ToString()); + } +} diff --git a/src/DiffEngineViewer.Tests/Fixtures.cs b/src/DiffEngineViewer.Tests/Fixtures.cs index c29dccf91..1e9e4f15c 100644 --- a/src/DiffEngineViewer.Tests/Fixtures.cs +++ b/src/DiffEngineViewer.Tests/Fixtures.cs @@ -164,11 +164,13 @@ public static QueueEntry Move( string name = "Sample.Test (txt)", string? solution = null, string left = Received, - string right = Expected) => + string right = Expected, + string? sourceKey = null) => QueueEntry.ForMove( $"move:temp/{name}", name, solution, + sourceKey, "temp/sample.received.txt", "code/sample.verified.txt", FileSide.OfText(left), @@ -177,11 +179,13 @@ public static QueueEntry Move( public static QueueEntry Delete( string name = "extra.verified.txt", string? solution = null, - string content = Expected) => + string content = Expected, + string? sourceKey = null) => QueueEntry.ForDelete( $"delete:code/{name}", name, solution, + sourceKey, $"code/{name}", FileSide.OfText(content)); @@ -255,8 +259,8 @@ public static SessionState Images() /// public static SessionState Document() { - var leftPage = WriteImage("page.received.png", SamplePng.Build(200, 260, 198, 64, 64)); - var rightPage = WriteImage("page.verified.png", SamplePng.Build(200, 260, 64, 150, 198)); + var leftPage = receivedPage.Value; + var rightPage = verifiedPage.Value; var left = new DocumentFile("sample.received.pdf", 1_234, DocumentFormat.Pdf, "AA"); var right = new DocumentFile("sample.verified.pdf", 1_240, DocumentFormat.Pdf, "BB"); var state = ViewerSession.EnqueueFile( @@ -284,8 +288,8 @@ public static SessionState DocumentDrawing() => /// public static SessionState DocumentInQueue() { - var leftPage = WriteImage("page.received.png", SamplePng.Build(200, 260, 198, 64, 64)); - var rightPage = WriteImage("page.verified.png", SamplePng.Build(200, 260, 64, 150, 198)); + var leftPage = receivedPage.Value; + var rightPage = verifiedPage.Value; var left = new DocumentFile("temp/sample.received.pdf", 1_234, DocumentFormat.Pdf, "AA"); var right = new DocumentFile("code/sample.verified.pdf", 1_240, DocumentFormat.Pdf, "BB"); var state = ViewerSession.EnqueueTracked( @@ -294,6 +298,7 @@ public static SessionState DocumentInQueue() "move:temp/sample.received.pdf", "Sample.Test (pdf)", null, + null, "temp/sample.received.pdf", "code/sample.verified.pdf", new(Long(false), null, null, null, left), @@ -302,6 +307,108 @@ public static SessionState DocumentInQueue() return ViewerSession.Rendered(state, "BB", new([new(rightPage, 200, 260, "RIGHT")], true)); } + /// + /// The key of the document builds, which is what a file derived + /// from it names as its source. + /// + public const string DocumentKey = "move:temp/Sample.Test.received.pdf"; + + /// + /// A document as a pending move, named and keyed the way a tracked one is: by its verified + /// file, so what is derived from it reads as that name and something more. + /// + public static QueueEntry DocumentMove(string? solution = null) + { + var left = new DocumentFile("temp/Sample.Test.received.pdf", 1_234, DocumentFormat.Pdf, "AA"); + var right = new DocumentFile("code/Sample.Test.verified.pdf", 1_240, DocumentFormat.Pdf, "BB"); + return QueueEntry.ForMove( + DocumentKey, + "Sample.Test (pdf)", + solution, + null, + "temp/Sample.Test.received.pdf", + "code/Sample.Test.verified.pdf", + new(Long(false), null, null, null, left), + new(Long(true), null, null, null, right)); + } + + /// + /// A file a snapshot library split out of , as a pending move of its + /// own that says so: #page_0001 and png for a page drawn, an empty suffix and + /// txt for what was read out of the whole document. + /// + public static QueueEntry DerivedMove( + string suffix, + string extension, + string? solution = null, + string? sourceKey = DocumentKey, + string left = Received, + string right = Expected) => + QueueEntry.ForMove( + $"move:temp/Sample.Test{suffix}.received.{extension}", + $"Sample.Test{suffix} ({extension})", + solution, + sourceKey, + $"temp/Sample.Test{suffix}.received.{extension}", + $"code/Sample.Test{suffix}.verified.{extension}", + FileSide.OfText(left), + FileSide.OfText(right)); + + /// + /// A page the document no longer has: the pending delete of the file it used to be. + /// + public static QueueEntry DerivedDelete( + string suffix, + string extension, + string? solution = null, + string? sourceKey = DocumentKey) => + QueueEntry.ForDelete( + $"delete:code/Sample.Test{suffix}.verified.{extension}", + $"Sample.Test{suffix}.verified.{extension}", + solution, + sourceKey, + $"code/Sample.Test{suffix}.verified.{extension}", + FileSide.OfText(Expected)); + + /// + /// What one failing document test leaves in a queue this process owns: the document, the + /// files derived from it - two pages drawn, the text of one, what was read out of the whole + /// document, and a page it has lost - and one pending file of some other test after them. + /// + /// The derived files arrive out of order, as they do over a tray's port, so where they end up + /// is the queue's doing. The document has its page drawn, as + /// has, so a screen of it says which page it is on rather than that it is still drawing. + /// + /// + public static SessionState DocumentWithDerived() + { + var leftPage = receivedPage.Value; + var rightPage = verifiedPage.Value; + var state = SessionState.Start(ViewerMode.Inline, Columns, Rows); + QueueEntry[] arrivals = + [ + DocumentMove(), + DerivedMove("#page_0002", "png"), + DerivedDelete("#page_0003", "png"), + DerivedMove("#page_0001", "txt"), + DerivedMove("#page_0001", "png"), + DerivedMove("", "txt"), + Move("Other.Test (txt)") + ]; + foreach (var entry in arrivals) + { + state = ViewerSession.EnqueueTracked(state, entry); + } + + state = ViewerSession.Rendered(state, "AA", new([new(leftPage, 200, 260, "LEFT")], true)); + return ViewerSession.Rendered(state, "BB", new([new(rightPage, 200, 260, "RIGHT")], true)); + } + + // The page every document scene draws, written once: the scenes are built by tests running + // side by side, and a second write of the same file would meet the first. + static readonly Lazy receivedPage = new(() => WriteImage("page.received.png", SamplePng.Build(200, 260, 198, 64, 64))); + static readonly Lazy verifiedPage = new(() => WriteImage("page.verified.png", SamplePng.Build(200, 260, 64, 150, 198))); + static string WriteImage(string name, byte[] content) { // A fixed directory and a fixed name: only the file name reaches a pane header, and a diff --git a/src/DiffEngineViewer.Tests/ImageScreenTests.cs b/src/DiffEngineViewer.Tests/ImageScreenTests.cs index 8e944c3de..fb828c644 100644 --- a/src/DiffEngineViewer.Tests/ImageScreenTests.cs +++ b/src/DiffEngineViewer.Tests/ImageScreenTests.cs @@ -71,6 +71,7 @@ public Task MoveInQueue() => "move:temp/sample.received.png", "Sample.Test (png)", null, + null, "temp/sample.received.png", "code/sample.verified.png", Received(), @@ -88,6 +89,7 @@ public Task DeleteInQueue() => "delete:code/extra.verified.png", "extra.verified.png", null, + null, "code/extra.verified.png", Expected())))); diff --git a/src/DiffEngineViewer.Tests/PixelTests.cs b/src/DiffEngineViewer.Tests/PixelTests.cs index 8141fc913..781608645 100644 --- a/src/DiffEngineViewer.Tests/PixelTests.cs +++ b/src/DiffEngineViewer.Tests/PixelTests.cs @@ -391,6 +391,7 @@ public Task NamesWithHashes() "move:temp/Notes##2.received.txt", "Notes##2 (txt)", null, + null, "temp/Notes##2.received.txt", "code/Notes##2.verified.txt", FileSide.OfText(Fixtures.Received), diff --git a/src/DiffEngineViewer.Tests/ReEnqueueTests.cs b/src/DiffEngineViewer.Tests/ReEnqueueTests.cs index caa4c24ce..0bcdce3c5 100644 --- a/src/DiffEngineViewer.Tests/ReEnqueueTests.cs +++ b/src/DiffEngineViewer.Tests/ReEnqueueTests.cs @@ -232,6 +232,7 @@ static QueueEntry Pair(string received, long written) => "move:temp/sample.received.txt", "Sample.Test (txt)", null, + null, "temp/sample.received.txt", "code/sample.verified.txt", new(received, new FileStamp(written, 1), null, null), @@ -249,6 +250,7 @@ static QueueEntry Document(bool read, string leftHash = "AA") "move:temp/sample.received.pdf", "Sample.Test (pdf)", null, + null, "temp/sample.received.pdf", "code/sample.verified.pdf", new(read ? DocumentScreenTests.LeftText : "", new FileStamp(1, 1), null, null, left), diff --git a/src/DiffEngineViewer.Tests/TrackedLabelTests.cs b/src/DiffEngineViewer.Tests/TrackedLabelTests.cs index f74cdf99f..23aa2d801 100644 --- a/src/DiffEngineViewer.Tests/TrackedLabelTests.cs +++ b/src/DiffEngineViewer.Tests/TrackedLabelTests.cs @@ -67,6 +67,7 @@ static QueueEntry Move(string project) => $"move:{project}", "sample.verified.txt", "SolutionA", + null, $"temp/{project}/sample.received.txt", $"code/SolutionA/{project}/sample.verified.txt", FileSide.OfText("received"), @@ -77,6 +78,7 @@ static QueueEntry Delete(string project) => $"delete:{project}", "extra.verified.txt", "SolutionA", + null, $"code/SolutionA/{project}/extra.verified.txt", FileSide.OfText("expected")); } diff --git a/src/DiffEngineViewer.Tests/TrackedWatchTests.cs b/src/DiffEngineViewer.Tests/TrackedWatchTests.cs index 499c159b5..cbb209f37 100644 --- a/src/DiffEngineViewer.Tests/TrackedWatchTests.cs +++ b/src/DiffEngineViewer.Tests/TrackedWatchTests.cs @@ -386,18 +386,18 @@ public async Task APairSentAgainUnchangedIsNotDiffedAgain() var (temp, target) = Pair("Sample.Test"); var host = new SessionHost(SessionState.Start(ViewerMode.Inline, Fixtures.Columns, Fixtures.Rows)); IQueueOwner owner = new MessageHandler(host, Fixtures.Applied, _ => { }); - owner.TrackMove(temp, target); + owner.TrackMove(temp, target, null); var before = host.State.Queue.Single(); File.SetLastWriteTimeUtc(temp, DateTime.UtcNow.AddMinutes(1)); - owner.TrackMove(temp, target); + owner.TrackMove(temp, target, null); var after = host.State.Queue.Single(); await Assert.That(ReferenceEquals(after.LeftRows, before.LeftRows)).IsTrue(); await Assert.That(after.LeftStamp).IsNotEqualTo(before.LeftStamp); await File.WriteAllTextAsync(temp, "what a later run received instead"); - owner.TrackMove(temp, target); + owner.TrackMove(temp, target, null); await Assert.That(host.State.Queue.Single().LeftText).IsEqualTo("what a later run received instead"); } diff --git a/src/DiffEngineViewer/AcceptBatch.cs b/src/DiffEngineViewer/AcceptBatch.cs index f5df62d3d..6c05936ab 100644 --- a/src/DiffEngineViewer/AcceptBatch.cs +++ b/src/DiffEngineViewer/AcceptBatch.cs @@ -27,6 +27,17 @@ public bool Covers(string key) => Only is null || Only.Contains(key); + /// + /// The name of the document this batch is the accept or the discard of, with the files derived + /// from it, or null for any other batch: see . + /// + /// It changes nothing about how the batch goes, only what it says when it is done. To the + /// reviewer that was one accept of one document, and "Accepted 0, plus 7 files" is the answer + /// to a question they did not ask. + /// + /// + public string? Cascade { get; init; } + /// /// How the snapshots have gone, which is the first half of what the batch says when it is done. /// diff --git a/src/DiffEngineViewer/CommandKind.cs b/src/DiffEngineViewer/CommandKind.cs index 0a9ce52cf..bfd154f9b 100644 --- a/src/DiffEngineViewer/CommandKind.cs +++ b/src/DiffEngineViewer/CommandKind.cs @@ -78,6 +78,13 @@ enum CommandKind /// ToggleGroup, + /// + /// Show or hide the files derived from the document on screen, which are otherwise beneath it + /// with no rows of their own. View only, as is: hidden or shown, + /// they are accepted and discarded with the document. + /// + ToggleDerived, + /// /// Select every line of one pane, the side of whatever is already selected. View only, and /// applied locally even when the queue belongs to someone else: what is on screen is this diff --git a/src/DiffEngineViewer/Documents/DocumentWatch.cs b/src/DiffEngineViewer/Documents/DocumentWatch.cs index 5b9bf0ded..225e1b7fd 100644 --- a/src/DiffEngineViewer/Documents/DocumentWatch.cs +++ b/src/DiffEngineViewer/Documents/DocumentWatch.cs @@ -744,19 +744,43 @@ QueueEntry Reread(QueueEntry entry) => entry.Key, entry.Name, entry.Solution, + entry.SourceKey, entry.LeftFile!, entry.TargetFile!, FileSide.Read(entry.LeftFile!, documents), FileSide.Read(entry.TargetFile!, documents)), - QueueEntryKind.Delete => QueueEntry.ForDelete( - entry.Key, - entry.Name, - entry.Solution, - entry.LeftFile!, - FileSide.Read(entry.LeftFile!, documents)), + QueueEntryKind.Delete => StillHeld( + entry, + QueueEntry.ForDelete( + entry.Key, + entry.Name, + entry.Solution, + entry.SourceKey, + entry.LeftFile!, + FileSide.Read(entry.LeftFile!, documents))), _ => entry }; + /// + /// A delete held because a move wrote its file is still held once its text has been read: the + /// hold is about what happened to the file, and reading it changes none of that. Built again + /// without it, the delete came back unmarked, and the next accept-all removed the file a move + /// had just put there. carries it for the same reason. + /// + static QueueEntry StillHeld(QueueEntry entry, QueueEntry fresh) + { + if (!entry.Written) + { + return fresh; + } + + return fresh with + { + Written = true, + Status = entry.Status + }; + } + IReadOnlySet? held; /// diff --git a/src/DiffEngineViewer/Ipc/MessageHandler.cs b/src/DiffEngineViewer/Ipc/MessageHandler.cs index d96de5288..c1c92417c 100644 --- a/src/DiffEngineViewer/Ipc/MessageHandler.cs +++ b/src/DiffEngineViewer/Ipc/MessageHandler.cs @@ -57,19 +57,19 @@ void IQueueOwner.Settle(string key, string? origin, string? member, string? valu /// put in is what the files were read as, which is all an arrival ever was. /// /// - void IQueueOwner.TrackMove(string temp, string target) + void IQueueOwner.TrackMove(string temp, string target, string? source) { var entry = Queued(TrackedKeys.ForMove(temp)) is { } queued - ? TrackedEntry.MoveAgain(queued, temp, target, documents) - : TrackedEntry.ForMove(temp, target, documents); + ? TrackedEntry.MoveAgain(queued, temp, target, documents, source) + : TrackedEntry.ForMove(temp, target, documents, source); RefuseWhenClosing(host.Mutate(_ => ViewerSession.EnqueueTracked(_, entry))); } - void IQueueOwner.TrackDelete(string file) + void IQueueOwner.TrackDelete(string file, string? source) { var entry = Queued(TrackedKeys.ForDelete(file)) is { } queued - ? TrackedEntry.DeleteAgain(queued, file, documents) - : TrackedEntry.ForDelete(file, documents); + ? TrackedEntry.DeleteAgain(queued, file, documents, source) + : TrackedEntry.ForDelete(file, documents, source); RefuseWhenClosing(host.Mutate(_ => ViewerSession.EnqueueTracked(_, entry))); } @@ -123,7 +123,12 @@ ViewerResponse IQueueOwner.Listing(bool withPatches) items, moves: queue .Where(_ => _.Kind == QueueEntryKind.Move) - .Select(_ => new ViewerResponseMove(_.Key, _.Name, _.Solution, _.LeftFile!, _.TargetFile!)) + .Select(_ => new ViewerResponseMove(_.Key, _.Name, _.Solution, _.LeftFile!, _.TargetFile!) + { + // As a tray owner says it, so whoever shows this queue lays it out as this + // process's own window does + SourceKey = _.SourceKey + }) .ToList(), deletes: queue .Where(_ => _.Kind == QueueEntryKind.Delete) @@ -131,7 +136,8 @@ ViewerResponse IQueueOwner.Listing(bool withPatches) { // As a tray owner says it, so whoever shows this queue leaves the delete out // of a bulk accept for the reason this process's own batch would - Held = ViewerSession.HeldReason(queue, _) + Held = ViewerSession.HeldReason(queue, _), + SourceKey = _.SourceKey }) .ToList(), progress: state.ListedProgress); diff --git a/src/DiffEngineViewer/Ipc/OwnerLink.cs b/src/DiffEngineViewer/Ipc/OwnerLink.cs index 7cf176e8a..a3cf0633a 100644 --- a/src/DiffEngineViewer/Ipc/OwnerLink.cs +++ b/src/DiffEngineViewer/Ipc/OwnerLink.cs @@ -394,16 +394,26 @@ List ReadChanges(ViewerResponse response) held = null; } - changes.Add(Read( + var pair = Read( held, () => QueueEntry.ForMove( move.Key, move.Name, move.Group, + move.SourceKey, move.Temp, move.Target, FileSide.Read(move.Temp, documents), - FileSide.Read(move.Target, documents)))); + FileSide.Read(move.Target, documents))); + // What the owner says it was derived from, on an entry kept because its files have + // not changed. A run that stops naming a source rewrites nothing. The same entry when + // the owner says what it said before, for the reason Read gives + if (pair.SourceKey != move.SourceKey) + { + pair = pair with { SourceKey = move.SourceKey }; + } + + changes.Add(pair); } foreach (var delete in response.Deletes) @@ -421,6 +431,7 @@ List ReadChanges(ViewerResponse response) delete.Key, delete.Name, delete.Group, + delete.SourceKey, delete.File, FileSide.Read(delete.File, documents))); // Why the owner's accept-all would leave it, where it would, said on the entry as a @@ -432,6 +443,12 @@ List ReadChanges(ViewerResponse response) entry = entry with { Status = delete.Held }; } + // As on a move above + if (entry.SourceKey != delete.SourceKey) + { + entry = entry with { SourceKey = delete.SourceKey }; + } + changes.Add(entry); } diff --git a/src/DiffEngineViewer/MenuState.cs b/src/DiffEngineViewer/MenuState.cs index c9fdc43aa..4dc9f5073 100644 --- a/src/DiffEngineViewer/MenuState.cs +++ b/src/DiffEngineViewer/MenuState.cs @@ -34,15 +34,29 @@ record MenuState(int Row, IReadOnlyList Items, IReadOnlyList Memb /// static class ContextMenu { - public static IReadOnlyList ForEntry(QueueEntry entry, bool hasSelection) + /// The entry the menu is for. + /// Whether pane text is selected, and so can be copied. + /// + /// How many files are shown beneath the entry, which is a document's: see + /// . Its accept and discard take them with it and say how many, + /// and it gains the item that shows or hides them. + /// + /// Whether those files have rows of their own at the moment. + public static IReadOnlyList ForEntry(QueueEntry entry, bool hasSelection, int derived = 0, bool unfolded = false) { var items = new List(); switch (entry.Kind) { case QueueEntryKind.Move: - items.Add(new("Accept move", CommandKind.Accept)); - items.Add(new("Discard", CommandKind.Discard)); + items.Add(new(WithDerived("Accept move", derived), CommandKind.Accept)); + items.Add(new(WithDerived("Discard", derived), CommandKind.Discard)); items.Add(new("Open target directory", CommandKind.RevealSource)); + if (derived > 0) + { + // After the three every move has, so those keep the places a hand has learned + items.Add(new(unfolded ? "Collapse" : "Expand", CommandKind.ToggleDerived)); + } + break; case QueueEntryKind.Delete: items.Add(new("Accept delete", CommandKind.Accept)); @@ -74,6 +88,21 @@ public static IReadOnlyList ForEntry(QueueEntry entry, bool hasSelecti return items; } + /// + /// An acting label saying how many files go with the entry, where any do: Accept move +6. + /// The footer's buttons say it the same way, and for the same reason: an accept that takes + /// seven files should not read as one that takes one. + /// + public static string WithDerived(string label, int derived) + { + if (derived == 0) + { + return label; + } + + return $"{label} +{derived}"; + } + /// /// What a right-click on a pane's text offers: the copying a reader would otherwise have to /// know the keys for. Nothing that acts on the entry, which the queue row's menu and the diff --git a/src/DiffEngineViewer/QueueEntry.cs b/src/DiffEngineViewer/QueueEntry.cs index b72d8ecf6..69c64b96e 100644 --- a/src/DiffEngineViewer/QueueEntry.cs +++ b/src/DiffEngineViewer/QueueEntry.cs @@ -182,6 +182,21 @@ public bool ShowsProperties(DrawingView drawing) => /// public bool Written { get; init; } + /// + /// On a move or a delete: the key of the pending move it was derived from, or null when it + /// stands alone. A page of a document, say, whose document is pending too. + /// + /// Only what was said about the file. Whether it is shown beneath that move is decided where + /// the queue is laid out (), since it turns on whether that move + /// is in the queue and is a document being drawn, neither of which an entry knows of another. + /// + /// + /// A parameter of and with no default, so every + /// place that builds a tracked entry again has to say where its source went. + /// + /// + public string? SourceKey { get; init; } + /// /// Whether one side of an entry holds what a side that arrived, or was read again, holds. /// @@ -271,6 +286,7 @@ public static QueueEntry ForMove( string key, string name, string? group, + string? sourceKey, string temp, string target, FileSide tempSide, @@ -299,12 +315,16 @@ public static QueueEntry ForMove( LeftImage: tempSide.Image, RightImage: targetSide.Image, LeftDocument: tempSide.Document, - RightDocument: targetSide.Document); + RightDocument: targetSide.Document) + { + SourceKey = sourceKey + }; public static QueueEntry ForDelete( string key, string name, string? group, + string? sourceKey, string file, FileSide current) => new( @@ -332,7 +352,10 @@ public static QueueEntry ForDelete( // The file on the right is the one that goes, so a picture being deleted is the right // side's picture. Nothing is on the left, which is the point of the entry. RightImage: current.Image, - RightDocument: current.Document); + RightDocument: current.Document) + { + SourceKey = sourceKey + }; static (string header, string text, string? warning) Expected(InlinePatch patch) { diff --git a/src/DiffEngineViewer/QueueProjection.cs b/src/DiffEngineViewer/QueueProjection.cs index a8e78ee7d..a26ada96c 100644 --- a/src/DiffEngineViewer/QueueProjection.cs +++ b/src/DiffEngineViewer/QueueProjection.cs @@ -1,7 +1,8 @@ /// /// The grouped view of the queue: solution buckets when more than one solution is represented, -/// test sub-groups when one test produced more than one change, and a deterministic order that -/// keeps , tab traversal and the drawn column one list. +/// test sub-groups when one test produced more than one change, the files derived from a document +/// beneath it, and a deterministic order that keeps , tab +/// traversal and the drawn column one list. /// /// Everything is encoded in the row labels — headers flush left, entries indented — so all three /// renderers agree with no per-renderer layout logic, and a queue with one solution and no test @@ -10,6 +11,197 @@ /// static class QueueProjection { + /// + /// Which entries of a queue are shown beneath another, and beneath which. + /// + /// A snapshot library that splits a document into its pages verifies the document and each + /// page, and when the document changes it reports them all: the document, and every page file + /// as a pending file of its own. A viewer drawing the document is already showing those + /// pages, so each of them was a row to open and accept after the one that mattered. An entry + /// that says what it was derived from () is attached to + /// that entry instead - no row of its own unless asked for, and accepted or discarded with it. + /// + /// + /// Only where all of this holds, and an entry that is not attached is an ordinary row: + /// + /// + /// The source is in the queue. One that was accepted, or settled, or never differed + /// leaves what was derived from it standing on its own. + /// The source is a move this viewer shows as a document. A text file with a picture + /// taken of it is not: its picture is what there is to look at, and hiding it beneath the + /// text would hide the review. + /// The source is derived from nothing itself. One level, which is what the sender + /// says: it names the outermost source that is pending. + /// The two are in one solution, since a solution's entries are one run of the queue + /// and a row beneath another has to be in the same run. + /// + /// + /// Decided here and nowhere else, as which rows have headers is, so the order, the rows and + /// what an accept takes with it cannot disagree about which entries those are. + /// + /// + sealed class Derivation + { + public static readonly Derivation None = new([], []); + + // For each entry, the index of the entry it is shown beneath, or -1. Empty when nothing + // in the queue is shown beneath anything, which is nearly every queue + readonly int[] sources; + + // For each entry, how many are shown beneath it + readonly int[] counts; + + Derivation(int[] sources, int[] counts) + { + this.sources = sources; + this.counts = counts; + } + + public bool Any => sources.Length > 0; + + public int SourceOf(int index) + { + if (index < 0 || + index >= sources.Length) + { + return -1; + } + + return sources[index]; + } + + public int CountOf(int index) + { + if (index < 0 || + index >= counts.Length) + { + return 0; + } + + return counts[index]; + } + + public static Derivation Of(IReadOnlyList entries) + { + // Asked for every change to the queue and every walk of it, so a queue where nothing + // names a source - one with no paged documents in it - costs one pass and nothing made + var named = false; + foreach (var entry in entries) + { + if (entry.SourceKey is not null) + { + named = true; + break; + } + } + + if (!named) + { + return None; + } + + Dictionary? documents = null; + for (var index = 0; index < entries.Count; index++) + { + if (entries[index] is { Kind: QueueEntryKind.Move, SourceKey: null, IsDocument: true } entry) + { + documents ??= new(StringComparer.Ordinal); + documents[entry.Key] = index; + } + } + + if (documents is null) + { + return None; + } + + int[]? sources = null; + int[]? counts = null; + for (var index = 0; index < entries.Count; index++) + { + var entry = entries[index]; + if (entry.SourceKey is not { } key || + !documents.TryGetValue(key, out var source) || + entries[source].Solution != entry.Solution) + { + continue; + } + + if (sources is null) + { + sources = new int[entries.Count]; + Array.Fill(sources, -1); + counts = new int[entries.Count]; + } + + sources[index] = source; + counts![source]++; + } + + if (sources is null) + { + return None; + } + + return new(sources, counts!); + } + } + + /// + /// for a queue, worked out once for as long as the queue is that one, + /// for the reason and in the way is. + /// + static Derivation DerivationOf(IReadOnlyList entries) => + derived.GetValue(entries, derivationOf); + + static readonly ConditionalWeakTable, Derivation> derived = new(); + static readonly ConditionalWeakTable, Derivation>.CreateValueCallback derivationOf = Derivation.Of; + + /// + /// The entries shown beneath the one at , in queue order: what + /// accepting or discarding it takes with it. None for an entry nothing is shown beneath, + /// which is nearly every entry. + /// + public static IReadOnlyList DerivedFrom(IReadOnlyList queue, int index) + { + var derivation = DerivationOf(queue); + if (derivation.CountOf(index) == 0) + { + return []; + } + + var indexes = new List(derivation.CountOf(index)); + for (var position = 0; position < queue.Count; position++) + { + if (derivation.SourceOf(position) == index) + { + indexes.Add(position); + } + } + + return indexes; + } + + /// + /// How many entries are shown beneath the one at , without listing + /// them: what a row, a button and a menu each say. + /// + public static int DerivedCount(IReadOnlyList queue, int index) => + DerivationOf(queue).CountOf(index); + + /// + /// The entry the one at is shown beneath, or -1 when it stands alone. + /// + public static int SourceOf(IReadOnlyList queue, int index) => + DerivationOf(queue).SourceOf(index); + + /// + /// Whether any entry of a queue is shown beneath another, and so whether there can be an entry + /// with no row while nothing is folded. + /// + public static bool AnyDerived(IReadOnlyList queue) => + DerivationOf(queue).Any; + /// /// Group-contiguous, deterministic order: solution buckets by first appearance with the /// ungrouped bucket last, entries in arrival order within a bucket except that a test's @@ -92,9 +284,80 @@ public static IReadOnlyList Order(IReadOnlyList entries) } } + return BeneathTheirSources(result); + } + + /// + /// An ordered queue with each entry that is shown beneath another put directly after it, which + /// is where its row goes when it has one. The same list when nothing is shown beneath + /// anything. + /// + /// By name beneath a source, rather than in the order they arrived. A tray's listing is its + /// dictionary's order, and a page that arrives second is not the second page; by name, a + /// document's files read page by page whichever process held them, and listing the same + /// queue twice gives the same list, which is what makes this safe to apply to a list it has + /// already been applied to. + /// + /// + static IReadOnlyList BeneathTheirSources(List ordered) + { + var derivation = Derivation.Of(ordered); + if (!derivation.Any) + { + return ordered; + } + + var beneath = new Dictionary>(); + for (var index = 0; index < ordered.Count; index++) + { + var source = derivation.SourceOf(index); + if (source < 0) + { + continue; + } + + if (!beneath.TryGetValue(source, out var list)) + { + beneath[source] = list = []; + } + + list.Add(ordered[index]); + } + + var result = new List(ordered.Count); + for (var index = 0; index < ordered.Count; index++) + { + if (derivation.SourceOf(index) >= 0) + { + continue; + } + + result.Add(ordered[index]); + if (!beneath.TryGetValue(index, out var list)) + { + continue; + } + + list.Sort(byName); + result.AddRange(list); + } + return result; } + // The key as well, so two files of one name - a move and the delete of what it replaces, say - + // still have one order + static readonly Comparison byName = (left, right) => + { + var names = string.CompareOrdinal(left.Name, right.Name); + if (names != 0) + { + return names; + } + + return string.CompareOrdinal(left.Key, right.Key); + }; + /// /// The full row list: headers inserted, labels indented, collisions disambiguated, conflicts /// marked. Assumes the queue is already ed, which every mutation ensures. @@ -122,7 +385,9 @@ enum SlotKind : byte Test, Entry, // An entry under a test's header - Member + Member, + // An entry shown beneath the document it was derived from, while that is unfolded + Derived } /// @@ -158,6 +423,7 @@ static List Walk(SessionState state) // Nothing folded is nearly every queue, and then no header's key needs making to ask var folds = state.Collapsed.Count > 0; + var derivation = DerivationOf(entries); var slots = new List(entries.Count + 1); var position = 0; while (position < entries.Count) @@ -184,6 +450,22 @@ static List Walk(SessionState state) while (position < bucketEnd) { + // An entry shown beneath its source has a row only while the source is unfolded. + // The other way about from a header, which hides its entries only once folded: + // these are the rows a reviewer was being made to step through, so hidden is what + // they are until asked for + var source = derivation.SourceOf(position); + if (source >= 0) + { + if (state.Unfolded.Contains(entries[source].Key)) + { + slots.Add(new(SlotKind.Derived, position, position + 1, header, false)); + } + + position++; + continue; + } + var group = TestGroup(entries[position]); var groupEnd = position; while (group is not null && @@ -270,9 +552,25 @@ static List Describe(SessionState state, List slots, int from, // its call site — and its tip leaves the name out for the same reason. rows.Add(EntryRow(entry, slot.Start, $"{indent} ", entry.Name, state, true)); break; + case SlotKind.Derived: + // Under its source, whose name it would repeat, so it says what it adds to it + rows.Add(EntryRow( + entry, + slot.Start, + $"{indent} ", + DerivedLabel(entries[DerivationOf(entries).SourceOf(slot.Start)], entry), + state)); + break; default: labels ??= LabelsOf(entries); - rows.Add(EntryRow(entry, slot.Start, indent, labels[slot.Start], state)); + var beneath = DerivationOf(entries).CountOf(slot.Start); + if (beneath == 0) + { + rows.Add(EntryRow(entry, slot.Start, indent, labels[slot.Start], state)); + break; + } + + rows.Add(SourceRow(entry, slot.Start, indent, labels[slot.Start], beneath, state)); break; } } @@ -280,6 +578,67 @@ static List Describe(SessionState state, List slots, int from, return rows; } + /// + /// The row of an entry others are shown beneath: marked and counted the way a header is, since + /// it stands over them as one does, and still an entry, since it is one. A click selects it. + /// + /// What is beneath it is hidden until it is unfolded, so the row answers for it: it carries + /// the failure of a file under it where it has none of its own, or a locked page would be a + /// failure with nothing on screen saying there was one, and its tip says the count is of + /// files that go with it. + /// + /// + static QueueItem SourceRow(QueueEntry entry, int index, string indent, string text, int beneath, SessionState state) + { + var folded = !state.Unfolded.Contains(entry.Key); + var status = entry.Status; + if (status is null && + folded) + { + foreach (var derived in DerivedFrom(state.Queue, index)) + { + if (state.Queue[derived].Status is { } hidden) + { + status = hidden; + break; + } + } + } + + var files = beneath == 1 + ? "1 file derived from it is accepted or discarded with it" + : $"{beneath} files derived from it are accepted or discarded with it"; + var tip = Tooltip(entry, text, false); + return new( + $"{indent}{Marker(folded)} {text} ({beneath})", + index == state.Selected, + status, + QueueRowKind.Entry, + index) + { + Tooltip = tip is null ? files : $"{tip}\n{files}" + }; + } + + /// + /// What a file adds to the name of the document it is shown beneath, which is all its row has + /// to say: #page_0001 (png) under Sample.Test (pdf). The whole name where it + /// does not begin with the document's, which is a file a sender derived and named its own way. + /// + static string DerivedLabel(QueueEntry source, QueueEntry entry) + { + // Twice, because a verified file carries two extensions, as TrackedEntry reads it + var stem = Path.GetFileNameWithoutExtension(Path.GetFileNameWithoutExtension(source.TargetFile)); + if (string.IsNullOrEmpty(stem) || + entry.Name.Length <= stem.Length || + !entry.Name.StartsWith(stem, StringComparison.Ordinal)) + { + return entry.Name; + } + + return entry.Name[stem.Length..].TrimStart(); + } + /// /// A disclosure marker, in both states. One that appeared only when folded would leave nothing /// on screen saying a group can be folded at all. @@ -301,7 +660,7 @@ public static List VisibleEntries(SessionState state) var visible = new List(); foreach (var slot in Walk(state)) { - if (slot.Kind is SlotKind.Entry or SlotKind.Member) + if (slot.Kind is SlotKind.Entry or SlotKind.Member or SlotKind.Derived) { visible.Add(slot.Start); } @@ -333,7 +692,7 @@ public static IReadOnlyList Visible(SessionState state, int body, out var selected = 0; for (var index = 0; index < slots.Count; index++) { - if (slots[index].Kind is SlotKind.Entry or SlotKind.Member && + if (slots[index].Kind is SlotKind.Entry or SlotKind.Member or SlotKind.Derived && slots[index].Start == state.Selected) { selected = index; diff --git a/src/DiffEngineViewer/ScreenBuilder.cs b/src/DiffEngineViewer/ScreenBuilder.cs index c1cca2f5e..20ea55c7e 100644 --- a/src/DiffEngineViewer/ScreenBuilder.cs +++ b/src/DiffEngineViewer/ScreenBuilder.cs @@ -332,10 +332,13 @@ static IReadOnlyList public IReadOnlySet Collapsed { get; init; } = new HashSet(); + /// + /// The documents whose derived files have rows of their own, by the document's + /// : see . + /// + /// A set of its own rather than more keys in , because it says the + /// opposite thing. A header shows its entries until it is folded, and a document hides its + /// files until it is unfolded, so in one set either every document's key would have to be + /// added as it arrived, on each of the paths an entry arrives by, or one absent key would + /// have to mean shown for a header and hidden for a document. + /// + /// + /// A view like , and by key for the reason it is by name: what is + /// unfolded stays unfolded across a re-run that sends the document and its files again. + /// + /// + public IReadOnlySet Unfolded { get; init; } = new HashSet(); + /// /// The pane text the reader has selected, or null. Carried here rather than in the frame /// because a drag survives scrolling, resizing and anything else that rebuilds a diff --git a/src/DiffEngineViewer/TrackedEntry.cs b/src/DiffEngineViewer/TrackedEntry.cs index 1e51159f0..294dcd41f 100644 --- a/src/DiffEngineViewer/TrackedEntry.cs +++ b/src/DiffEngineViewer/TrackedEntry.cs @@ -12,24 +12,59 @@ /// static class TrackedEntry { - public static QueueEntry ForMove(string temp, string target, DocumentPlugin? documents = null) => + /// The received file. + /// The file it belongs at. + /// The viewer's documents folder, which the files are read with. + /// + /// The received file of the pending move this one was derived from, as the sender said it, or + /// null: see . + /// + public static QueueEntry ForMove(string temp, string target, DocumentPlugin? documents = null, string? source = null) => QueueEntry.ForMove( TrackedKeys.ForMove(temp), $"{Name(target)} ({Extension(target)})", SolutionDirectoryFinder.Find(target), + KeyOfSource(source), temp, target, FileSide.Read(temp, documents), FileSide.Read(target, documents)); - public static QueueEntry ForDelete(string file, DocumentPlugin? documents = null) => + /// + public static QueueEntry ForDelete(string file, DocumentPlugin? documents = null, string? source = null) => QueueEntry.ForDelete( TrackedKeys.ForDelete(file), Path.GetFileName(file), SolutionDirectoryFinder.Find(file), + KeyOfSource(source), file, FileSide.Read(file, documents)); + /// + /// The key the source is queued under, which is the key gives a move + /// for that received file. The same thing a tray's listing says, so an entry reads the same + /// whichever process is holding it. + /// + static string? KeyOfSource(string? source) + { + if (source is null) + { + return null; + } + + return TrackedKeys.ForMove(source); + } + + /// + /// What an entry that arrived again was derived from. An arrival that names a source says so. + /// One that names none keeps what the queued entry had, because naming none is also what a + /// pair handed on by something that was never told looks like - a viewer started by hand on + /// the two files - and a source that has stopped being pending costs nothing to remember: an + /// entry is shown beneath its source only while the source is in the queue. + /// + static string? KeyOfSource(QueueEntry queued, string? source) => + KeyOfSource(source) ?? queued.SourceKey; + /// /// The entry for a pair whose files have been read again: the queued one with the files' new /// stamps when the two sides hold what it shows, and one built from them when they do not. @@ -39,9 +74,14 @@ public static QueueEntry ForDelete(string file, DocumentPlugin? documents = null /// the pair on every run. So the sides are asked as they were read, before anything is built /// from them. A with keeps the rows the queued entry already has. /// + /// + /// is what an arrival said the pair was derived from. The watch, + /// which is reading a file again and has been told nothing, passes none. + /// /// - public static QueueEntry MoveAgain(QueueEntry queued, string temp, string target, DocumentPlugin? documents = null) + public static QueueEntry MoveAgain(QueueEntry queued, string temp, string target, DocumentPlugin? documents = null, string? source = null) { + var sourceKey = KeyOfSource(queued, source); var tempSide = FileSide.Read(temp, documents); var targetSide = FileSide.Read(target, documents); if (queued.Kind == QueueEntryKind.Move && @@ -54,28 +94,34 @@ public static QueueEntry MoveAgain(QueueEntry queued, string temp, string target return queued with { LeftStamp = tempSide.Stamp, - RightStamp = targetSide.Stamp + RightStamp = targetSide.Stamp, + SourceKey = sourceKey }; } - return QueueEntry.ForMove(queued.Key, queued.Name, queued.Solution, temp, target, tempSide, targetSide); + return QueueEntry.ForMove(queued.Key, queued.Name, queued.Solution, sourceKey, temp, target, tempSide, targetSide); } /// /// As , for a pending delete, whose one file is its right side. /// - public static QueueEntry DeleteAgain(QueueEntry queued, string file, DocumentPlugin? documents = null) + public static QueueEntry DeleteAgain(QueueEntry queued, string file, DocumentPlugin? documents = null, string? source = null) { + var sourceKey = KeyOfSource(queued, source); var current = FileSide.Read(file, documents); if (queued.Kind == QueueEntryKind.Delete && queued.LeftFile == file && queued.Warning == current.Warning && Shows(queued.RightText, queued.RightImage, queued.RightDocument, current)) { - return queued with { LeftStamp = current.Stamp }; + return queued with + { + LeftStamp = current.Stamp, + SourceKey = sourceKey + }; } - var fresh = QueueEntry.ForDelete(queued.Key, queued.Name, queued.Solution, file, current); + var fresh = QueueEntry.ForDelete(queued.Key, queued.Name, queued.Solution, sourceKey, file, current); if (!queued.Written) { return fresh; diff --git a/src/DiffEngineViewer/ViewerProgram.cs b/src/DiffEngineViewer/ViewerProgram.cs index 58be1bf28..9f8b075db 100644 --- a/src/DiffEngineViewer/ViewerProgram.cs +++ b/src/DiffEngineViewer/ViewerProgram.cs @@ -577,6 +577,25 @@ input is Math.Max(40, input.Columns) == state.Columns && Math.Max(10, input.Rows) == state.Rows; + /// + /// A left click on an entry's row, which selects it. On the document already on screen it + /// folds or unfolds what was derived from it instead: that row carries the marker a header + /// does, and a marker that did nothing when clicked would be the only one. The first click on + /// a document is still only a selection, so reading one never unfolds it, and so is a click + /// with a menu open, which is the click closing it. + /// + static SessionState ClickEntry(SessionState state, int index) + { + if (index == state.Selected && + state.Menu is null && + ViewerSession.HasDerived(state)) + { + return ViewerSession.Apply(state, CommandKind.ToggleDerived); + } + + return ViewerSession.Apply(state, Command.Select(index)); + } + /// /// One frame of input against one state. Internal so SelectionTests can drive a drag and a /// copy the way a head does, since the clipboard and the drag are only connected here. @@ -620,7 +639,7 @@ internal static SessionState Apply(SessionState state, ViewerInput input, OwnerL var row = input.ClickedQueueItem < rows.Count ? rows[input.ClickedQueueItem] : null; if (row?.EntryIndex >= 0) { - state = ViewerSession.Apply(state, Command.Select(row.EntryIndex)); + state = ClickEntry(state, row.EntryIndex); } else if (row?.GroupKey is { } group) { @@ -759,6 +778,21 @@ static SessionState Dispatch(SessionState state, Command command, OwnerLink? lin { return ViewerSession.BeginDiscardGroup(state); } + + // A document with files shown beneath it is accepted and discarded with them, + // which is a batch as a header's is: one file a step, each outside the lock + if (ViewerSession.HasDerived(state)) + { + if (command.Kind == CommandKind.Accept) + { + return ViewerSession.BeginAcceptWithDerived(state); + } + + if (command.Kind == CommandKind.Discard) + { + return ViewerSession.BeginDiscardWithDerived(state); + } + } } return ViewerSession.Apply(state, command, ViewerActions.Real); @@ -776,6 +810,12 @@ static SessionState Dispatch(SessionState state, Command command, OwnerLink? lin return DispatchGroup(state, command.Kind, link); } + if (command.Kind is CommandKind.Accept or CommandKind.Discard && + ViewerSession.HasDerived(state)) + { + return DispatchWithDerived(state, command.Kind, link); + } + var verb = Remote(command.Kind); if (verb is null) { @@ -897,6 +937,55 @@ static SessionState DispatchGroup(SessionState state, CommandKind kind, OwnerLin }; } + /// + /// An accept or a discard of a document, against someone else's queue, with the files shown + /// beneath it: what is to a queue this + /// process owns, sent as keys. + /// + /// The files derived from it first, as the group they are, so a delete among them the owner + /// holds is left out the way a header's accept leaves it (). + /// Then the document, by its key, as any accept of one entry is sent. Both are posted to one + /// queue and sent in order, so the document is the last to leave: sent first, it left its + /// files in the listing as rows of their own for as long as they took to follow it. + /// + /// + /// An owner that predates all this needs nothing new to do it. It is sent accepts by key, + /// which it has always carried out. What it cannot do is say which files were derived from + /// which, and with none said nothing is shown beneath anything, so this is never reached. + /// + /// + static SessionState DispatchWithDerived(SessionState state, CommandKind kind, OwnerLink link) + { + var source = state.Current!; + var derived = QueueProjection + .DerivedFrom(state.Queue, state.Selected) + .Select(_ => state.Queue[_]) + .ToList(); + if (kind == CommandKind.Accept) + { + link.PostAcceptGroup( + KeysOf(derived, QueueEntryKind.Move), + [], + KeysOf(derived, QueueEntryKind.Delete)); + link.Post(ViewerVerb.Accept, source.Key); + } + else + { + foreach (var entry in derived) + { + link.Post(ViewerVerb.Discard, entry.Key); + } + + link.Post(ViewerVerb.Discard, source.Key); + } + + return state with + { + Message = "Waiting for the queue owner.", + Menu = null + }; + } + static List KeysOf(IEnumerable entries, QueueEntryKind kind) => entries .Where(_ => _.Kind == kind) diff --git a/src/DiffEngineViewer/ViewerSession.cs b/src/DiffEngineViewer/ViewerSession.cs index 2b2e75931..a245fba5c 100644 --- a/src/DiffEngineViewer/ViewerSession.cs +++ b/src/DiffEngineViewer/ViewerSession.cs @@ -202,10 +202,15 @@ public static SessionState EnqueueTracked(SessionState state, QueueEntry entry) if (currentKey is null || replacedCurrent) { - return Open(next); + next = Open(next); } - return Clamp(next); + // An arrival can put the entry being read beneath another: a document arriving after one + // of its pages, which the tray's route does not prevent, or the page arriving again and + // naming it. The document is then what is read. Otherwise the reader stays where they + // were, so a page arriving beneath the document on screen changes its count and nothing + // else + return Seen(next); } /// @@ -275,6 +280,9 @@ static bool SameContent(QueueEntry queued, QueueEntry arrived) => queued.Kind == arrived.Kind && queued.Name == arrived.Name && queued.Solution == arrived.Solution && + // Not something a pane shows, but it decides where the entry sits in the queue, and an + // entry kept where it stood would stay beneath a source it has stopped being derived from + queued.SourceKey == arrived.SourceKey && queued.LeftFile == arrived.LeftFile && queued.TargetFile == arrived.TargetFile && queued.LeftHeader == arrived.LeftHeader && @@ -343,7 +351,8 @@ public static SessionState Sync( return Reopen(next); } - return Clamp(next); + // As EnqueueTracked: a listing can put the entry being read beneath its source + return Seen(next); } /// @@ -521,13 +530,16 @@ public static SessionState OpenMenu(SessionState state, int visibleRow) } var selected = Select(state, row.EntryIndex); + var entry = selected.Queue[row.EntryIndex]; return selected with { Menu = new( fullRow, ContextMenu.ForEntry( - selected.Queue[row.EntryIndex], - selected.LiveSelection is { IsEmpty: false }), + entry, + selected.LiveSelection is { IsEmpty: false }, + QueueProjection.DerivedCount(selected.Queue, row.EntryIndex), + selected.Unfolded.Contains(entry.Key)), [row.EntryIndex]) }; } @@ -704,6 +716,8 @@ public static SessionState Apply(SessionState state, Command command, ViewerActi return menu is null || !inline ? state : DiscardGroup(state, menu, actions); case CommandKind.ToggleGroup: return menu?.GroupKey is not { } key ? state : Toggle(state, key); + case CommandKind.ToggleDerived: + return ToggleDerived(state); case CommandKind.RevealSource: return Reveal(state, actions); case CommandKind.ScrollUp: @@ -965,7 +979,11 @@ public static SessionState BeginDiscardAll(SessionState state) => /// The entries to discard, for a group header acting on its own members. Null discards /// everything, which is what the unqualified discard-all means. /// - static SessionState BeginDiscard(SessionState state, IReadOnlyList? members) + /// + /// The name of the document the batch is the discard of, when it is one: see + /// . + /// + static SessionState BeginDiscard(SessionState state, IReadOnlyList? members, string? cascade = null) { if (state.Mode != ViewerMode.Inline || state.Batch is not null) @@ -996,10 +1014,11 @@ static SessionState BeginDiscard(SessionState state, IReadOnlyList? } var only = members is null ? null : TrackedKeysOf(members).ToHashSet(); + var rebuilt = Rebuild(state, pending, discarded); var remaining = new List(); - var moves = new List(); + var discarding = new HashSet(); var untracked = 0; - foreach (var entry in Rebuild(state, pending, discarded)) + foreach (var entry in rebuilt) { if (entry.Kind is not (QueueEntryKind.Move or QueueEntryKind.Delete) || (only is not null && !only.Contains(entry.Key))) @@ -1015,15 +1034,19 @@ static SessionState BeginDiscard(SessionState state, IReadOnlyList? continue; } - moves.Add(entry.Key); + discarding.Add(entry.Key); remaining.Add(entry); } + // A document after the files beneath it, as an accept takes them, and for its reason. + // Asked of the list the deletes have left, which is the one the batch works through + var moves = FilesInBatchOrder(remaining, discarding.Contains); var batch = new AcceptBatch(moves, moves.Count) { Discarding = true, Said = said, - Swept = untracked + Swept = untracked, + Cascade = cascade }; var begun = Remove(state, remaining, null); // No received file to throw away, so nothing to report progress on @@ -1183,7 +1206,11 @@ public static SessionState BeginAcceptAll(SessionState state) => /// The keys to accept, for a group header acting on its own members. Null accepts everything, /// which is what the unqualified accept-all means. /// - static SessionState BeginAccept(SessionState state, IReadOnlyCollection? only) + /// + /// The name of the document the batch is the accept of, when it is one: see + /// . + /// + static SessionState BeginAccept(SessionState state, IReadOnlyCollection? only, string? cascade = null) { if (state.Mode != ViewerMode.Inline || state.Batch is not null) @@ -1193,7 +1220,8 @@ static SessionState BeginAccept(SessionState state, IReadOnlyCollection? var batch = new AcceptBatch([], 0) { - Only = only?.ToHashSet() + Only = only?.ToHashSet(), + Cascade = cascade }; var keys = new List(); foreach (var entry in state.Queue) @@ -1205,14 +1233,7 @@ static SessionState BeginAccept(SessionState state, IReadOnlyCollection? } } - foreach (var entry in state.Queue) - { - if (entry.Kind is QueueEntryKind.Move or QueueEntryKind.Delete && - batch.Covers(entry.Key)) - { - keys.Add(entry.Key); - } - } + keys.AddRange(FilesInBatchOrder(state.Queue, batch.Covers)); batch = batch with { @@ -1234,6 +1255,137 @@ static SessionState BeginAccept(SessionState state, IReadOnlyCollection? }; } + /// + /// The keys of the moves and deletes a batch covers, in the order it takes them: queue order, + /// but a document after the files shown beneath it. + /// + /// Taken first, the document left the queue with its files still in it, and what is shown + /// beneath a document that has gone is an ordinary row. A batch over a document and its + /// twenty pages put twenty rows on screen and took them away one at a time, for an accept + /// whose whole point was that they were never rows. Last, it leaves when they have. + /// + /// + /// For every batch, since an accept-all and a header's accept go the same way for the same + /// reason. It decides nothing about what is taken: whether a delete is held is still asked as + /// its turn comes (). + /// + /// + static List FilesInBatchOrder(IReadOnlyList queue, Func covers) + { + var keys = new List(); + string? document = null; + var documentIndex = -1; + for (var index = 0; index < queue.Count; index++) + { + var entry = queue[index]; + // What is beneath a document directly follows it, so the first entry that is not + // beneath it is where it goes + if (document is not null && + QueueProjection.SourceOf(queue, index) != documentIndex) + { + keys.Add(document); + document = null; + } + + if (entry.Kind is not (QueueEntryKind.Move or QueueEntryKind.Delete) || + !covers(entry.Key)) + { + continue; + } + + if (QueueProjection.DerivedCount(queue, index) > 0) + { + document = entry.Key; + documentIndex = index; + continue; + } + + keys.Add(entry.Key); + } + + if (document is not null) + { + keys.Add(document); + } + + return keys; + } + + /// + /// Starts an accept of the document on screen together with every file shown beneath it: the + /// accept a window makes of an entry that has any (see ). The + /// state as it is, less the menu, for an entry that has none, which is accepted the ordinary + /// way. + /// + /// A batch over those entries rather than a transition of its own, as "Accept all in" a + /// header is, and for its reason: a document with fifty pages is fifty-one files to move, and + /// moved inside one transition they held the lock the render loop takes. So it goes by the + /// batch's rules too. A file that could not be moved stays in the queue saying why, a delete + /// a move has written the file of is kept, and the rest go. + /// + /// + /// Only the window's accept. An accept by key over the wire is carried out as asked, one + /// entry, as it always was: that is another surface saying which file it means. + /// + /// + public static SessionState BeginAcceptWithDerived(SessionState state) + { + state = state with { Menu = null }; + if (WithDerived(state) is not { } entries) + { + return state; + } + + return BeginAccept( + state, + entries.Select(_ => _.Key).ToList(), + entries[0].Name); + } + + /// + /// , for a discard: the document's received file thrown + /// away with those of the files derived from it, and the deletes among them untracked. + /// + public static SessionState BeginDiscardWithDerived(SessionState state) + { + state = state with { Menu = null }; + if (WithDerived(state) is not { } entries) + { + return state; + } + + return BeginDiscard(state, entries, entries[0].Name); + } + + /// + /// Whether the entry on screen has files shown beneath it, which is when a window's accept or + /// discard of it is one of and + /// rather than of the entry alone. + /// + public static bool HasDerived(SessionState state) => + state.Mode == ViewerMode.Inline && + state.Current is not null && + QueueProjection.DerivedCount(state.Queue, state.Selected) > 0; + + /// + /// The entry on screen and what is shown beneath it, the entry first, or null when nothing is. + /// + static List? WithDerived(SessionState state) + { + if (!HasDerived(state)) + { + return null; + } + + List entries = [state.Current!]; + foreach (var index in QueueProjection.DerivedFrom(state.Queue, state.Selected)) + { + entries.Add(state.Queue[index]); + } + + return entries; + } + /// /// Claims the next entry of the running batch, so it can be applied outside the lock: it is /// in the state this returns. With nothing left to claim, the @@ -1651,6 +1803,22 @@ state with /// static SessionState Finish(SessionState state, AcceptBatch batch) { + // One document and what was derived from it, which says so rather than counting files + // beside a count of snapshots there were none of + if (batch.Cascade is { } document) + { + var said = OfDocument(batch.Discarding ? "Discarded" : "Accepted", document, batch.Swept, batch.Kept); + if (batch.KeptForAMove) + { + said = $"{said}. {DeletesKept}"; + } + + return Remove( + state with { Batch = null }, + state.Queue, + said); + } + // A discard says what its beginning said of the snapshots, then the files: worded the way // an owning tray words its own, with what stayed pending counted rather than hidden if (batch.Discarding) @@ -1683,6 +1851,31 @@ static SessionState Finish(SessionState state, AcceptBatch batch) message); } + /// + /// What an accept or a discard of a document with its derived files says when it is done: + /// the document and how many went with it, or, where any stayed, how many of the lot went. + /// What stayed is still in the queue saying why, the document among them if it was the one. + /// + /// Accepted, or Discarded. + /// The document's name. + /// The files that went, the document among them when it did. + /// The files that stayed. + static string OfDocument(string did, string document, int swept, int kept) + { + if (kept > 0) + { + return $"{did} {swept} of {swept + kept} files of {document} ({kept} kept)"; + } + + var derived = Math.Max(0, swept - 1); + if (derived == 1) + { + return $"{did} {document} and 1 derived file"; + } + + return $"{did} {document} and {derived} derived files"; + } + /// /// The list once a move has been carried out: a delete pending on the file it wrote is marked /// as written and says why it is held from here on (). The same list @@ -2539,6 +2732,46 @@ static SessionState Toggle(SessionState state, string key) return Clamp(folded); } + /// + /// Shows or hides the files derived from the document on screen. Nothing to do for an entry + /// with none, which is every entry but a document that arrived with its pages. + /// + /// Hiding them can hide the entry being read, when that is one of them, and the selection + /// then goes to the document they went under. + /// + /// + static SessionState ToggleDerived(SessionState state) + { + if (state.Current is not { } current || + QueueProjection.DerivedCount(state.Queue, state.Selected) == 0) + { + return state; + } + + var unfolded = new HashSet(state.Unfolded); + if (!unfolded.Add(current.Key)) + { + unfolded.Remove(current.Key); + } + + return Seen(state with { Unfolded = unfolded }); + } + + /// + /// The state with its selection on an entry that has a row, where the one it is on has none: + /// see . The same state otherwise, clamped, which is what every + /// path through here went on to do. + /// + static SessionState Seen(SessionState state) + { + if (NearestVisible(state) is { } visible) + { + return Select(state, visible); + } + + return Clamp(state); + } + /// /// Where the selection goes when it is under a fold, or null when it is not. The column follows /// the selection, so leaving it there would leave the whole list with nothing highlighted, @@ -2549,13 +2782,20 @@ static SessionState Toggle(SessionState state, string key) /// For a fold, and for the entry being read going: an index kept across that can name the /// first entry of a folded group that follows it. /// + /// + /// An entry hidden beneath the document it was derived from goes to that document instead, + /// where the document has a row: it is the same change being read, from the entry that + /// stands for it, and the one after it is some other test's. + /// /// static int? NearestVisible(SessionState state) { // Nothing folded, nothing hidden: every entry has a row, the selected one among them. The // answer the walk below would give, without the walk, which is every entry of the queue - // and is asked after each entry a batch takes out. - if (state.Collapsed.Count == 0) + // and is asked after each entry a batch takes out. A queue with a document's files + // beneath it has entries with no row and nothing folded, so that is asked too + if (state.Collapsed.Count == 0 && + !QueueProjection.AnyDerived(state.Queue)) { return null; } @@ -2567,6 +2807,13 @@ static SessionState Toggle(SessionState state, string key) return null; } + var source = QueueProjection.SourceOf(state.Queue, state.Selected); + if (source >= 0 && + visible.Contains(source)) + { + return source; + } + var before = -1; var after = -1; foreach (var index in visible) @@ -2644,6 +2891,21 @@ static SessionState Step(SessionState state, int delta) /// static SessionState Reveal(SessionState state, int index) { + // An entry beneath the document it was derived from has no row until that is unfolded, + // whatever else is or is not folded + var source = QueueProjection.SourceOf(state.Queue, index); + if (source >= 0 && + !state.Unfolded.Contains(state.Queue[source].Key)) + { + state = state with + { + Unfolded = new HashSet(state.Unfolded) + { + state.Queue[source].Key + } + }; + } + if (state.Collapsed.Count == 0 || QueueProjection.VisibleEntries(state).Contains(index)) { diff --git a/todo.md b/todo.md index 3479b0d8a..3ab470fd7 100644 --- a/todo.md +++ b/todo.md @@ -73,6 +73,10 @@ An item with no tag was said by whoever made the fix it follows from. A tag says - [ ] `DocumentWatch` does nothing for a window a head reports as unseen, as for a hidden one, so pages are not drawn until it is seen again. On macOS that now includes a wholly covered window: if `occlusionState` is ever wrong, pages stall. - [ ] Both sides of a document are drawn at once, and four things about that could be better: a drawing is not stopped when the reader leaves its entry, though between two pages of a PDF it could be; the pages of a PDF that is put back because the other side stopped inside PDFium are dropped, and drawn again once PDFium is free; a PDF pair's right side waits for the left's first page, which is what lets the two be told apart when both stop; and `Withdrawn`, which takes a rendering back out of the state, lives in `DocumentWatch` where it belongs beside `ViewerSession.Rendered`. - [ ] Which of two PDFs stopped inside PDFium is inferred from whose pages stopped first, not known. A thread descheduled between landing a page and asking for the lock, at the moment the other side hangs, would have the innocent side given up on and the culprit put back. +- [ ] Files derived from a document fold beneath it only where the viewer is drawing the document. A document accepted from the tray's menu, or by a viewer older than the fold, leaves its derived files as ordinary rows: the tray's accept is of one file, and its menu lists each derived file as it lists any move. +- [ ] A folded document has only its menu and a second click on its row to unfold it. A key for it is a `DeviewKey` value on both native heads, which was left out so the fold changed no ABI. +- [ ] The header's position still counts the files beneath a folded document, so `Tab` from a document with five of them goes from `1 of 7` to `7 of 7`. A folded solution reads the same way. +- [ ] The fold's rows, menu and footer are labels the three heads are handed, and were checked through the model and the ASCII screen. No pixel capture of a folded or unfolded document was added, and neither native head was run with one. ## Viewer, Windows head