feat!: 🏗️ major plugin overhaul with many new features #50
Conversation
while the sanitized title is necessary for file names, one migth actually want to use the *real* title in the contents. This change implements this. The old default header template uses `title` which will create invalid links for titles with special characters. Users are advised to update their templates before running a sync.
This change moves the API from the `books` and `highlights` endpoints to the newish `export` endpoint which is made for that kind of solution. It also implements the new `document_note` and `summary` fields introduced with Reader.
readwise-mirror is currently unaware if an form of sync is implemented. This proposal implements "onExternalSettingsChange" to reload the settings and update the Status according to the last successful external sync.
Use the [`Vault.process()`](https://docs.obsidian.md/Reference/TypeScript+API/Vault/process) to atomically read, modify, and save the contents of existing notes when updating from Readwise, instead of completely deleting and recreating the files. Should improve overall compatibility with other plugins. The switch from `getAbstractFileByPath()` to `getFileByPath()` should simplify things without compromising functionalty.
…here Articles highlighted with Readwise Reader will have a `unique_url` pointing to Reader. Otherwise, this URL will point to the same location as `source_url`. Implements #29 for the whole article.
with this change, and using the `export` API, the plugin will keep a local cache of the full library and will integrate updated highlights into the cache. This also opens the door to 3-way merge operations as we will have the original, the loacl, and the remote Readwise changes available which should allow us to work towards more robust forms of merging. major arcitectural change with respect to keeping a local cache.
make downloading the library more robust by using "fetch-retry"
Use `getAbstractFilebyPath()` instead of `getFileByPath()` to increase compatibility with older Obsidian versions. Bump ` obsidian` package version to `1.1.1`.
…oks` and `highlughts` API on updates. First, fetch updated books, and then fetch all highlights just for the changed books.
feat: ✨ hot-reload settings after sync
feat: ✨ differentiate title from sanitized title
feat: ⚡ implement "fetch-retry"
refactor: ♻️ use process() function to write new content
upgrade from master
implement github workflow to build new versions automatically
|
🎉 This PR is included in version 1.5.0-beta.16 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
3-4: Improve Warning Block WordingThe warning block is clear overall; however, consider adding the preposition “on” to improve clarity. For example, change “due to changes how filenames are generated and validated” to “due to changes in how filenames are generated and validated.” Also, “a step-by-step guide how you can prepare…” might be clearer as “a step-by-step guide on how you can prepare….”
🧰 Tools
🪛 LanguageTool
[uncategorized] ~4-~4: Possible missing preposition found.
Context: ...which might break things due to changes how filenames are generated and validated. ...(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~4-~4: Possible missing preposition found.
Context: ...mentation contains a step-by-step guide how you can prepare an existing Readwise li...(AI_EN_LECTOR_MISSING_PREPOSITION)
42-43: Fix Typographical Error in the Command DescriptionThere is a minor typo in the “Adjust Filenames to current settings” command description—“CLean” should be “Clean.”
🧰 Tools
🪛 LanguageTool
[style] ~42-~42: This phrasing can be wordy. For improved clarity, try opting for something more concise.
Context: ...gs (whitespace removal and slugify only for the time being)Settings
General
...
(FOR_THE_TIME_BEING)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
README.md(7 hunks)manifest.json(1 hunks)package.json(1 hunks)src/main.ts(1 hunks)src/ui/settings-tab.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- manifest.json
🧰 Additional context used
🧬 Code Definitions (1)
src/main.ts (4)
src/types.d.ts (7)
PluginSettings(104-137)Tag(34-37)Highlight(19-32)Export(1-17)Library(43-47)ReadwiseFile(57-62)ReadwiseDocument(69-91)src/ui/notify.ts (1)
Notify(3-23)src/services/logger.ts (1)
error(32-34)src/constants/index.ts (1)
DEFAULT_SETTINGS(4-92)
🪛 LanguageTool
README.md
[uncategorized] ~4-~4: Possible missing preposition found.
Context: ...which might break things due to changes how filenames are generated and validated. ...
(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~4-~4: Possible missing preposition found.
Context: ...mentation contains a step-by-step guide how you can prepare an existing Readwise li...
(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~38-~38: Loose punctuation mark.
Context: ...
Commands
Sync new highlights: Download all new highlights since previ...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~42-~42: This phrasing can be wordy. For improved clarity, try opting for something more concise.
Context: ...gs (whitespace removal and slugify only for the time being)
Settings
General
...
(FOR_THE_TIME_BEING)
[uncategorized] ~110-~110: Possible missing comma found.
Context: ...sting highlights in Readwise are always reflected rather than getting out of sync. The no...
(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~141-~141: Possible typo: you repeated a word.
Context: ... Beta Plugin with frozen version"
- Enter jsonMartin/readwise-mirror
- Enter the version number you noted from relea...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~231-~231: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...he file
4. Fields listed for protection but not present in the file will be:
- ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~311-~311: Loose punctuation mark.
Context: ...date |
Template Filters
bq: Add blockquote markers.is_qa: Che...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~312-~312: Loose punctuation mark.
Context: ...bq: Add blockquote markers.
is_qa: Check for Q&A format.qa: Convert ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~313-~313: Loose punctuation mark.
Context: ...- is_qa: Check for Q&A format.
qa: Convert to Q&A format.
Default ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~416-~416: A punctuation mark might be missing here.
Context: ...le |
|--------|-------------|---------|
| bq | Add blockquote markers | `{{ te...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~455-~455: Loose punctuation mark.
Context: ...lters available for that:
is_qa: this filter returnstrueif the strin...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~478-~478: Possible missing comma found.
Context: ...put
Input note with .qa tag:
.qa What is the capital of France? Paris is...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~558-~558: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...that Markdown / Wikilinks in your notes can not be preserved, as the plugin can not syn...
(CAN_NOT_PREMIUM)
[style] ~558-~558: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...tes can not be preserved, as the plugin can not sync changes to the original Readwise c...
(CAN_NOT_PREMIUM)
🔇 Additional comments (25)
README.md (14)
16-18: "Flexible Organization" Feature Block Looks GoodThe bullet points under “Flexible Organization” clearly describe the feature. No issues found.
20-22: "Seamless Syncing" Section is Well DefinedThe description for seamless syncing is concise and adequately explains the one-way sync process.
24-28: "Enhanced Obsidian Integration" Is Clearly DocumentedThe bullet points detailing Obsidian integration, including automatic link creation and block references, are clear and informative.
30-34: "Smart File Management" Details the New CapabilitiesThis new section effectively outlines how the plugin tracks files using unique URLs, updates filenames, and preserves custom frontmatter.
133-146: "Using BRAT for Beta Testing" Section is ComprehensiveThis new section provides a clear step-by-step guide for beta testing via BRAT. The instructions are easy to follow. Verify that the repeated entries (if any) and phrasing are consistent with your style guide.
🧰 Tools
🪛 LanguageTool
[duplication] ~141-~141: Possible typo: you repeated a word.
Context: ... Beta Plugin with frozen version"
- EnterjsonMartin/readwise-mirror
- Enter the version number you noted from relea...(ENGLISH_WORD_REPEAT_RULE)
162-177: Renaming "Templating" to "Author Name Settings"The renaming reflects the updated functionality for processing author names. The examples provided are clear.
177-186: "Slugify Filenames" Block Clarifies the New Filename OptionThe explanation regarding slugifying filenames is detailed and clear. The warning about potential duplicate files is important; ensure that users understand why a full re-sync may be required.
188-198: "Templating" Section Update is ClearThe updated section outlining the three template types (Header, Highlight, and Frontmatter) using Nunjucks is straightforward and informative.
200-205: "Frontmatter Validation" Details Are Well PresentedListing the validation rules (valid YAML, field escaping, correct variables, sample preview) improves user confidence.
452-455: Q&A Filter Introduction is InformativeThe introduction to the Q&A filters (“is_qa” and “qa”) explains their purpose well. The message is clear for users looking to format Q&A notes differently.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~455-~455: Loose punctuation mark.
Context: ...lters available for that:
is_qa: this filter returnstrueif the strin...(UNLIKELY_OPENING_PUNCTUATION)
460-472: Example Template for Q&A Filtering is Well StructuredThe provided Nunjucks example demonstrates proper usage of the Q&A filters. The branching between Q&A formatted notes and regular notes is easy to follow.
475-482: "Example Q&A Output" Section Accurately Demonstrates Filter UsageThe example output clearly correlates with the input note, making it easy for users to understand the rendering result.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~478-~478: Possible missing comma found.
Context: ...putInput note with
.qatag:.qa What is the capital of France? Paris is...
(AI_HYDRA_LEO_MISSING_COMMA)
490-583: Deduplication Section is Detailed and InformativeThe comprehensive explanation of file grouping, tracking methods, and duplicate management provides clear insight into the deduplication strategy. Consider verifying that all technical terms (e.g., “normalized path”) are defined elsewhere or linked to documentation for clarity.
🧰 Tools
🪛 LanguageTool
[style] ~558-~558: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...that Markdown / Wikilinks in your notes can not be preserved, as the plugin can not syn...(CAN_NOT_PREMIUM)
[style] ~558-~558: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...tes can not be preserved, as the plugin can not sync changes to the original Readwise c...(CAN_NOT_PREMIUM)
584-604: "Upgrading from 1.x.x to 2.x.x" Section Provides Essential Upgrade InstructionsThe upgrade guide is thorough, outlining necessary backup steps and configuration changes (such as adding the
uritracking property). This clear step-by-step instruction will help users transition smoothly.package.json (4)
2-5: Versioning and Basic FieldsThe update to the package metadata reflects the new release. However, note that the PR objectives mention breaking changes and a major overhaul; if these changes are incompatible with previous versions, consider bumping the major version (e.g., to “2.0.0”) to adhere to semantic versioning guidelines.
6-12: Updated Scripts Support the New Build and Deployment WorkflowThe modifications in the scripts—such as switching to the
rollup.config.mjsconfiguration and adding new deploy/release scripts—are well implemented. Ensure that the documentation is updated accordingly to reflect these workflow changes.
16-34: DevDependencies Section Reflects New Workflow RequirementsThe added dependencies (e.g.,
@semantic-release/git,semantic-release-obsidian-plugin, and others) properly support the new semantic release workflow and module expectations. The version constraints appear solid.
35-42: Dependencies Updated for Improved Filename HandlingReplacing
fetch-retrywith@sindresorhus/slugifyandfilenamifyaligns with the new filename management strategy. The other dependencies remain consistent.src/main.ts (4)
22-46: Overall Implementation Looks SolidThe plugin class definition and associated getters/setters are concise and follow a clean design.
197-197: Useawaitwhen creating a new file to ensure proper completionThis mirrors a past comment, and still applies:
If the subsequent logic relies on the file to exist, omittingawaitmay lead to race conditions.
436-438: Await asynchronous writes to avoid partial or incomplete synchronizationAs previously noted in old review comments for similar lines, calling:
this.writeLibraryToMarkdown(library); this.writeLogToMarkdown(library);without
awaitcan cause potential race conditions if future steps rely on those operations finishing.
1-676: No Additional Issues DetectedBeyond the noted concurrency concerns, the rest of the code is well-structured, with clear boundary checks and error handling.
src/ui/settings-tab.ts (3)
1-188: General Structure Looks GreatThe tab-based settings interface is neatly organized. The split into multiple render methods for different sections (authentication, sync, highlights, etc.) is consistent with good code organization and maintainability.
189-198: Enhance client ID generation securityThis comment matches a previous suggestion:
UsingMath.random()is not cryptographically secure. Consider leveragingwindow.crypto.getRandomValuesfor better randomness in the generated client ID.
199-1271: All Other Sections Appear Well-StructuredThe new tab-based system for settings, template validation, and user-facing controls demonstrates clear design, logical grouping, and appropriate user guidance. No further issues to report.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
README.md (8)
3-5: Clarify Warning Message
The warning clearly flags that Readwise Mirror 2.x is a major rewrite; however, the wording can be improved for grammatical clarity. For instance, consider revising “due to changes how filenames are generated and validated” to “due to changes in how filenames are generated and validated.”🧰 Tools
🪛 LanguageTool
[uncategorized] ~4-~4: Possible missing preposition found.
Context: ...which might break things due to changes how filenames are generated and validated. ...(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~4-~4: Possible missing preposition found.
Context: ...mentation contains a step-by-step guide how you can prepare an existing Readwise li...(AI_EN_LECTOR_MISSING_PREPOSITION)
42-43: Command Description: Adjust Filenames
The description explains that the command cleans up filenames by removing extra whitespace and applying slugification. Consider rephrasing “for the time being” to a more formal or definitive note if the functionality is intended to be permanent until a future update.🧰 Tools
🪛 LanguageTool
[style] ~42-~42: This phrasing can be wordy. For improved clarity, try opting for something more concise.
Context: ...gs (whitespace removal and slugify only for the time being)Settings
General
...
(FOR_THE_TIME_BEING)
133-145: Section: Using BRAT for Beta Testing
This section provides clear, step-by-step instructions for beta testing via BRAT. It would be worthwhile to review the quoted button labels (e.g., “Add Beta Plugin with frozen version”) for any potential repeated or ambiguous phrasing.🧰 Tools
🪛 LanguageTool
[duplication] ~141-~141: Possible typo: you repeated a word.
Context: ... Beta Plugin with frozen version"
- EnterjsonMartin/readwise-mirror
- Enter the version number you noted from relea...(ENGLISH_WORD_REPEAT_RULE)
178-186: Section: Slugify Filenames
The explanation of slugifying filenames is thorough and informative. For improved clarity, consider specifying that “safe characters” means URL‑safe characters and emphasize the warning that enabling this option may create duplicate files unless a full library sync is performed.
217-236: Section: Frontmatter Protection
These detailed instructions help ensure that specific frontmatter fields remain unaltered during sync operations. Consider adding a comma before “but” in the clause (“Fields listed for protection, but not present in the file, will be:”) to enhance readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~231-~231: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...he file
4. Fields listed for protection but not present in the file will be:
- ...(COMMA_COMPOUND_SENTENCE_2)
326-342: Complex Frontmatter Template Example
This example is comprehensive and shows how to roll up document and highlight tags. Adding inline comments within the template might help clarify the logic for users unfamiliar with advanced Nunjucks templating techniques.
422-433: Default Highlight Template
This template effectively uses inline conditional logic to display optional details (such as location, color, notes, and tags). Consider introducing additional line breaks or indentation to improve readability and maintainability of the template code.
460-472: Q&A Template Example
The provided code example demonstrates the use of Q&A filters in a practical manner. Adding brief inline comments within the code block could further aid users in understanding the conditional logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(7 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~4-~4: Possible missing preposition found.
Context: ...which might break things due to changes how filenames are generated and validated. ...
(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~4-~4: Possible missing preposition found.
Context: ...mentation contains a step-by-step guide how you can prepare an existing Readwise li...
(AI_EN_LECTOR_MISSING_PREPOSITION)
[uncategorized] ~38-~38: Loose punctuation mark.
Context: ...
Commands
Sync new highlights: Download all new highlights since previ...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~42-~42: This phrasing can be wordy. For improved clarity, try opting for something more concise.
Context: ...gs (whitespace removal and slugify only for the time being)
Settings
General
...
(FOR_THE_TIME_BEING)
[uncategorized] ~110-~110: Possible missing comma found.
Context: ...sting highlights in Readwise are always reflected rather than getting out of sync. The no...
(AI_HYDRA_LEO_MISSING_COMMA)
[duplication] ~141-~141: Possible typo: you repeated a word.
Context: ... Beta Plugin with frozen version"
- Enter jsonMartin/readwise-mirror
- Enter the version number you noted from relea...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~231-~231: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...he file
4. Fields listed for protection but not present in the file will be:
- ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~311-~311: Loose punctuation mark.
Context: ...date |
Template Filters
bq: Add blockquote markers.is_qa: Che...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~312-~312: Loose punctuation mark.
Context: ...bq: Add blockquote markers.
is_qa: Check for Q&A format.qa: Convert ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~313-~313: Loose punctuation mark.
Context: ...- is_qa: Check for Q&A format.
qa: Convert to Q&A format.
Default ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~416-~416: A punctuation mark might be missing here.
Context: ...le |
|--------|-------------|---------|
| bq | Add blockquote markers | `{{ te...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~455-~455: Loose punctuation mark.
Context: ...lters available for that:
is_qa: this filter returnstrueif the strin...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~558-~558: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...that Markdown / Wikilinks in your notes can not be preserved, as the plugin can not syn...
(CAN_NOT_PREMIUM)
[style] ~558-~558: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...tes can not be preserved, as the plugin can not sync changes to the original Readwise c...
(CAN_NOT_PREMIUM)
[uncategorized] ~603-~603: Possible missing comma found.
Context: ...ties like author are omitted from the template as these have a tendency to break front...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (16)
README.md (16)
16-18: Feature Update: Flexible Organization
The bullet points succinctly describe the flexible organization feature. The descriptions are clear and informative.
20-22: Feature Update: Seamless Syncing
This segment correctly details the one-way synchronization capabilities; the language is concise and clear.
24-28: Feature Update: Enhanced Obsidian Integration
The integration details—including automatic link creation, block references, and tag support—are well presented.
30-34: Feature Update: Smart File Management
The explanation of using unique URLs to track changes, updating filenames, protecting frontmatter fields, and slugification is clear and adequately detailed.
190-196: Section: Template Types
The description of the three template types (Header, Highlight, and Frontmatter) using Nunjucks is concise and clear.
198-205: Section: Frontmatter Validation
This checklist is very helpful. It clearly lists the validations (YAML syntax, field escaping, template variables, and preview support) that users can expect.
207-215: Section: Updating Frontmatter
The instructions for managing frontmatter updates—both when enabled and disabled—are well articulated and user-friendly.
237-248: Section: Document Metadata
The variable table is well formatted and clearly presents information about each metadata field.
250-260: Section: Header / Frontmatter Content
This markdown table is clear and well organized. Ensure that its formatting is consistent with the previous metadata table for a uniform documentation style.
317-324: Default Frontmatter Template
The default frontmatter template is straightforward, making it easy for users to understand and use as a base for customization.
344-363: Default Header Template
The header template is well-structured and conveys all necessary metadata. The conditional rendering (e.g., for tags and source URLs) is implemented cleanly; just ensure that consistent formatting is maintained throughout the conditional blocks.
435-443: Blockquote Filter Explanation
The section explaining the blockquote filter is clear and instructive. The documentation shows how to apply the filter effectively without breaking Markdown formatting.🧰 Tools
🪛 LanguageTool
[grammar] ~437-~437: Probable usage error. Use “and” after ‘both’.
Context: ...s useful for both multi-line highlights as well as multi-line notes.With this filter, ...
(BOTH_AS_WELL_AS)
451-457: Q&A Filter Section
This part clearly explains how the Q&A filters work (i.e. detecting and extracting question–answer pairs). It might be beneficial to mention any dependencies on the current version of the Readwise API if applicable.🧰 Tools
🪛 LanguageTool
[uncategorized] ~455-~455: Loose punctuation mark.
Context: ...lters available for that:
is_qa: this filter returnstrueif the strin...(UNLIKELY_OPENING_PUNCTUATION)
474-481: Example Q&A Output
The before-and-after demonstration effectively shows how a note with a.qatag is transformed into a clear Q&A format.
568-574: Deduplication Limitations
This section succinctly outlines the key limitations of the deduplication strategy. The points provided give users a realistic understanding of the inherent trade-offs.
588-603: Upgrading Guide
The step-by-step upgrade instructions for moving from 1.x.x to 2.x.x are thorough and clear. The inclusion of code snippets and backup advice makes this guide very user friendly.🧰 Tools
🪛 LanguageTool
[uncategorized] ~603-~603: Possible missing comma found.
Context: ...ties likeauthorare omitted from the template as these have a tendency to break front...(AI_HYDRA_LEO_MISSING_COMMA)
fix a bug intreoduce during refactoring of the frontmatter classes, ensure YAML is compatible with Obsidian Linter (platers/obsidian-linter#1227).
….json` to 1.5.0-beta.17 [skip ci] ## [1.5.0-beta.17](1.5.0-beta.16...1.5.0-beta.17) (2025-03-23) ### Bug Fixes * 🚑️ fix YAML stringify options ([b1bcda4](b1bcda4)), closes [platers/obsidian-linter#1227](platers/obsidian-linter#1227)
|
🎉 This PR is included in version 1.5.0-beta.17 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
package.json (1)
35-42: Dependencies Enhancement
The dependencies section now includes packages like@sindresorhus/slugifyandfilenamifyto support robust filename deduplication. It's worth verifying that these libraries correctly handle edge cases such as duplicate items or special characters in filenames across different operating systems.src/services/frontmatter.ts (6)
20-22: Consider specialized handling forDatetypes.
WhileDateis listed underPrimitiveValue, YAML parse/stringify might produce strings instead ofDateobjects. To preserve date types, consider using a custom YAML schema or implementing a re-hydration step.
43-56: Enhance error context when validation fails.
Currently, the code throws a generic'Invalid frontmatter data'message. Providing more detailed guidance about which part of the data is invalid or including YAML parsing error details can help users debug more quickly.
79-84: Consider atomic set operations.
ValidatingnewDatabefore updatingthis.datais good. For more complex use cases, you could employ an atomic approach that reverts changes if validation fails (though this code’s current design consistently throws on failure, preventing partial updates).
139-151: Review frontmatter delimiter placement.
The current arrangement returns an empty string if no data exists, or encloses YAML with two triple-dash lines. You might want to ensure the final document structure includes or omits trailing newlines as expected by downstream tools.
166-191: Parsing only the first frontmatter block may be limiting.
Your regex captures a single frontmatter block. If you ever plan to support multiple frontmatter segments or advanced YAML documents, consider an optional expansion to parse or merge additional blocks.Would you like me to propose a change that handles multiple frontmatter blocks?
196-203: Optimize repeated parse calls inisValidfor large files.
isValidcallsFrontmatter.fromStringunder the hood, which can be expensive for large files if called repeatedly. Caching or a more direct parse-check might enhance performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
manifest.json(1 hunks)package.json(1 hunks)src/services/frontmatter.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- manifest.json
🔇 Additional comments (5)
package.json (3)
2-5: Semantic Versioning & Breaking Changes
The version has been updated to"1.5.0-beta.17". Given that this release introduces breaking changes (for example, modifications in filename generation and tracking), please verify if a major version bump (e.g., to"2.0.0") might be more appropriate to clearly communicate these breaking changes per semantic versioning guidelines.
6-12: New Deployment and Release Scripts
The addition of new scripts (deploy:local,release:local, andrelease:dry-run) enhances the deployment and release workflow. Please ensure that:
- The Rollup configuration has been updated accordingly from
rollup.config.jstorollup.config.mjs.- Environment variables such as
$npm_package_nameare set correctly across different environments.
16-33: DevDependencies Update Review
The updates to thedevDependencies(e.g., addition of@biomejs/biome,semantic-release-obsidian-plugin, etc.) align with the new GitHub Actions workflow and deployment processes introduced in this overhaul. Confirm that these packages are required and compatible with the rest of the project’s ecosystem.src/services/frontmatter.ts (2)
1-2: Verify thatYAML_TOSTRING_OPTIONSis defined properly.
Ensure that theconstants/indexmodule accurately exportsYAML_TOSTRING_OPTIONS. A missing or undefined constant could lead to runtime errors.Would you like to run a quick check to confirm this constant is properly declared and imported?
7-16: Use of domain-specific error is commendable.
DefiningFrontmatterErrorclarifies intent and encapsulates frontmatter-specific issues. The usage of acauseparameter is a neat approach to preserve error context.
….json` to 1.5.0-beta.18 [skip ci] ## [1.5.0-beta.18](1.5.0-beta.17...1.5.0-beta.18) (2025-03-23) ### Bug Fixes * 🚑️ fix a regression from eager code refactoring with file renames ([7b14189](7b14189))
|
🎉 This PR is included in version 1.5.0-beta.18 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
make sure double-first and -last names are properly capitalised.
….json` to 1.5.0-beta.19 [skip ci] ## [1.5.0-beta.19](1.5.0-beta.18...1.5.0-beta.19) (2025-03-24) ### Bug Fixes * 🐛 fix a regression with double names ("First-First", "Last-Last") ([aae443f](aae443f))
|
🎉 This PR is included in version 1.5.0-beta.19 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/services/author-parser.ts (3)
21-35: Robust parsing with flexible separators.
Splitting and filtering blank authors is straightforward. Consider adding a test case for scenarios like trailing commas or consecutive separators.
56-68: Title removal logic and normalization are well-organized.
The flow (trim → remove title → normalize) is clear. If there are additional honorifics to handle (e.g., "Sr.", "Jr.", "PhD"), consider expanding the pattern or making it user-configurable.
82-93: Informative usage examples.
Usage comments clearly demonstrate how to instantiate and modify parser behavior. Consider adding formal unit tests reflecting these examples.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
manifest.json(1 hunks)package.json(1 hunks)src/services/author-parser.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- manifest.json
🧰 Additional context used
🧬 Code Definitions (1)
src/services/author-parser.ts (1)
src/types.d.ts (1)
AuthorParserOptions(99-102)
🔇 Additional comments (9)
src/services/author-parser.ts (5)
3-8: Regex choices look good.
Your choice of regex for author separators and titles is concise and readable.
9-14: Default options assignment is solid.
Assigning defaults via the constructor ensures consistent parser behavior even if no options are provided.
37-46: Dynamic configuration updates look correct.
Merging new options preserves existing parser behavior while allowing on-the-fly reconfiguration.
48-54: Returning a clone is a good practice.
This prevents external modifications from unintentionally affecting internal parser options.
70-79:❓ Verification inconclusive
Name normalization approach is comprehensive.
Splitting on spaces and hyphens helps handle typical multi-part names. For names with apostrophes or non-Latin characters, ensure your approach remains valid.
🏁 Script executed:
#!/bin/bash # Check the codebase for tests covering names with apostrophes or non-latin characters rg -A 5 "test.*normalizeName"Length of output: 31
Action Needed: Verify Test Coverage for Edge Cases in
normalizeNameThe current implementation of
normalizeNameinsrc/services/author-parser.ts(lines 70–79) effectively normalizes names with spaces and hyphens. However, our automated search did not reveal any tests covering edge cases such as names with apostrophes or non-Latin characters. Please manually verify if these cases are properly tested or add tests to ensure the method handles them correctly.package.json (4)
2-4: Check major version alignment with breaking changes.
The PR indicates a major plugin overhaul, but version is set to"1.5.0-beta.19". If it truly introduces breaking changes, adopting a2.xversion might align better with semantic versioning guidelines.
5-12: Scripts for local deployment and release look good.
They align with the new semantic-release workflow. Ensure there's a reliable fallback if local deployment or release attempts fail.
16-34:❓ Verification inconclusive
Review devDependencies for version compatibility.
Updating rollup, TypeScript, and semantic-release is beneficial. Just confirm they’re all mutually compatible with your current code.
🏁 Script executed:
#!/bin/bash # Check TypeScript, Rollup, and plugin usage across the codebase rg -A 3 "import.*rollup" rg -A 3 "import.*semantic-release"Length of output: 389
DevDependencies Compatibility Check – Manual Verification Recommended
- The updated packages (rollup, TypeScript, and semantic-release) look to be on the right track.
- The rollup plugins (from
@rollup/plugin-typescript,@rollup/plugin-node-resolve, and@rollup/plugin-commonjs) are confirmed for usage inrollup.config.mjs.- No direct import of semantic-release was detected, which may indicate it’s used via CLI commands or configuration files instead.
- Please manually verify that semantic-release and the updated TypeScript and rollup versions are fully compatible with your current build and release processes.
35-42:❓ Verification inconclusive
New dependencies chosen are appropriate.
sindresorhus/slugifyandfilenamifyare reliable libraries for naming tasks. Ensure no functionality is lost from the removedfetch-retrylibrary if previously used for network calls.
🏁 Script executed:
#!/bin/bash # Search references to fetch-retry in the codebase to confirm it was removed and replaced properly ast-grep --pattern $'import $_ from \"fetch-retry\"'Length of output: 51
Action Required: Verify Complete Removal of
fetch-retryUsageThe changes in
package.json—including the new dependencies—look appropriate. Our initial check using an import-based search forfetch-retrydid not return any results. However, please confirm that no network calls are still relying onfetch-retryby verifying that it isn’t being used via an alternative syntax (e.g., CommonJSrequirecalls).
- Please manually review or run an additional search (for example, using
rg "fetch-retry") to check for anyrequire("fetch-retry")references.- Confirm that any functionality previously provided by
fetch-retryis now handled appropriately.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/pr.yml (1)
10-14: Update Checkout Action Version
The checkout step currently specifiesactions/checkout@v2, which has been flagged by static analysis as dated. Upgrading toactions/checkout@v3can offer improvements and better compatibility with current GitHub Actions features.Apply this diff:
- uses: actions/checkout@v2 + uses: actions/checkout@v3🧰 Tools
🪛 actionlint (1.7.4)
11-11: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pr.yml
11-11: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (4)
.github/workflows/pr.yml (4)
1-4: Pull Request Event Triggers Verification
The workflow is set to trigger on pull request events for typesopenedandedited. Please verify that these trigger types cover all intended scenarios. For instance, if you wish to update release notes when new commits are pushed to an already-open PR, consider adding thesynchronizeevent type.
15-16: Release Notes Generation Step Validity
The release notes generation step usingguilhermetod/semantic-release-notes-preview@v1.0.0appears to be correctly configured. Ensure that this action produces the desired preview output, and that its version remains compatible with your release process.
17-20: Comment Release Notes Step Configuration
The step that utilizespeter-evans/create-or-update-comment@v1is properly set up, and it correctly references the issue number from the GitHub event payload. This should work as intended for adding the release notes preview comment on the PR.
21-27: Multi-line Comment Body Formatting
The multi-line literal block for the comment body is well-formatted and includes a clear separator. Please verify that the final rendered comment maintains the intended indentation and spacing for optimal readability.
|
Thank you for your contribution. Below is a preview of the release notes if your PR gets merged. |
|
Thank you for your contribution. Below is a preview of the release notes if your PR gets merged. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/pr.yml (1)
11-14: Update Checkout Action Version.
The static analysis tool indicates that the runner foractions/checkout@v3is outdated. It is recommended to update to a newer release (if available) to ensure improved compatibility and performance.Apply the following diff if appropriate:
- uses: actions/checkout@v3 + uses: actions/checkout@v3.5.1🧰 Tools
🪛 actionlint (1.7.4)
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pr.yml(1 hunks)tsconfig.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tsconfig.json
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/pr.yml
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (6)
.github/workflows/pr.yml (6)
1-5: Workflow Trigger Configuration is Correct.
The workflow file correctly triggers on pull request events (opened, edited), which aligns with the PR objectives for providing a release notes preview.
6-10: Job and Steps Structure is Well-Organized.
The job named "release" and its associated steps are clearly defined. The configuration of the job—including specifying the Ubuntu environment and sequential steps—is straightforward and meets the intended functionality.
15-18: Node.js Setup Step is Configured Properly.
The use ofactions/setup-node@v4withnode-version: latestis appropriate. This step aligns with the requirement to set up the Node.js environment for subsequent tasks.
19-22: Dependency Installation Step is Correct.
The commandnpm clean-installalong with the environment variableHUSKY: 0is clearly specified, ensuring that dependencies install cleanly without interfering Husky hooks.
23-26: Release Notes Generation Step is Well-Defined.
The step that usesguilhermetod/semantic-release-notes-preview@v1.0.0correctly generates a preview of the release notes. The assignment of anidfor subsequent referencing is clear and appropriate.
27-35: Comment Release Notes Preview Step is Configured Correctly.
The final step, which employspeter-evans/create-or-update-comment@v1to post a comment with the release notes, is well set up. Consider verifying that the workflow does not accidentally post duplicate comments when the same PR event is re-triggered.
revert changes up to 61e5a83
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
package.json (2)
13-15: Metadata Suggestion: Populate Author FieldThe
"author"field is currently empty. For improved package metadata and clarity on project ownership, consider providing this information, if applicable.
35-42: Dependency Review: Assess Usage of MD5The inclusion of
"md5": "^2.3.0"in the dependencies warrants a review. If MD5 is used solely for non-cryptographic purposes (e.g., generating hashes for identifiers), it may be acceptable; otherwise, consider using a more secure alternative for any security-sensitive operations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json(1 hunks)
🔇 Additional comments (3)
package.json (3)
2-3: Version Field: Evaluate Semantic VersioningThe version
"1.5.0-beta.19"suggests a beta release, even though the PR introduces breaking changes. Consider bumping the major version (e.g., to"2.0.0") in the final release to clearly communicate these breaking changes per semantic versioning guidelines.
6-12: Script Configuration: Verify Semantic Release CommandsThe new deployment and release scripts reference the updated
rollup.config.mjsand add commands likedeploy:local,release:local, andrelease:dry-run. Please ensure that the use ofsemantic-release --dry-run falsein therelease:localscript is correctly interpreted and that these commands behave as intended in your build and deployment processes.
16-34: DevDependencies Upgrade: Validate CompatibilityThe updated
devDependenciessupport the new semantic release workflow and build processes. Please verify that all new dependency versions (for example,@biomejs/biome,semantic-release-obsidian-plugin, and the upgraded Rollup plugins) are mutually compatible and don’t introduce conflicts in your environment.
The updated version implements quite a few changes and new features
nunjucksfilters for Readwise features like.qafunction for Q&A in your notesAdjust Filenames to current settings) which will iterate over your Readwise library and clean up filenamesBREAKING CHANGE: due to the substantial changes in filename generation and tracking, a major, breaking, version bump seems appropriate. We'll see if
@semantic-releasethinks likewise.Summary by CodeRabbit
New Features
Breaking Changes
Documentation