Conversation
- Use case-insensitive claimed note path keys to avoid case-only filename collisions during import. - Refactor legacy list conversion so non-paragraph children stay properly nested under list items. - Count attachments only when files are actually downloaded (not cache-hit path reuse).
tgrosinger
left a comment
There was a problem hiding this comment.
Thank you for working on adding this new format! Looks like a great start so far.
I left a few comments in line. In addition, please consider padding human-readable error messages to reportFailed(). Passing error objects through directly may result in verbose and hard to understand errors, or [object Object] being logged. The full error can be logged to the console.
.gitignore
Outdated
| # Exclude files specific to personal development process | ||
| .hotreload | ||
| .devtarget | ||
|
|
There was a problem hiding this comment.
Please do not add these entries here.
There was a problem hiding this comment.
FYI if you need to exclude files locally without modifying .gitignore, you can modify your .git/info/exclude file.
src/main.ts
Outdated
|
|
||
| if (selectedId && importers.hasOwnProperty(selectedId)) { | ||
| let importer = this.importer = new selectedImporter.importer(this.app, this); | ||
| importer.init(); |
There was a problem hiding this comment.
Why is this change necessary? This should probably be reverted.
| titleFrontmatter: boolean; | ||
|
|
||
| init() { | ||
| // Initialize defaults in init() because FormatImporter calls init() from its constructor. |
There was a problem hiding this comment.
This contradicts another change in this PR.
There was a problem hiding this comment.
This should be consistent again now that the extra importer.init() call in main.ts is gone
src/formats/reflect-json.ts
Outdated
|
|
||
| const MAX_FILENAME_LENGTH = 200; | ||
|
|
||
| function truncateTitle(title: string): string { |
src/formats/reflect-json.ts
Outdated
|
|
||
| // Build frontmatter | ||
| let content = result.markdown; | ||
| const frontMatter: Record<string, any> = {}; |
There was a problem hiding this comment.
| const frontMatter: Record<string, any> = {}; | |
| const frontMatter: FrontmatterCache = {}; |
src/formats/reflect/convert.ts
Outdated
| idToSubject: Map<string, string>; | ||
| tags: Set<string>; | ||
| images: ImageInfo[]; | ||
| depth: number; |
There was a problem hiding this comment.
Removed the unused depth field from ConvertContext
src/formats/reflect-json.ts
Outdated
| if (note.daily_at) { | ||
| return moment(note.daily_at).format(userDNPFormat); | ||
| } | ||
| return truncateTitle(note.subject); |
There was a problem hiding this comment.
Provide a fallback title ("Untitled") if the returned value would be empty.
| return truncateTitle(note.subject); | |
| return truncateTitle(note.subject).trim() || 'Untitled'; |
There was a problem hiding this comment.
Added the Untitled fallback here
| }; | ||
| } | ||
|
|
||
| private async downloadImage( |
There was a problem hiding this comment.
Where are images generally downloaded from? Is it a centralized location which would require rate-limiting or backoffs?
There was a problem hiding this comment.
This is a good question. Reflect assets are stored with a link that looks something like this
https://reflect-assets.app/v1/users/tfgFpw.../999abc79-e45...?key=2227f9…
I doubt they have strict rate limits, but it's probably not a bad idea to put it in there as a safety measure.
There was a problem hiding this comment.
Yea, I actually didn't have enough images to even know if there were limits. My tests on my own export only had around 190 images and they all downloaded fine. I'll add some sort of rate limiting
There was a problem hiding this comment.
Added retries with backoff for attachment downloads and respect Retry-After when the server sends it. Downloads are still sequential.
| note.subject, | ||
| convertOptions, | ||
| ); | ||
| const outputPath = idToOutputPath.get(note.id); |
There was a problem hiding this comment.
Consider putting the note.id in the frontmatter like is done in other importer format. That would allow incremental imports in the future.
|
|
||
| async import(ctx: ImportContext) { | ||
| // Snapshot option values for this run so they can't drift mid-import. | ||
| const shouldDownloadAttachments = this.downloadAttachments === true; |
There was a problem hiding this comment.
The downloadAttachments toggle is snapshotted at import start (here) with a comment about preventing mid-import drift, but tagsFrontmatter, dateFrontmatter, and titleFrontmatter are read directly from this throughout the loop (lines 265, 282, 285, 289).
Either all four settings should be snapshotted for consistency, or none of them need to be — the import modal presumably isn't interactive while the import is running.
There was a problem hiding this comment.
Good catch, all four settings are snapshotted at import start now
|
@rossbrg Thanks for your work on this; I'm currently in the process of importing Reflect Notes into Obsidian (just using a temporary Python script for the time being). |
- Revert .gitignore to upstream state (remove personal dev entries) - Remove duplicate importer.init() call from main.ts - Move this.init() into FormatImporter base constructor - Use truncateText utility with 'Untitled' fallback for note titles - Wrap JSON parsing in try/catch with structural validation - Use FrontMatterCache type, include note.id unconditionally - Snapshot all four settings consistently at import start - Add retry/backoff for image downloads (max 4 attempts, exponential delay) - Pass human-readable error messages to reportFailed() - Remove unused depth field from ConvertContext - Fix ordered list numbering and nested list handling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract sanitizeFileName into standalone src/sanitize-file-name.ts to break the transitive obsidian dependency, enabling Node.js test imports. Wire up node:test + esbuild bundling with a smoke test for convertDocument.
Change hardBreak output from bare '\n' to '<br>\n' in both convertNode (block-level) and convertInline paths so forced line breaks render correctly in Obsidian/CommonMark.
Add text.trim() guards and skippedContent flag to all three list serializers so tag-only items produce no bare bullets. Archived annotations are preserved since the guard checks after appending the <!-- archived --> comment.
Add prefixFirstLine helper so headings as first children of bullet and legacy list items appear on the same line as the prefix (e.g. "- ### Heading") instead of on a separate indented line. Task list items are intentionally unchanged since heading semantics don't work with checkbox prefixes.
|
@tgrosinger Just pushed a follow-up pass for the review comments, appreciate the help. This removes the local I also changed the frontmatter object to While testing this against my own Reflect export, I ran into a few small converter issues and fixed them here too: hard breaks now render as |
|
@tgrosinger When this is merged, please merge obsidianmd/obsidian-help#1039 as well. |
Fixes #248.
This adds a native Reflect (
.json) importer so people can move from Reflect to Obsidian without relying on HTML export.The issue describes formatting loss when importing Reflect via HTML. This importer reads Reflect's JSON export directly and converts the ProseMirror document model to Markdown.
What is included
ReflectImporterwired into the importer list (Reflect (.json)).ctime/mtime).Hardening work from real export edge cases
],|, or\\fall back to Markdown link form when wiki-link syntax would be unsafe.reflect.academy/_next/image) are unwrapped to download the original asset URL.Files and fixtures
src/formats/reflect-json.tssrc/formats/reflect/convert.tssrc/formats/reflect/models.tssrc/main.tsmanifest.jsonREADME.mdtests/reflect/sample-reflect-export.jsontests/reflect/hardening-edge-cases.jsonGuideline alignment
FormatImporter+ registration inmain.ts).Testing
npm run buildRelated