{{ message }}
Repository navigation
fix(desktop): give the renderer a platform-native path bridge (#5683, #5669) - #5691
Merged
Merged
Conversation
Contributor
Jocs
force-pushed
the
poc/std-path-windows
branch
3 times, most recently
from
October 10, 2026 04:19
4f0884b to
9c4377f
Compare
Jocs
changed the title
Jocs
marked this pull request as ready for review
October 10, 2026 04:19
…5669) Follow-up to #5689. Rather than comparing mixed-separator paths, remove the mismatch at its source: the renderer and its preload now use a real Windows path module, so every path they build matches the native paths from the watcher, the main process and the OS. - add `createPlatformPath(isWindows)` (`@jsr/std__path` win32 on Windows, `pathe` on POSIX) and use it for the preload's `window.path` - alias the bare `path` specifier in the renderer build to the same platform path, so shared helpers like `common/envPaths` stop producing `/` paths on Windows - title bar breadcrumb: split on the platform separator — fixes #5669, where a native `C:\…` path was split on `/` and only the filename showed - replace the last hardcoded `/` filesystem join (`CREATE_FILE_DIRECTORY`) - drop the #5689 separator patches (`isSamePath` comparisons, `isPathWithinRoot` folding, `toNativePath`) and the specs that encoded the mixed-separator state `platform-path-windows.spec.ts` drives the real stores with a win32 path module and asserts renderer-built paths are byte-identical to the watcher/main paths, so plain `===` matches — the gate for dropping the patches.
Jocs
force-pushed
the
poc/std-path-windows
branch
from
October 10, 2026 04:25
9c4377f to
e12100a
Compare
Jocs
changed the title
This was referenced Oct 10, 2026
Contributor
|
Build artifacts for PR #5691: Run: https://github.com/marktext/marktext/actions/runs/38024006665 |
3 of 6 tasks
Zane-0x5a
added a commit
to Zane-0x5a/marktext
that referenced
this pull request
Oct 10, 2026
Takes the open PR's head (e12100a) ahead of its upstream merge: the renderer and preload get a win32 path module on Windows, so the paths they build match the watcher's and plain comparisons work again. Its full unit suite and the desktop e2e suite pass on Windows 11, apart from the failures develop already has on this machine. Two fork specs still encoded the mixed-separator tree: the marktext#528 reveal spec matches folder rows by their native path, and the rename spec checks a Windows root with a Windows separator.
Tkaixiang
approved these changes
Oct 11, 2026
Tkaixiang
left a comment
Tkaixiang
left a comment
Member
There was a problem hiding this comment.
lgtm, tested reading, saving, tab switching + external file edits etc, all works fine
Zane-0x5a
added a commit
to Zane-0x5a/marktext
that referenced
this pull request
Oct 15, 2026
Upstream reworked the old print path (marktext#5722 makes MarkdownPrint wait for the notation fonts; marktext#5716 resolves URL-encoded Windows image paths). The fork replaced that path with the paged preview, so printService and its two specs keep the fork's side. Both fixes still reach the preview: the image fix lives in resolveLocalImageSrc, which exportHtml uses, and the notation fonts arrive through the muya export styles, the export themes and getCssForOptions. Main already waits for fonts.ready under print media before printToPDF. marktext#5746 drops the Info tab and explains that a table of contents needs a [TOC] line. The HTML export dialog takes both changes. The print preview panel shows the same explanation under its contents settings. The preload keeps the fork's visual zoom limit next to upstream's platform path import, and the editor store keeps isSameList. marktext#5691 was merged earlier from its PR head; upstream's squash differs from it only by commits that came in alongside it. Verified on Windows 11: typecheck, desktop and muya lint, and the desktop unit suite pass. In the muya unit suite, only the tableChessboard entrypoint import timed out, and only on cold runs (it passes alone). The print, export, sidebar, theme and title bar e2e specs plus upstream's new 3510, 5394, 5683 and 5710 specs pass in background windows, except 5394's exact `toBe(400)` width check. At this machine's 1.75 device pixel ratio the restored width reads 400.0000305, with or without the camera transform; with `toBeCloseTo(400, 3)` all three scenarios pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5669
Follow-up to #5689 (issue #5683, closed there).
Summary
#5689 made the sidebar/tab path lookups tolerant of mixed separators. This PR removes the mismatch at its source instead: the renderer and its preload now use a real Windows path module, so every path they build matches the native paths from the watcher, the main process and the OS — and plain
===matches. The #5689 patches (isSamePathcomparisons,isPathWithinRootfolding,toNativePath) are then redundant and are removed here.createPlatformPath(isWindows)(@jsr/std__pathwin32 on Windows,patheon POSIX) and use it for the preload'swindow.pathpathspecifier in the renderer build to the same platform path, so shared helpers likecommon/envPathsstop producing/paths on WindowsC:\…path was split on/so no directory tokens were produced and only the filename showed on Windows/filesystem join (CREATE_FILE_DIRECTORY)Type of change
Test plan
platform-path-windows.spec.tsdrives the real stores (treeCtrl, project, editor, trashKey) with a win32 path module and asserts renderer-built paths are byte-identical to the watcher/main paths — the gate for dropping the patches — pluscreatePlatformPathfor both branchestitle-bar-path-breadcrumb.spec.tscovers the [Bug] The title bar only displays the file name, but not the file path. #5669 breadcrumb split on\and/pnpm run typecheckandpnpm run lintcleanPOSIX verified locally (macOS) and by the e2e job. Not yet on Windows. CI's
windows-latestunit job stubswindow.pathand e2e only runs on Linux, so CI cannot validate the preload/alias change on a real Windows renderer. This needs a Windows e2e job or a manual Windows pass before merge.Notes for reviewers [optional]
@jsr/std__pathfrom JSR (@jsr:registry=https://npm.jsr.ioin.npmrc). If that is undesirable, the alternatives arepath-win32(npm, older) or vendoring the win32 subset.pathtosrc/renderer/src/node/platformPath.ts(waspathe), which is the module-level counterpart of the preload'swindow.path.By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.