Skip to content

Fix the verified bugs from the review - #881

Merged
SimonCropp merged 1 commit into
mainfrom
fix-bugs
Sep 23, 2026
Merged

SimonCropp merged 1 commit into
mainfrom
fix-bugs

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

The verified bugs from the review in todo.md, one fix each.

  • Attached viewer's context menu closed within 200ms. Sync replaced the queue and cleared the menu on every poll, even when nothing changed. An unchanged listing now returns the state as it was, and a changed one keeps the menu while the entries are the same ones in the same places.
  • One quoted PATH entry broke DiffTools on .NET Framework. Path.Combine throws on the quote, from the static constructor that reads PATH. Entries are now unquoted, and empty or invalid ones dropped.
  • DiffEngineViewer left right never exited while the tray ran. Closing hid the window in file mode too, where nothing can show it again. Hiding is now for inline mode only.
  • F# double-backtick test names got no member narrowing. The backtick before the name is not a declaring keyword, so the member was never found. Declarations are now judged from before the opening backticks.
  • An empty entry-point name hung the patcher under the file's mutex. Empty names are skipped.
  • Copying a whole side turned tabs into spaces. It now copies the file's lines rather than the flattened screen text.
  • Linux shortcuts followed key position rather than layout, so on AZERTY the key labelled Q accepted. Letters are now read as typed characters.
  • The inline applier's mutex was per terminal session on Linux and macOS, so an IDE and a viewer started from a terminal did not exclude each other. It is machine wide there now, falling back to the session name.
  • ProcessCleanup killed by a PID from a list taken once per run, which a closed tool's reused PID made into an unrelated process. A hit is now re-checked against that process's command line, and tools launched in the run are added to the list.
  • The render loop took the session lock every frame, behind an accept that can wait ten seconds on the applier's mutex. Frames with no input no longer take it.
  • A wire accept checked for a conflict outside the lock. The lookup and the refusal now happen in the mutation that acts.

The Linux key change is in native/src/deview.cpp, so this push runs build-native, which should open its own PR with the rebuilt binaries.

Run locally on Windows, all passing: DiffEngine.Tests 703, DiffEngineViewer.Tests 359, DiffEngineTray.Tests 246, and a Release build of the solution.

Eleven fixes, each described in the pull request: the attached viewer's menu, quoted PATH entries on .NET Framework, file mode never exiting under a tray, F# backticked member names, an empty entry point hanging the patcher, tabs lost copying a side, Linux shortcuts following key position, a session scoped applier mutex on Unix, killing by a stale PID, the render loop taking the session lock every frame, and a wire accept checking for a conflict outside the lock.
@SimonCropp
SimonCropp merged commit a9911f7 into main Sep 23, 2026
15 of 17 checks passed
@SimonCropp
SimonCropp deleted the fix-bugs branch September 23, 2026 01:58
SimonCropp added a commit that referenced this pull request Sep 23, 2026
Every item that was unverified, checked on c37bf9e: all of them hold, and
none was fixed by #878, #881 or #884. Each now says whether it was reproduced,
verified by reading, or needs a platform this machine lacks, with the current
lines, the repro test names, and where the suggested fix turned out wrong or
incomplete.

An attached "Accept all in <solution>" deleting verified files whose snapshots
were not written moves to Data loss. The repro tests are on a local branch,
review-repros, rather than here: each fails until its item is fixed.
SimonCropp added a commit that referenced this pull request Sep 23, 2026
…885)

* Make the Linux viewer draw frames, read input and wait between them

raylib 6.0's CMake reads every SUPPORT_ flag in config.h into an option that
defaults to ON, including the ones config.h sets to 0, and CUSTOMIZE_BUILD
skips config.h's own values. So SUPPORT_CUSTOM_FRAME_CONTROL was on, and
EndDrawing never swapped a frame onto the screen, never polled input and never
waited for the next frame: the Linux window stayed blank and ignored every key
and click while the loop spun. SUPPORT_BUSY_WAIT_LOOP was on the same way, and
would have spun through the whole of every frame's wait once there was one.

Switch both off, with every other flag config.h defaults to 0, and turn off
SUPPORT_SCREEN_CAPTURE, whose F12 handler becomes reachable once input is read.

PixelTests.PresentWaitsForTheNextFrame times sixty presents against the
renderer the Ubuntu job builds. The pixel snapshots could never have noticed:
a capture draws into a texture and never reaches EndDrawing.

* Record what checking the review's unverified items found

Every item that was unverified, checked on c37bf9e: all of them hold, and
none was fixed by #878, #881 or #884. Each now says whether it was reproduced,
verified by reading, or needs a platform this machine lacks, with the current
lines, the repro test names, and where the suggested fix turned out wrong or
incomplete.

An attached "Accept all in <solution>" deleting verified files whose snapshots
were not written moves to Data loss. The repro tests are on a local branch,
review-repros, rather than here: each fails until its item is fixed.

* Rebuild native renderer binaries (#886)

Co-authored-by: SimonCropp <122666+SimonCropp@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: SimonCropp <122666+SimonCropp@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant