Skip to content

[IMP] Update existing notes but don't create new note with zero highlights #71

Description

@johannrichard

Currently, there is a limitation related to Reader items and deletions due to how Reader and Readwise are linked. If you delete a document in Reader, it is not completely removed from Readwise, even though it will not be visible anymore.

This leads to a regression where "deleted" Reader documents with be kept as Readwise notes with zero highlights, and where a new note without any highlights is created in return, unless we don't update existing notes if highlights are (completely) missing. However, users expect that deleted highlights are reliably removed on sync.

Furthermore, if all notes of a Readwise item are discarded, the same might happen.

In other words: we need to improve the way notes without highlights are treated:

  • ✅ Update existing ones
  • ❌ Don't create new note when no highlights are present

The current code does not support this easily and needs updates in several places, including the following ones:

private async updateExistingFile(file: TFile, readwiseFile: ReadwiseFile): Promise<void> {
this.notifyFileCount();
try {
// Only update frontmatter if frontmatter is enabled
if (this.settings.frontMatter && this.settings.updateFrontmatter) {
const updatedFrontmatter = this.frontmatterManager.getFrontmatter(
readwiseFile.doc,
this.app.metadataCache.getFileCache(file)?.frontmatter
);
this.logger.debug(`Updating file ${file.path} with new frontmatter`, updatedFrontmatter);
await this.vault.process(file, () => `${updatedFrontmatter.toString()}\n${readwiseFile.contents}`);
} else {
const frontmatter = this.frontmatterManager.getFrontmatter(readwiseFile.doc);
this.logger.debug(`Not updating frontmatter for file ${file.path}`, frontmatter);
await this.vault.process(file, () => `${frontmatter.toString()}\n${readwiseFile.contents}`);
}
// We only rename files if the respective settings are enabled and the filenames differ
if (this.settings.trackFiles && this.settings.enableFileNameUpdates && readwiseFile.basename !== file.basename) {
let newPath = this.getNormalizedPath(file.parent.path, `${readwiseFile.basename}.md`);
const newFileExists = await this.app.vault.adapter.exists(newPath, false);
if (newFileExists) {
// Add hash to filename if there's a collision
const hash = this.generateShortHash(readwiseFile.doc);
newPath = this.getNormalizedPath(file.parent.path, `${readwiseFile.basename} ${hash}.md`);
}
if (newPath !== file.path) {
this.logger.debug(`Renamed file from ${file.path} to ${newPath}`);
await this.app.fileManager.renameFile(file, newPath);
}
}
} catch (err) {
this.logger.error(`Readwise: Attempt to update file ${file.path} failed`, err);
throw err;
}
}

private async processTrackedFile(trackedPrimary: ReadwiseFile) {
const existingFiles = await this.findExistingByHighlightsUrl(trackedPrimary.doc);
if (existingFiles.length > 0) {
const [primary, ...duplicates] = existingFiles;
// TODO: Add an option to the plugin to link remote duplicates to the primary file
await this.updateExistingFile(primary, trackedPrimary);
for (const duplicate of duplicates) {
this.logger.warn('Existing duplicate file found', { duplicate });
await this.handleDuplicate(duplicate, trackedPrimary);
}
} else {
// If the file already exists, create a new file with a hash
if (await this.app.vault.adapter.exists(trackedPrimary.path, false)) {
await this.writeFile(trackedPrimary);
} else {
await this.writeFile(trackedPrimary, true);
}
}
}

const fileExists = await this.app.vault.adapter.exists(path, false);
if (fileExists) {
if (overwrite) {
const existingFile = await this.vault.getFileByPath(path);
this.logger.debug('Overwriting existing file', { doc: readwiseFile.doc, ...fileOptions });
await this.vault.process(existingFile, () => fileContents, fileOptions);
return existingFile;
}
// Create new path with hash
const hash = this.generateShortHash(readwiseFile.doc);
const newPath = this.getNormalizedPath(
this.getCategoryPath(readwiseFile.doc.category),
`${readwiseFile.basename} ${hash}.md`
);
const newFileExists = await this.app.vault.adapter.exists(newPath, false);
if (newFileExists) {
const existingNewFile = await this.vault.getFileByPath(newPath);
this.logger.debug('Overwriting existing file (with hash)', { doc: readwiseFile.doc, ...fileOptions });
await this.vault.process(existingNewFile, () => fileContents, fileOptions);
return existingNewFile;
}
this.logger.debug('Creating new file (with hash)', { doc: readwiseFile.doc, ...fileOptions });
return await this.vault.create(newPath, fileContents, fileOptions);
}

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions