Google Keep: Add created/modified timestamps & handle duplicates#472
Google Keep: Add created/modified timestamps & handle duplicates#472keanucz wants to merge 1 commit intoobsidianmd:masterfrom
Conversation
keanucz
changed the title
tgrosinger
left a comment
tgrosinger
left a comment
There was a problem hiding this comment.
Thank you @keanucz for helping to improve the Keep importer! I've left some comments and questions. Let me know what you think.
| // may not find an existing folder if the casing doesn't match, and then | ||
| // `createFolder` throws because it already exists. Try again with an | ||
| // insensitive lookup before failing. | ||
| folder = vault.getAbstractFileByPathInsensitive(normalizedFolderPath); |
There was a problem hiding this comment.
The change made above on 155 seems like it should cover this case and remove the need to repeat the lookup here.
The only case I can think of where this would have an effect is if you are on a case-sensitive OS and the provided path is a file, but an insensitive match finds a folder. I might argue in that case we should just be throwing an error, not finding the insensitive-matching folder on a case-sensitive OS.
| await importer.import(ctx); | ||
| } | ||
| catch (e) { | ||
| ctx.reportFailed('Import', e); |
There was a problem hiding this comment.
Were you running into an importer that was throwing errors? Maybe we should fix that more directly?
| existingFileBehavior: ExistingFileBehavior = 'skip'; | ||
|
|
||
| init() { | ||
| this.importArchived = this.importArchived ?? false; |
There was a problem hiding this comment.
What is the purpose of these lines?
|
|
||
| this.existingFileSetting = new Setting(this.modal.contentEl) | ||
| .setName('If note or attachment already exists') | ||
| .setDesc('When importing into the same folder again, choose to skip, overwrite, or create a duplicate.') |
There was a problem hiding this comment.
| .setDesc('When importing into the same folder again, choose to skip, overwrite, or create a duplicate.') | |
| .setDesc('When importing into the same folder again, choose how to handle content that was previously imported.') |
|
|
||
| const createdMs = normalizeEpochMs(keepJson.createdTimestampUsec); | ||
| const editedMs = normalizeEpochMs(keepJson.userEditedTimestampUsec); | ||
| if (!createdMs && !editedMs) { |
| ctx.reportAttachmentSuccess(fullpath); | ||
| const imported = await this.copyFile(file, assetFolderPath, ctx); | ||
| if (imported) { | ||
| ctx.reportAttachmentSuccess(fullpath); |
There was a problem hiding this comment.
Maybe this should be moved into copyFile instead? Then it would be next to the other reporting calls in that function.
| ctx.reportNoteSuccess(fullpath); | ||
| const imported = await this.convertKeepJson(keepJson, folder, basename, fullpath, ctx); | ||
| if (imported) { | ||
| ctx.reportNoteSuccess(fullpath); |
There was a problem hiding this comment.
Same comment as above. Move this into convertKeepJson?
| const createdMs = normalizeEpochMs(keepJson.createdTimestampUsec); | ||
| const editedMs = normalizeEpochMs(keepJson.userEditedTimestampUsec) ?? createdMs; | ||
|
|
||
| if (createdMs !== undefined) frontMatter['keepCreatedAt'] = new Date(createdMs).toISOString(); |
There was a problem hiding this comment.
I've actually been having a very similar conversation about this in !467.
I don't think we need to store these times in the frontmatter. They should be set on the file correctly when creating the file. Setting it on the file means that in a Base you can view the file.mtime and file.ctime, filter using them, or even transform it into another format.
2 participants
This fixes issues #377 and #385. I've tested it against my local vault (by importing my own Keep takeout with a dozen or so attachments and 675 notes) and this PR adds the following functionality:
keepCreatedAtandkeepUpdatedAtto the frontmatter of all imported notes, output using ISO8601.I've never contributed to an Obsidian plugin before so I'm happy to take any pointers or suggestions for how my implementation could be refined!