fix: resolve attachment export inconsistencies (#108)#121
Conversation
- Unify filename encoding between tryCopyImage and tryCopyMarkdownByRead (both now use encodeURI(fileName) when fileNameEncode is off) - Add check to skip copying when source and target paths are identical (prevents file corruption for attachments already in output folder) Fixes issues where: - Some attachments had encoded names while others didn't - Attachment files were corrupted during export - Attachments were placed in different folders inconsistently 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughModified the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils.ts
🔇 Additional comments (1)
src/utils.ts (1)
358-358: LGTM! Encoding consistency achieved.This change correctly unifies the filename encoding behavior with
tryCopyMarkdownByRead(line 751), addressing the inconsistency mentioned in the PR objectives wheretryCopyImageused rawfileNamewhiletryCopyMarkdownByReadusedencodeURI(fileName).
| // Skip if source and target are the same (prevents file corruption) | ||
| // This can happen when an attachment is already in the output folder | ||
| if (filePath === targetPath) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Critical: Path separator inconsistency breaks self-copy prevention on Windows.
The string comparison filePath === targetPath will fail on Windows when ifile === null because:
filePathis constructed usingpath.join()(line 368), which produces backslashes on WindowstargetPathis normalized to forward slashes (line 395)
This defeats the purpose of the guard, potentially allowing file corruption from self-copy in the fallback case.
🔎 Proposed fix to normalize both paths before comparison
- // Skip if source and target are the same (prevents file corruption)
- // This can happen when an attachment is already in the output folder
- if (filePath === targetPath) {
- continue;
- }
+ // Skip if source and target are the same (prevents file corruption)
+ // This can happen when an attachment is already in the output folder
+ // Normalize both paths to forward slashes for consistent comparison across platforms
+ if (filePath.replace(/\\/g, "/") === targetPath) {
+ continue;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Skip if source and target are the same (prevents file corruption) | |
| // This can happen when an attachment is already in the output folder | |
| if (filePath === targetPath) { | |
| continue; | |
| } | |
| // Skip if source and target are the same (prevents file corruption) | |
| // This can happen when an attachment is already in the output folder | |
| // Normalize both paths to forward slashes for consistent comparison across platforms | |
| if (filePath.replace(/\\/g, "/") === targetPath) { | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In src/utils.ts around lines 397-401 (and note filePath built at ~line 368 and
targetPath normalized at ~line 395), the direct string comparison filePath ===
targetPath fails on Windows due to inconsistent path separators; normalize both
paths before comparing by resolving and normalizing each path (e.g. use
path.resolve(path.normalize(...))) and, on Windows (process.platform ===
'win32'), compare their lowercase forms to handle case-insensitivity; replace
the current equality check with the normalized/resolved (and lowercased on
Windows) comparison so the self-copy guard reliably detects identical paths
across platforms.
1 participant
Summary
tryCopyImageandtryCopyMarkdownByReadfunctionsDetails
Problem (Issue #108)
When exporting a folder with attachments:
02fb6aa80be454d22bffb5ca5d55727e.jpg) while others kept original namesattachmentvsattachments) inconsistentlyRoot Cause
tryCopyImageusedfileNamedirectly for encodingtryCopyMarkdownByReadusedencodeURI(fileName)for encodingFix
encodeURI(fileName)whenfileNameEncodeis offfilePath === targetPathTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.