Repository navigation
fix: reuse image on case-insensitive path match (#3119) - #5326
Conversation
…arktext#3119) When the dropped image matches outputDir only ignoring case, return the outputDir-based path instead of the image's own casing. pathe.relative compares segments case-sensitively, and the document path keeps whatever casing it was opened with, so relativizing the image's casing produced links like `../../../Users/me/proj/assets/x.png` that break on case-sensitive systems. The outputDir-based path yields `assets/x.png`, matching the links written for hashed copies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TMdsfzfm4LYG7MkBckAqQ
…mments (marktext#3119) Drive letters can't differ in case here (pathe upper-cases them on both sides), so explain the real source of the mismatch: the dropped file's on-disk casing versus the preference and document path casing. Note that the isSamePathSync stub models a case-insensitive filesystem, drop its redundant strict-equality branch, and stop referencing the removed `noHashPath === imagePath` check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011TMdsfzfm4LYG7MkBckAqQ
Jocs
left a comment
Jocs
left a comment
There was a problem hiding this comment.
Thanks for the fix, @dzid26! The approach is right. I checked the real common/filesystem/paths.isSamePathSync on a case-insensitive APFS folder: Images/foo.png vs images/foo.png gives === → false (the old code copied a hashed duplicate) and isSamePathSync → true. A missing file still returns false. Drag-and-drop reaches this branch through muya's getPathForFile, so #3119 benefits on macOS as well as Windows.
One regression, fixed in 07b5fea
The match returned toResult(imagePath), which keeps the dropped file's on-disk casing. toResult relativizes with pathe.relative, which compares segments case-sensitively. The main process only path.resolves opened files and never canonicalizes their casing. So when the document path's ancestor casing differs from the disk (e.g. marktext c:\users\me\proj\a.md typed in a terminal), the inserted link climbs out of the document folder:
Those links still open on Windows but break on GitHub, Linux, or a clone elsewhere. I pushed a commit that returns toResult(noHashPath). The link now always goes through outputDir, spelled like the links written for hashed copies. It also adds an isRelative: true case with mismatched ancestor casing. Without the fix that case yields ../../../../../../VAR/FOLDERS/…/ASSETS/Already.PNG; with it, assets/Already.PNG.
Spec tidy-up, in 273cb6d
- The "drive-letter case" comment can't happen in production:
window.pathispathe, which upper-cases drive letters on both sides. The comment now names the real source, the dropped file's on-disk casing versus the preference and document-path casing. - The
isSamePathSyncstub now says it models a case-insensitive filesystem (it never stats), and its redundantna === nbbranch is gone. - An older comment still referenced the removed
noHashPath === imagePathcheck.
Lint (0 errors), typecheck, and the spec (9/9) pass locally.
Out of scope, for a follow-up
getContentHash(imagePath) hashes the path string, not the image bytes. Re-saving a screenshot at the same source path and dropping it again reuses the hashed name, so copy overwrites the earlier asset that other documents may still reference. That predates this PR, so I left it alone.
Closes #3119
Summary
The #3119 still was the issue (hashed file being copied) if for example the Relative folder was named
images, but actual folder on the disk wasImages. On Windows it is expected to reuse the folder regardless of the letter casing!Use OS aware
isSamePathSyncfunction instead of string comparison.Type of change
Test plan
Notes for reviewers [optional]
Worth first merging #5323 which will trigger CI on Windows target.
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.