diff --git a/native/src/deview.cpp b/native/src/deview.cpp
index 2b9b1734..70d9c5f2 100644
--- a/native/src/deview.cpp
+++ b/native/src/deview.cpp
@@ -539,24 +539,36 @@ int ReadKey()
return DEVIEW_KEY_NONE;
}
+ /* Letters by the character typed rather than by key position. raylib's key codes are
+ * positions on a US layout, so on AZERTY the key labelled Q reported KEY_A and accepted - a
+ * snapshot written into source by a key meant to quit - while the one labelled A quit.
+ * Characters follow the layout, the way the macOS and Windows heads already do. */
+ for (int character = GetCharPressed(); character != 0; character = GetCharPressed())
+ {
+ switch (character)
+ {
+ case 'a': return DEVIEW_KEY_ACCEPT;
+ case 'A': return DEVIEW_KEY_ACCEPT_ALL;
+ case 'd': return DEVIEW_KEY_DISCARD;
+ case 'v': return DEVIEW_KEY_NEXT_VARIANT;
+ case 'q': return DEVIEW_KEY_QUIT;
+ case 'n': return DEVIEW_KEY_NEXT_CHANGE;
+ case 'p': return DEVIEW_KEY_PREVIOUS_CHANGE;
+ case 'm': return DEVIEW_KEY_TOGGLE_MINIMAL;
+ default: break;
+ }
+ }
+
if (IsKeyPressed(KEY_UP)) return DEVIEW_KEY_SCROLL_UP;
if (IsKeyPressed(KEY_DOWN)) return DEVIEW_KEY_SCROLL_DOWN;
if (IsKeyPressed(KEY_PAGE_UP)) return DEVIEW_KEY_PAGE_UP;
if (IsKeyPressed(KEY_PAGE_DOWN)) return DEVIEW_KEY_PAGE_DOWN;
if (IsKeyPressed(KEY_HOME)) return DEVIEW_KEY_HOME;
if (IsKeyPressed(KEY_END)) return DEVIEW_KEY_END;
- if (IsKeyPressed(KEY_N)) return DEVIEW_KEY_NEXT_CHANGE;
- if (IsKeyPressed(KEY_P)) return DEVIEW_KEY_PREVIOUS_CHANGE;
- if (IsKeyPressed(KEY_M)) return DEVIEW_KEY_TOGGLE_MINIMAL;
if (IsKeyPressed(KEY_TAB)) return IsKeyDown(KEY_LEFT_SHIFT) || IsKeyDown(KEY_RIGHT_SHIFT)
? DEVIEW_KEY_PREVIOUS_ITEM
: DEVIEW_KEY_NEXT_ITEM;
- if (IsKeyPressed(KEY_A)) return IsKeyDown(KEY_LEFT_SHIFT) || IsKeyDown(KEY_RIGHT_SHIFT)
- ? DEVIEW_KEY_ACCEPT_ALL
- : DEVIEW_KEY_ACCEPT;
- if (IsKeyPressed(KEY_D)) return DEVIEW_KEY_DISCARD;
- if (IsKeyPressed(KEY_V)) return DEVIEW_KEY_NEXT_VARIANT;
- if (IsKeyPressed(KEY_Q) || IsKeyPressed(KEY_ESCAPE)) return DEVIEW_KEY_QUIT;
+ if (IsKeyPressed(KEY_ESCAPE)) return DEVIEW_KEY_QUIT;
return DEVIEW_KEY_NONE;
}
diff --git a/src/DiffEngine.Tests/InlinePatcherFsTests.cs b/src/DiffEngine.Tests/InlinePatcherFsTests.cs
index d4eeba98..36ceec2c 100644
--- a/src/DiffEngine.Tests/InlinePatcherFsTests.cs
+++ b/src/DiffEngine.Tests/InlinePatcherFsTests.cs
@@ -285,6 +285,20 @@ let TestB () =
""");
// A call above TestB's declaration is not inside TestB, whatever the hint says, so the
+ // The usual way an F# test is named. Judged from the name, the backtick in front of it is no
+ // keyword, so the member was never found and the search ran across the whole file
+ [Test]
+ public async Task MemberNameBoundsTheSearchForABacktickedName()
+ {
+ var source = Source("module Tests\n\nlet ``test a`` () =\n Verifier.Verify(a).Snapshot(\"dup\").ToTask()\n\nlet ``test b`` () =\n Verifier.Verify(b).Snapshot(\"dup\").ToTask()\n");
+
+ var status = TryApply(source, 4, InlinePatchMode.Set, null, "new", out var newSource, out _, originalValue: "dup", memberName: "test b");
+
+ await Assert.That(status).IsEqualTo(PatchStatus.Applied);
+ await Assert.That(newSource).Contains("Verify(a).Snapshot(\"dup\")");
+ await Assert.That(newSource).Contains("Verify(b).Snapshot(\"new\")");
+ }
+
// identical snapshot in the test above is not even a candidate
[Test]
public async Task MemberNameBoundsTheSearch()
diff --git a/src/DiffEngine.Tests/InlinePatcherTests.cs b/src/DiffEngine.Tests/InlinePatcherTests.cs
index ff3a2bfc..bdb4df0a 100644
--- a/src/DiffEngine.Tests/InlinePatcherTests.cs
+++ b/src/DiffEngine.Tests/InlinePatcherTests.cs
@@ -1173,6 +1173,21 @@ public async Task RemoveWithNoSnapshotCallIsAlreadyDone()
///
/// Nothing at the recorded line at all, and nothing anywhere else either, is still reported.
///
+ ///
+ /// An empty name matches everywhere and advances nothing, so the search for it never ended.
+ /// A payload of "VerifyDocx," declares one.
+ ///
+ [Test]
+ public async Task AnEmptyEntryPointIsIgnored()
+ {
+ var source = "class Tests\n{\n Task Test() =>\n Verify(a);\n}\n";
+
+ var apply = Task.Run(() => InlinePatcher.TryApply(SourceLanguage.CSharp, source, 4, InlinePatchMode.Append, null, null, null, ["VerifyDocx", ""], true, "x", out _, out _));
+
+ await Assert.That(await Task.WhenAny(apply, Task.Delay(TimeSpan.FromSeconds(10)))).IsSameReferenceAs(apply);
+ await Assert.That(await apply).IsEqualTo(PatchStatus.Applied);
+ }
+
[Test]
public async Task RemoveWithNoCallAtAll()
{
diff --git a/src/DiffEngine.Tests/OsSettingsResolverTest.cs b/src/DiffEngine.Tests/OsSettingsResolverTest.cs
index 984e59b6..1701ec50 100644
--- a/src/DiffEngine.Tests/OsSettingsResolverTest.cs
+++ b/src/DiffEngine.Tests/OsSettingsResolverTest.cs
@@ -3,6 +3,19 @@
[NotInParallel]
public class OsSettingsResolverTest
{
+ ///
+ /// Windows allows a PATH entry in quotes, and .NET Framework's Path.Combine throws on the
+ /// quote, out of the static constructor that reads PATH, so every tool lookup in the process
+ /// failed for good.
+ ///
+ [Test]
+ public async Task PathEntriesAreUnquotedAndEmptiesDropped()
+ {
+ var paths = OsSettingsResolver.ParsePath(@"C:\one;""C:\Program Files\two"" ; ;C:\thr|ee;", ';');
+
+ await Assert.That(paths).IsEquivalentTo([@"C:\one", @"C:\Program Files\two"]);
+ }
+
[Test]
public async Task Simple()
{
diff --git a/src/DiffEngine/DiffRunner.cs b/src/DiffEngine/DiffRunner.cs
index bf5ae719..142de20d 100644
--- a/src/DiffEngine/DiffRunner.cs
+++ b/src/DiffEngine/DiffRunner.cs
@@ -272,6 +272,7 @@ static LaunchResult InnerLaunch(TryResolveTool tryResolveTool, string tempFile,
}
var processId = LaunchProcess(tool, arguments);
+ ProcessCleanup.Track(command, processId);
DiffEngineTray.AddMove(tempFile, targetFile, tool.ExePath, arguments, canKill, processId);
@@ -316,6 +317,7 @@ static async Task InnerLaunchAsync(TryResolveTool tryResolveTool,
}
var processId = LaunchProcess(tool, arguments);
+ ProcessCleanup.Track(command, processId);
await DiffEngineTray.AddMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, processId);
diff --git a/src/DiffEngine/Inline/InlineApplier.cs b/src/DiffEngine/Inline/InlineApplier.cs
index 4512a9a4..6ecda9f9 100644
--- a/src/DiffEngine/Inline/InlineApplier.cs
+++ b/src/DiffEngine/Inline/InlineApplier.cs
@@ -81,7 +81,7 @@ static InlineApplyResult Run(InlinePatch patch, bool write, bool anchorOnly = fa
var normalizedPath = fullPath.ToLowerInvariant();
lock (gates.GetOrAdd(normalizedPath, static _ => new()))
{
- using var mutex = new Mutex(false, MutexName(normalizedPath));
+ using var mutex = OpenMutex(MutexName(normalizedPath));
var owned = false;
try
{
@@ -397,6 +397,32 @@ static void MoveIntoPlace(string temporary, string fullPath, Exception replaceFa
return (new UTF8Encoding(false, true), 0);
}
+ ///
+ /// Machine wide off Windows. A name with no prefix is session scoped, and on Linux and macOS a
+ /// session is a POSIX session - every terminal has its own - so an IDE applying a staged patch
+ /// and a viewer started from a terminal's test run each held a mutex of their own, both
+ /// rewrote the file, and one literal was lost. On Windows the session is the logon session,
+ /// which every process involved already shares. Falls back to the session scoped name where
+ /// the global namespace cannot be used.
+ ///
+ static Mutex OpenMutex(string name)
+ {
+ if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+ {
+ return new(false, name);
+ }
+
+ try
+ {
+ return new(false, $@"Global\{name}");
+ }
+ catch (Exception exception)
+ when (exception is UnauthorizedAccessException or IOException)
+ {
+ return new(false, name);
+ }
+ }
+
static string MutexName(string normalizedPath)
{
var hash = SHA256.HashData(Encoding.UTF8.GetBytes(normalizedPath));
diff --git a/src/DiffEngine/Inline/InlinePatcher.cs b/src/DiffEngine/Inline/InlinePatcher.cs
index f5986d46..d2ae4ade 100644
--- a/src/DiffEngine/Inline/InlinePatcher.cs
+++ b/src/DiffEngine/Inline/InlinePatcher.cs
@@ -86,6 +86,13 @@ static string[] EntryPoints(string[]? declared)
var names = new List(builtInEntryPoints);
foreach (var name in declared)
{
+ // An empty name matches everywhere and advances nothing, so the search for it never
+ // ended - while holding the file's mutex. "VerifyDocx," arrives as one from a payload
+ if (string.IsNullOrWhiteSpace(name))
+ {
+ continue;
+ }
+
if (!names.Contains(name, StringComparer.Ordinal))
{
names.Add(name);
@@ -969,7 +976,7 @@ static int NextMemberLine(string source, SourceScan scan, List lineStarts,
continue;
}
- if (scan.IsDeclaration(index) &&
+ if (scan.IsDeclaration(DeclarationStart(source, index)) &&
LeadingWhitespace(source, lineStarts, index).Length <= memberIndent)
{
return LineOf(lineStarts, index);
@@ -1096,7 +1103,7 @@ static int Clamp(int line, int lineCount) =>
if (scan.IsCode(index) &&
StartsToken(source, scan, index) &&
(end >= source.Length || !scan.IsIdentifierChar(source[end])) &&
- scan.IsDeclaration(index))
+ scan.IsDeclaration(DeclarationStart(source, index)))
{
var line = LineOf(lineStarts, index);
if (best < 0 ||
@@ -1112,6 +1119,24 @@ static int Clamp(int line, int lineCount) =>
return best < 0 ? null : best;
}
+ ///
+ /// Where a declared name starts for the purpose of asking what declares it: before the opening
+ /// backticks of an F# ``test name``, the usual way an F# test is named. Judged from the
+ /// name itself, the backtick in front of it is not a keyword, so no such member was ever found
+ /// and the search it should have bounded ran across the whole file.
+ ///
+ static int DeclarationStart(string source, int nameStart)
+ {
+ if (nameStart >= 2 &&
+ source[nameStart - 1] == '`' &&
+ source[nameStart - 2] == '`')
+ {
+ return nameStart - 2;
+ }
+
+ return nameStart;
+ }
+
static bool StartsToken(string source, SourceScan scan, int index) =>
index == 0 ||
!scan.IsIdentifierChar(source[index - 1]);
diff --git a/src/DiffEngine/OsSettingsResolver.cs b/src/DiffEngine/OsSettingsResolver.cs
index 8d5fb518..13b174f4 100644
--- a/src/DiffEngine/OsSettingsResolver.cs
+++ b/src/DiffEngine/OsSettingsResolver.cs
@@ -11,12 +11,12 @@ static OsSettingsResolver()
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
- envPaths = pathVariable.Split(';');
+ envPaths = ParsePath(pathVariable, ';');
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ||
RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
- envPaths = pathVariable.Split(':');
+ envPaths = ParsePath(pathVariable, ':');
}
else
{
@@ -24,6 +24,32 @@ static OsSettingsResolver()
}
}
+ ///
+ /// PATH as directories that can be combined with a file name. Windows allows an entry in
+ /// quotes, and some installers write them that way, and .NET Framework's Path.Combine throws
+ /// on the quote - out of this type's static constructor, so every tool lookup in the process
+ /// failed for good. Quotes and surrounding space are taken off, and whatever still holds a
+ /// character no path can is dropped, along with empty entries.
+ ///
+ internal static string[] ParsePath(string value, char separator)
+ {
+ var invalid = Path.GetInvalidPathChars();
+ var paths = new List();
+ foreach (var entry in value.Split(separator))
+ {
+ var path = entry.Trim().Trim('"').Trim();
+ if (path.Length == 0 ||
+ path.IndexOfAny(invalid) >= 0)
+ {
+ continue;
+ }
+
+ paths.Add(path);
+ }
+
+ return paths.ToArray();
+ }
+
public static bool Resolve(
string tool,
OsSupport osSupport,
diff --git a/src/DiffEngine/Process/ProcessCleanup.cs b/src/DiffEngine/Process/ProcessCleanup.cs
index f94324cc..3b925380 100644
--- a/src/DiffEngine/Process/ProcessCleanup.cs
+++ b/src/DiffEngine/Process/ProcessCleanup.cs
@@ -3,6 +3,7 @@
public static class ProcessCleanup
{
static List commands;
+ static readonly object gate = new();
static Func?, List> findAll;
static Func tryTerminateProcess;
@@ -61,7 +62,9 @@ public static void Kill(string command)
}
var matchingCommands = Commands
- .Where(_ => _.Command == command).ToList();
+ .Where(_ => _.Command == command)
+ .Where(StillRunning)
+ .ToList();
Logging.Write($"Kill: {command}. Matching count: {matchingCommands.Count}");
if (matchingCommands.Count == 0)
{
@@ -92,7 +95,55 @@ public static bool TryGetProcessInfo(string command, out ProcessCommand process)
}
process = commands.FirstOrDefault(_ => _.Command == command);
- return !process.Equals(default(ProcessCommand));
+ if (process.Equals(default(ProcessCommand)))
+ {
+ return false;
+ }
+
+ if (StillRunning(process))
+ {
+ return true;
+ }
+
+ Forget(process);
+ process = default;
+ return false;
+ }
+
+ ///
+ /// A tool this process started, so a relaunch or a kill later in the same run finds it. The
+ /// list is otherwise taken once, when the type initialises.
+ ///
+ internal static void Track(string command, int processId)
+ {
+ if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+ {
+ command = TrimCommand(command);
+ }
+
+ lock (gate)
+ {
+ commands = [new(command, processId), ..commands];
+ }
+ }
+
+ ///
+ /// Whether the process a snapshot of the list named is still the one it named. The list is
+ /// taken once per test process, so a tool closed since then has left a PID that Windows can
+ /// hand to anything else - and killing by that PID terminated whatever got it. Asked only on a
+ /// hit, which is rare, and answered by reading that process's command line again.
+ ///
+ static bool StillRunning(ProcessCommand process) =>
+ findAll(CandidateExeNames())
+ .Any(_ => _.Process == process.Process &&
+ _.Command == process.Command);
+
+ static void Forget(ProcessCommand process)
+ {
+ lock (gate)
+ {
+ commands = commands.Where(_ => !_.Equals(process)).ToList();
+ }
}
static void TerminateProcessIfExists(in int processId)
diff --git a/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so b/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so
index 4453961b..59317ee2 100644
Binary files a/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so and b/src/DiffEngineViewer.Linux/runtimes/linux-arm64/native/libdiffengine_viewer.so differ
diff --git a/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so b/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so
index f4e18c99..8be542a9 100644
Binary files a/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so and b/src/DiffEngineViewer.Linux/runtimes/linux-x64/native/libdiffengine_viewer.so differ
diff --git a/src/DiffEngineViewer.Tests/ViewerSessionTests.cs b/src/DiffEngineViewer.Tests/ViewerSessionTests.cs
index afc37b5c..8a717619 100644
--- a/src/DiffEngineViewer.Tests/ViewerSessionTests.cs
+++ b/src/DiffEngineViewer.Tests/ViewerSessionTests.cs
@@ -780,6 +780,40 @@ public async Task AQueueChangeClosesTheMenu()
await Assert.That(synced.Menu).IsNull();
}
+ ///
+ /// An attached viewer syncs five times a second, almost always to the same queue. Each one
+ /// cleared the open menu, so a right-click menu closed within 200ms.
+ ///
+ [Test]
+ public async Task AnUnchangedListingKeepsTheMenuOpen()
+ {
+ var open = ViewerSession.OpenMenu(Fixtures.Attached(Fixtures.Pending(Fixtures.Patch())), 0);
+ await Assert.That(open.Menu).IsNotNull();
+
+ var synced = ViewerSession.Sync(open, Fixtures.Pending(Fixtures.Patch()), [], null);
+
+ await Assert.That(synced.Menu).IsNotNull();
+ }
+
+ ///
+ /// Copying a whole side hands over the file's lines. Flattened for the screen, every tab
+ /// became four spaces, and pasting that into a verified file changed it.
+ ///
+ [Test]
+ public async Task CopyingAWholeSideKeepsTabs() =>
+ await Assert.That(SelectionText.All(Fixtures.Move(left: "a\tb", right: "a\tb"), PaneSide.Left)).IsEqualTo("a\tb");
+
+ [Test]
+ public async Task AFrameWithNothingInItTakesNoLock()
+ {
+ var state = Fixtures.Inline(Fixtures.Patch());
+ var idle = new ViewerInput(CommandKind.None, -1, -1, 0, false, state.Columns, state.Rows);
+
+ await Assert.That(ViewerProgram.IsIdle(idle, state)).IsTrue();
+ await Assert.That(ViewerProgram.IsIdle(idle with { ScrollDelta = 1 }, state)).IsFalse();
+ await Assert.That(ViewerProgram.IsIdle(idle with { Columns = state.Columns + 1 }, state)).IsFalse();
+ }
+
///
/// A settle that empties the queue sets Exit, and the loop acts on it a frame later. An arrival
/// in between is a reason to stay: carried across, Exit took the new entry out with the window.
diff --git a/src/DiffEngineViewer/Ipc/MessageHandler.cs b/src/DiffEngineViewer/Ipc/MessageHandler.cs
index a4809719..b7350753 100644
--- a/src/DiffEngineViewer/Ipc/MessageHandler.cs
+++ b/src/DiffEngineViewer/Ipc/MessageHandler.cs
@@ -122,31 +122,32 @@ bool IQueueOwner.Has(string key) =>
(bool ok, string? message) Act(string key, CommandKind command, string? origin = null)
{
- var index = IndexOf(host.State, key);
- if (index < 0)
- {
- return (false, null);
- }
-
- // Refused before anything moves, and as a wire error, matching the tray owner: an
- // un-targeted accept of a conflicted entry has no honest way to pick a side.
- var entry = host.State.Queue[index];
- if (command == CommandKind.Accept &&
- origin is null &&
- entry.Conflicted)
- {
- return (false, new PendingInline(entry.Variants, entry.Status).ConflictRefusal);
- }
-
+ // Looked up and refused inside the same mutation that acts, rather than on a read taken
+ // before it: the queue can change in between, which threw on an index that had gone, and
+ // let an accept through on an entry a second framework had just made a conflict of.
+ var found = false;
+ string? refusal = null;
var state = host.Mutate(_ =>
{
- var found = IndexOf(_, key);
- if (found < 0)
+ var index = IndexOf(_, key);
+ if (index < 0)
+ {
+ return _;
+ }
+
+ found = true;
+ // Refused before anything moves, and as a wire error, matching the tray owner: an
+ // un-targeted accept of a conflicted entry has no honest way to pick a side.
+ var entry = _.Queue[index];
+ if (command == CommandKind.Accept &&
+ origin is null &&
+ entry.Conflicted)
{
+ refusal = new PendingInline(entry.Variants, entry.Status).ConflictRefusal;
return _;
}
- var selected = ViewerSession.Apply(_, Command.Select(found));
+ var selected = ViewerSession.Apply(_, Command.Select(index));
if (origin is not null)
{
selected = ViewerSession.SelectVariant(selected, origin);
@@ -155,6 +156,16 @@ origin is null &&
return ViewerSession.Apply(selected, command, actions);
});
+ if (!found)
+ {
+ return (false, null);
+ }
+
+ if (refusal is not null)
+ {
+ return (false, refusal);
+ }
+
return (true, state.Message);
}
diff --git a/src/DiffEngineViewer/SelectionText.cs b/src/DiffEngineViewer/SelectionText.cs
index 222b0981..2aaa992f 100644
--- a/src/DiffEngineViewer/SelectionText.cs
+++ b/src/DiffEngineViewer/SelectionText.cs
@@ -115,13 +115,18 @@ public static string Of(TextSelection selection, QueueEntry entry)
///
/// One whole side, which is what the copy commands that name a pane hand over. Filler rows are
/// dropped for the same reason they are dropped from a selection.
+ ///
+ /// The file's own text rather than the flattened row: a whole side has no columns to keep in
+ /// step with the screen, and flattening turned every tab into four spaces, so pasting a copied
+ /// side into a verified file changed it.
+ ///
///
public static string All(QueueEntry entry, PaneSide side) =>
string.Join(
"\n",
Rows(entry, side)
.Where(_ => _.Kind != RowKind.Filler)
- .Select(_ => RowText.Flatten(_.Text)));
+ .Select(_ => _.Text));
///
/// What the status line says while something is selected. The universal statement about a
diff --git a/src/DiffEngineViewer/ViewerProgram.cs b/src/DiffEngineViewer/ViewerProgram.cs
index 5959ce89..b3ce0796 100644
--- a/src/DiffEngineViewer/ViewerProgram.cs
+++ b/src/DiffEngineViewer/ViewerProgram.cs
@@ -346,7 +346,14 @@ static void Loop(
}
var input = window.Poll();
- host.Mutate(_ => Apply(_, input, link, window));
+ // Not on a frame with nothing in it, which is almost all of them. The listener thread
+ // takes the same lock to accept a snapshot, which can wait ten seconds on
+ // InlineApplier's mutex, and taking it every frame put the render loop behind that
+ // wait - the stall SessionHost's lock free reads exist to prevent.
+ if (!IsIdle(input, state))
+ {
+ host.Mutate(_ => Apply(_, input, link, window));
+ }
// An accept-all this frame's input began is carried out on a worker, so this thread
// goes back to drawing the queue as it shrinks. One the tray began is already being
@@ -379,7 +386,11 @@ static void Loop(
// Hidden rather than exited even when the tray owns the queue and could relaunch:
// staying up makes reopening a focus rather than a process start, and the tray tracks
// the process it launched, so it sends that focus instead of starting a second one.
- if (TrayDetector.IsRunning() &&
+ //
+ // Never in file mode, which owns no port and which the tray does not know about:
+ // nothing could ever show it again, and a caller blocked on the process waited forever.
+ if (host.State.Mode == ViewerMode.Inline &&
+ TrayDetector.IsRunning() &&
host.State.Queue.Count > 0)
{
window.SetHidden(true);
@@ -390,6 +401,24 @@ static void Loop(
}
}
+ ///
+ /// A frame that would change nothing: no key, click, scroll, drag or close, and the window
+ /// the size the state already is.
+ ///
+ internal static bool IsIdle(ViewerInput input, SessionState state) =>
+ input.Key == CommandKind.None &&
+ input.ClickedButton < 0 &&
+ input.ClickedQueueItem < 0 &&
+ input.ScrollDelta == 0 &&
+ !input.CloseRequested &&
+ input.RightClickedQueueItem < 0 &&
+ input.ClickedMenuItem < 0 &&
+ !input.MenuClosed &&
+ input.ScrollTo < 0 &&
+ input.DragSide < 0 &&
+ Math.Max(40, input.Columns) == state.Columns &&
+ Math.Max(10, input.Rows) == state.Rows;
+
///
/// 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.
diff --git a/src/DiffEngineViewer/ViewerSession.cs b/src/DiffEngineViewer/ViewerSession.cs
index 13c6a27c..46242968 100644
--- a/src/DiffEngineViewer/ViewerSession.cs
+++ b/src/DiffEngineViewer/ViewerSession.cs
@@ -193,6 +193,18 @@ public static SessionState Sync(
var entries = new List(Project(state, pending));
entries.AddRange(changes);
var queue = QueueProjection.Order(entries);
+
+ // A listing that changed nothing, which is most of them at five a second. Project and
+ // ReadChanges hand back the same entries when nothing moved, so the list can be compared
+ // by reference. Replacing it anyway cleared the open menu on every poll, so an attached
+ // viewer's right-click menu closed within 200ms and a click on it went nowhere.
+ if (message is null &&
+ progress == state.OwnerProgress &&
+ SameEntries(queue, state.Queue))
+ {
+ return state;
+ }
+
var key = state.Current?.Key;
var selected = key is null ? -1 : IndexOf(queue, key);
var next = state with
@@ -203,8 +215,9 @@ public static SessionState Sync(
OwnerProgress = progress,
// Nothing left to show, and this window is not what is holding the queue.
Exit = queue.Count == 0,
- // The open menu indexes the queue it was opened over, which was just replaced.
- Menu = null
+ // The open menu indexes the queue it was opened over. Kept when the entries are the
+ // same ones in the same places, since its indexes still mean what they did.
+ Menu = SameEntries(queue, state.Queue) ? state.Menu : null
};
if (selected < 0)
@@ -1391,6 +1404,24 @@ static bool VariantsMatch(IReadOnlyList left, IReadOnlyList left, IReadOnlyList right)
+ {
+ if (left.Count != right.Count)
+ {
+ return false;
+ }
+
+ for (var index = 0; index < left.Count; index++)
+ {
+ if (!ReferenceEquals(left[index], right[index]))
+ {
+ return false;
+ }
+ }
+
+ return true;
+ }
+
static int IndexOf(IReadOnlyList queue, string key)
{
for (var index = 0; index < queue.Count; index++)
diff --git a/todo.md b/todo.md
index 5b51aeac..21a20f45 100644
--- a/todo.md
+++ b/todo.md
@@ -64,52 +64,52 @@ Findings from a review of `main` at 4244ebe6 (2026-09-23).
## Bugs
-- [ ] **Attached viewer's right-click menu closes within 200 ms** (repro)
+- [x] **Attached viewer's right-click menu closes within 200 ms** (repro)
- `src/DiffEngineViewer/ViewerSession.cs:189`: `Sync` always sets `Menu = null`. `OwnerLink.List` calls it on every poll (`src/DiffEngineViewer/Ipc/OwnerLink.cs:120`), `ViewerForm.ApplyMenu` then closes the popup, and a later click is dropped (`ViewerProgram.cs:385`). Every viewer is attached when the tray owns the queue, which is the default on Windows.
- `EnqueueInline` also clears the menu when an identical snapshot is re-sent.
- Fix: when the new queue is element-wise `ReferenceEquals` to `state.Queue` (`Project` and `ReadChanges` already reuse unchanged entries), keep `Menu`; if the message is null and progress unchanged too, return `state` itself. `AQueueChangeClosesTheMenu` only covers a listing that changed.
-- [ ] **.NET Framework: one quoted PATH entry breaks DiffTools for the whole process** (verified)
+- [x] **.NET Framework: one quoted PATH entry breaks DiffTools for the whole process** (verified)
- `src/DiffEngine/OsSettingsResolver.cs:14` splits PATH with no cleanup, and `:152` `Path.Combine("\"C:\\Program Files\\Foo\\bin\"", name)` throws `ArgumentException: Illegal characters in path` on net4x (checked in Windows PowerShell 5.1). It runs in `DiffTools`' static constructor, so it is a permanent `TypeInitializationException`, and `DiffRunner.Kill` throws on every passing test.
- Fix: trim whitespace and quotes, and drop entries that are empty or contain invalid path characters.
-- [ ] **`DiffEngineViewer left right` never exits while the tray runs** (verified)
+- [x] **`DiffEngineViewer left right` never exits while the tray runs** (verified)
- `src/DiffEngineViewer/ViewerProgram.cs:354`: hide-instead-of-exit has no mode check. File mode owns no port and the tray does not know the process, so X, Close, q and Esc hide it forever, and a blocking `git difftool` style caller hangs.
- Fix: add `host.State.Mode == ViewerMode.Inline &&`.
-- [ ] **F# double-backtick test names get no MemberName narrowing** (repro)
+- [x] **F# double-backtick test names get no MemberName narrowing** (repro)
- `src/DiffEngine/Inline/InlinePatcher.cs:991-994` (`MemberLine`) and `src/DiffEngine/Inline/FsLanguage.cs:120` (`IsDeclaration`): for `let ``test b`` () =` the character before the name is a backtick, so it is not a declaration, `MemberLine` returns null, and the search is hint only. `NextMemberLine` never sees backticked siblings either. Double backticks are the common F# test naming style.
- Repro: two tests ` ``test a`` ` and ` ``test b`` ` with identical literals, stale hint on test a, member "test b": test a's literal is rewritten.
- Fix: when the name is enclosed in double backticks, judge the declaration from before the opening pair.
-- [ ] **An empty entry-point name hangs the patcher** (repro)
+- [x] **An empty entry-point name hangs the patcher** (repro)
- `src/DiffEngine/Inline/InlinePatcher.cs:915`: `IndexOf("", i, n)` returns `i` and `index += 0`, so `CallsOnLine` spins (or grows `matches` until out of memory) while holding the per-file mutex. `InlinePatchFile.TryParse` produces `["VerifyDocx", ""]` from a payload of `"VerifyDocx,"`.
- Fix: skip null or empty (ideally any non-identifier) names in `EntryPoints()` (:78).
-- [ ] **"Copy received" and "Copy expected" turn tabs into four spaces** (repro)
+- [x] **"Copy received" and "Copy expected" turn tabs into four spaces** (repro)
- `src/DiffEngineViewer/SelectionText.cs:119` (`All`) goes through `RowText.Flatten`. A whole-side copy has no columns to keep aligned. Pasting the result into a verified file changes it.
- Fix: `.Select(_ => _.Text)`.
-- [ ] **Linux shortcuts follow physical key position rather than layout** (verified)
+- [x] **Linux shortcuts follow physical key position rather than layout** (verified)
- `native/src/deview.cpp:526` (`ReadKey`) tests raylib/GLFW key tokens, which are US key positions. On AZERTY the key labelled Q sends `KEY_A`, which accepts (with Shift, accepts all), and the key labelled A sends `KEY_Q`, which quits; Ctrl+A and Ctrl+Q are swapped. The macOS and WinForms heads follow the layout.
- Fix: drain `GetKeyPressed()` and map letters through `GetKeyName(key)`, falling back to the position for non-Latin layouts. `IsKeyPressedRepeat` for navigation keys.
-- [ ] **InlineApplier's mutex is session scoped on macOS and Linux** (verified against .NET semantics)
+- [x] **InlineApplier's mutex is session scoped on macOS and Linux** (verified against .NET semantics)
- `src/DiffEngine/Inline/InlineApplier.cs:84,355-365`: `DiffEngineInline_` has no prefix, so it is Local, which on Unix means per POSIX session, and every terminal is its own session. Rider's plugin and a viewer launched from a terminal test run do not exclude each other: both read, both swap, the later rename wins, and one literal is silently lost while both report Applied.
- Fix: `Global\` prefix off Windows, falling back to Local on `UnauthorizedAccessException` or `IOException`.
-- [ ] **ProcessCleanup's process list is taken once and PIDs are not re-checked** (verified)
+- [x] **ProcessCleanup's process list is taken once and PIDs are not re-checked** (verified)
- `src/DiffEngine/Process/ProcessCleanup.cs:27` (the only `Refresh` call, in the static constructor), `:94` (`TryGetProcessInfo`), `src/DiffEngine/Process/WindowsProcess.cs:155` (`TryTerminateProcess` kills whatever holds the PID).
- A tool window from a previous run closed mid run: an AutoRefresh tool is reported `AlreadyRunningAndSupportsRefresh` and no window opens. If Windows has reused that PID, a passing test's `Kill`, or a replacement launch, terminates an unrelated process. Tools launched in this run never enter the list.
- Fix: on a hit, re-read that PID's command line and drop it if it no longer matches; remove entries once terminated; add `(command, pid)` after a launch.
-- [ ] **Slow work runs inside SessionHost's lock, and the render loop takes that lock every frame** (verified)
+- [x] **Slow work runs inside SessionHost's lock, and the render loop takes that lock every frame** (verified)
- `src/DiffEngineViewer/ViewerProgram.cs:321`: `host.Mutate(_ => Apply(...))` every frame, even with no input.
- `src/DiffEngineViewer/Ipc/MessageHandler.cs:47-51`: `TrackedEntry.ForMove`/`ForDelete` (file reads, SHA-256 of images, the DiffPlex diff) is evaluated inside the `Mutate` lambda, contrary to the comment above it. `Act` (:102-133) runs `InlineApplier`, with its up to 10 s mutex wait, inside `Mutate`.
- The window stalls, which `SessionHost`'s doc says lock-free reads exist to prevent.
- Fix: build tracked entries before `Mutate`; skip the per-frame `Mutate` when the input is empty and the size unchanged; apply wire accepts outside the lock the way `AcceptAllRunner` does.
-- [ ] **`MessageHandler.Act` checks the conflict refusal outside the lock** (verified)
+- [x] **`MessageHandler.Act` checks the conflict refusal outside the lock** (verified)
- `src/DiffEngineViewer/Ipc/MessageHandler.cs:102-133` reads `host.State` twice, so `Queue[index]` can throw `ArgumentOutOfRangeException`, and the conflict check can pass just before a second framework's patch makes the entry conflicted, after which the accept picks a side.
- Fix: do the lookup and the refusal inside the `Mutate` lambda.