feat: docusaurus support #376
Conversation
![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
Outside diff range and nitpick comments (1)
src/jekyll/chirpy.ts (1)
Line range hint
56-99: Enhance error handling and logging inconvertToChirpy.While the function has some error handling, adding more descriptive logging can help in troubleshooting and maintaining the code.
await validateSettings(plugin, settings); try { const markdownFiles = await copyMarkdownFile(plugin); for (const file of markdownFiles) { const fileName = convertFileName(file.name); const frontMatterConverter = new FrontMatterConverter( fileName, settings.jekyllRelativeResourcePath, settings.isEnableBanner, settings.isEnableUpdateFrontmatterTimeOnEdit, ); const resourceLinkConverter = new ResourceLinkConverter( fileName, settings.resourcePath(), vaultAbsolutePath(plugin), settings.attachmentsFolder, settings.jekyllRelativeResourcePath, ); const curlyBraceConverter = new CurlyBraceConverter( settings.isEnableCurlyBraceConvertMode, ); const result = ConverterChain.create() .chaining(frontMatterConverter) .chaining(resourceLinkConverter) .chaining(curlyBraceConverter) .chaining(new WikiLinkConverter()) .chaining(new CalloutConverter()) .chaining(new FootnotesConverter()) .chaining(new CommentsConverter()) .chaining(new EmbedsConverter()) .converting(await plugin.app.vault.read(file)); await plugin.app.vault.modify(file, result); await moveFiles( file, `${vaultAbsolutePath(plugin)}/${settings.readyFolder}`, settings.targetPath(), settings.pathReplacer, ) .then(() => new Notice('Moved files to Chirpy successfully.', 5000)); } } catch (e) { console.error('Error during Chirpy conversion:', e); new Notice('Chirpy conversion failed, see console for more information.'); }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/docusaurus/docusaurus.ts (1 hunks)
- src/jekyll/FrontMatterConverter.ts (4 hunks)
- src/jekyll/chirpy.ts (2 hunks)
- src/utils.ts (2 hunks)
Files not reviewed due to errors (1)
- src/jekyll/FrontMatterConverter.ts (no review received)
Files skipped from review as they are similar to previous changes (2)
- src/docusaurus/docusaurus.ts
- src/utils.ts
Additional context used
Learnings (1)
src/jekyll/FrontMatterConverter.ts (1)
Learnt from: songkg7 PR: songkg7/o2#376 File: src/jekyll/FrontMatterConverter.ts:9-29 Timestamp: 2024-06-28T03:24:10.861Z Learning: In `js-yaml` version 4, `yaml.safeLoad` has been removed, and `yaml.load` is safe by default and does not execute arbitrary code.
Biome
src/jekyll/FrontMatterConverter.ts
[error] 145-145: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 156-156: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 157-157: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 158-158: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- README.md (2 hunks)
- src/docusaurus/docusaurus.ts (1 hunks)
- src/jekyll/chirpy.ts (2 hunks)
- src/utils.ts (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/docusaurus/docusaurus.ts
- src/jekyll/chirpy.ts
- src/utils.ts
Additional context used
LanguageTool
README.md
[uncategorized] ~56-~56: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...theassets/img/<yyyy-MM-dd>folder. - Auto create folders: If enabled, the plugin will au...(AUTO_HYPHEN)
[style] ~94-~94: Using many exclamation marks might seem excessive (in this case: 13 exclamation marks for a text that’s 4247 characters long)
Context: ...B0/) Welcome to write articles about O2! ## License This project is published ...(EN_EXCESSIVE_EXCLAMATION)
Markdownlint
README.md
106-106: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
Additional comments not posted (4)
README.md (4)
57-57: LGTM!
63-63: LGTM!
65-65: LGTM!
66-66: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range and nitpick comments (1)
src/jekyll/FrontMatterConverter.ts (1)
Line range hint
134-149: Consider Returning a New Object and Avoid UsingdeleteModifying the input object directly can lead to side effects. Consider returning a new object instead to ensure immutability. Also, avoid using the
deleteoperator for performance reasons.- delete frontMatter.updated; + const newFrontMatter = { ...frontMatter }; + newFrontMatter.updated = undefined; + return newFrontMatter;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/docusaurus/docusaurus.ts (1 hunks)
- src/jekyll/FrontMatterConverter.ts (4 hunks)
- src/tests/FootnotesConverter.test.ts (2 hunks)
- src/tests/FrontMatterConverter.test.ts (3 hunks)
- src/utils.ts (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/docusaurus/docusaurus.ts
- src/tests/FootnotesConverter.test.ts
Additional context used
Learnings (1)
src/jekyll/FrontMatterConverter.ts (1)
Learnt from: songkg7 PR: songkg7/o2#376 File: src/jekyll/FrontMatterConverter.ts:9-29 Timestamp: 2024-06-28T03:24:10.861Z Learning: In `js-yaml` version 4, `yaml.safeLoad` has been removed, and `yaml.load` is safe by default and does not execute arbitrary code.
Biome
src/jekyll/FrontMatterConverter.ts
[error] 145-145: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 158-158: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 161-161: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 162-162: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Additional comments not posted (8)
src/jekyll/FrontMatterConverter.ts (2)
9-30: Correct Usage ofyaml.loadThe usage of
yaml.loadfrom thejs-yamllibrary is appropriate here, considering the updates in version 4.
32-39: LGTM!The
joinfunction correctly constructs a string from the front matter and body content.src/utils.ts (5)
Line range hint
9-19: LGTM!The
vaultAbsolutePathfunction correctly retrieves the absolute path of the vault.
42-45: LGTM!The
getFilesInReadyfunction correctly retrieves markdown files from the ready folder.
55-62: LGTM!The
copyFilefunction correctly copies a file from the source path to the target path.
127-144: LGTM!The
transformPathfunction correctly transforms a file path based on a date extraction pattern.
152-162: LGTM!The
parseLocalDatefunction correctly parses a date string into aLocalDateobject.src/tests/FrontMatterConverter.test.ts (1)
Line range hint
1-17: LGTM!The test case correctly verifies that the
convertmethod does not alter the input if the image conversion is disabled.
src/jekyll/FrontMatterConverter.ts
Outdated
| function convertImageFrontMatter( | ||
| isEnable: boolean, | ||
| fileName: string, | ||
| resourcePath: string, | ||
| frontMatter: FrontMatter, | ||
| ) { | ||
| if (!isEnable) { | ||
| return frontMatter; | ||
| } | ||
|
|
||
| if (ObsidianRegex.ATTACHMENT_LINK.test(frontMatter.image)) { | ||
| const match = frontMatter.image.match(ObsidianRegex.ATTACHMENT_LINK); | ||
| if (match) { | ||
| frontMatter.image = frontMatter.image.replace(ObsidianRegex.ATTACHMENT_LINK, '$1.$2'); | ||
| } | ||
| } | ||
| frontMatter.image = convertImagePath(this.fileName, frontMatter.image, this.resourcePath); | ||
| if (!frontMatter.image) { | ||
| return frontMatter; | ||
| } | ||
|
|
||
| const match = ObsidianRegex.ATTACHMENT_LINK.exec(frontMatter.image); | ||
| if (match) { | ||
| frontMatter.image = `${match[1]}.${match[2]}`; | ||
| } | ||
| frontMatter.image = convertImagePath(fileName, frontMatter.image, resourcePath); | ||
| return frontMatter; |
There was a problem hiding this comment.
Consider Returning a New Object and Avoid Using delete
Modifying the input object directly can lead to side effects. Consider returning a new object instead to ensure immutability. Also, avoid using the delete operator for performance reasons.
- frontMatter.image = `${match[1]}.${match[2]}`;
- delete frontMatter['updated'];
+ const newFrontMatter = { ...frontMatter };
+ newFrontMatter.image = `${match[1]}.${match[2]}`;
+ newFrontMatter.updated = undefined;
+ return newFrontMatter;Committable suggestion was skipped due to low confidence.
src/jekyll/FrontMatterConverter.ts
Outdated
| export const convertFrontMatter = (input: string) => { | ||
| const [frontMatter, body] = parseFrontMatter(input); | ||
| if (Object.keys(frontMatter).length === 0) { | ||
| return input; | ||
| } | ||
|
|
||
| if (frontMatter.updated) { | ||
| frontMatter.date = frontMatter.updated; | ||
| delete frontMatter.updated; | ||
| } | ||
|
|
||
| delete frontMatter['aliases']; | ||
| delete frontMatter['published']; | ||
|
|
||
| return join( | ||
| convert({ ...frontMatter }), | ||
| body, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Consider Returning a New Object and Avoid Using delete
Modifying the input object directly can lead to side effects. Consider returning a new object instead to ensure immutability. Also, avoid using the delete operator for performance reasons.
- delete frontMatter['aliases'];
- delete frontMatter['published'];
+ const newFrontMatter = { ...frontMatter };
+ newFrontMatter.aliases = undefined;
+ newFrontMatter.published = undefined;
+ return join(
+ convert(newFrontMatter),
+ body,
+ );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.
| export const convertFrontMatter = (input: string) => { | |
| const [frontMatter, body] = parseFrontMatter(input); | |
| if (Object.keys(frontMatter).length === 0) { | |
| return input; | |
| } | |
| if (frontMatter.updated) { | |
| frontMatter.date = frontMatter.updated; | |
| delete frontMatter.updated; | |
| } | |
| delete frontMatter['aliases']; | |
| delete frontMatter['published']; | |
| return join( | |
| convert({ ...frontMatter }), | |
| body, | |
| ); | |
| }; | |
| export const convertFrontMatter = (input: string) => { | |
| const [frontMatter, body] = parseFrontMatter(input); | |
| if (Object.keys(frontMatter).length === 0) { | |
| return input; | |
| } | |
| if (frontMatter.updated) { | |
| frontMatter.date = frontMatter.updated; | |
| frontMatter.updated = undefined; | |
| } | |
| const newFrontMatter = { ...frontMatter }; | |
| newFrontMatter.aliases = undefined; | |
| newFrontMatter.published = undefined; | |
| return join( | |
| convert(newFrontMatter), | |
| body, | |
| ); | |
| }; |
Tools
Biome
[error] 158-158: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 161-161: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 162-162: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
src/jekyll/FrontMatterConverter.ts
Outdated
| const convert = (frontMatter: FrontMatter) => { | ||
| const fm = { ...frontMatter }; | ||
| // if not around front matter title using double quote, add double quote | ||
| fm.title = fm.title?.startsWith('"') ? fm.title : `"${fm.title}"`; | ||
|
|
||
| // if not around front matter categories using an array, add an array | ||
| if (fm.categories && JSON.stringify(fm.categories).startsWith('[')) { | ||
| fm.categories = `${JSON.stringify(fm.categories) | ||
| .replace(/,/g, ', ') | ||
| .replace(/"/g, '') | ||
| }`; | ||
| } | ||
|
|
||
| // if fm.tags is array | ||
| if (fm.tags) { | ||
| fm.tags = Array.isArray(fm.tags) ? `[${fm.tags.join(', ')}]` : `[${fm.tags}]`; | ||
| } | ||
|
|
||
| return fm; | ||
| }; |
There was a problem hiding this comment.
Consider Returning a New Object and Avoid Using delete
Modifying the input object directly can lead to side effects. Consider returning a new object instead to ensure immutability. Also, avoid using the delete operator for performance reasons.
- return fm;
+ return { ...fm };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.
| const convert = (frontMatter: FrontMatter) => { | |
| const fm = { ...frontMatter }; | |
| // if not around front matter title using double quote, add double quote | |
| fm.title = fm.title?.startsWith('"') ? fm.title : `"${fm.title}"`; | |
| // if not around front matter categories using an array, add an array | |
| if (fm.categories && JSON.stringify(fm.categories).startsWith('[')) { | |
| fm.categories = `${JSON.stringify(fm.categories) | |
| .replace(/,/g, ', ') | |
| .replace(/"/g, '') | |
| }`; | |
| } | |
| // if fm.tags is array | |
| if (fm.tags) { | |
| fm.tags = Array.isArray(fm.tags) ? `[${fm.tags.join(', ')}]` : `[${fm.tags}]`; | |
| } | |
| return fm; | |
| }; | |
| const convert = (frontMatter: FrontMatter) => { | |
| const fm = { ...frontMatter }; | |
| // if not around front matter title using double quote, add double quote | |
| fm.title = fm.title?.startsWith('"') ? fm.title : `"${fm.title}"`; | |
| // if not around front matter categories using an array, add an array | |
| if (fm.categories && JSON.stringify(fm.categories).startsWith('[')) { | |
| fm.categories = `${JSON.stringify(fm.categories) | |
| .replace(/,/g, ', ') | |
| .replace(/"/g, '') | |
| }`; | |
| } | |
| // if fm.tags is array | |
| if (fm.tags) { | |
| fm.tags = Array.isArray(fm.tags) ? `[${fm.tags.join(', ')}]` : `[${fm.tags}]`; | |
| } | |
| return { ...fm }; | |
| }; |
| export const moveFiles = async ( | ||
| sourceFolderPath: string, | ||
| targetFolderPath: string, | ||
| pathReplacer: (year: string, month: string, day: string, title: string) => string, | ||
| publishedDate?: LocalDate, | ||
| ) => { | ||
|
|
||
| copy( | ||
| sourceFolderPath, | ||
| targetFolderPath, | ||
| pathReplacer, | ||
| publishedDate, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Consider Using Asynchronous File Operations in moveFiles
Using synchronous operations (fs.copyFileSync) in this context can block the Node.js event loop, especially with a large number of files. Consider refactoring to use asynchronous methods to improve performance and non-blocking behavior.
- fs.copyFileSync(sourceFilePath, targetFilePath);
+ fs.copyFile(sourceFilePath, targetFilePath, (err) => {
+ if (err) {
+ console.error('Error copying file:', err);
+ return;
+ }
+ console.log(`Copied ${sourceFilePath} to ${targetFilePath}`);
+ });Committable suggestion was skipped due to low confidence.
| export const copy = ( | ||
| sourceFolderPath: string, | ||
| targetFolderPath: string, | ||
| replacer: (year: string, month: string, day: string, title: string) => string, | ||
| publishedDate?: LocalDate, | ||
| ) => { | ||
| fs.readdirSync(sourceFolderPath) | ||
| .filter(filename => filename.startsWith(TEMP_PREFIX)) | ||
| .forEach((filename) => { | ||
| const transformedFileName = transformPath( | ||
| filename, | ||
| replacer, | ||
| publishedDate, | ||
| ); | ||
|
|
||
| const sourceFilePath = path.join(sourceFolderPath, filename); | ||
| const targetFilePath = path.join(targetFolderPath, transformedFileName.replace(TEMP_PREFIX, '').replace(/\s/g, '-')); | ||
|
|
||
| copyFile(sourceFilePath, targetFilePath); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Consider Using Asynchronous File Operations in copy
Using synchronous operations (fs.copyFileSync) in this context can block the Node.js event loop, especially with a large number of files. Consider refactoring to use asynchronous methods to improve performance and non-blocking behavior.
- fs.copyFileSync(sourceFilePath, targetFilePath);
+ fs.copyFile(sourceFilePath, targetFilePath, (err) => {
+ if (err) {
+ console.error('Error copying file:', err);
+ return;
+ }
+ console.log(`Copied ${sourceFilePath} to ${targetFilePath}`);
+ });Committable suggestion was skipped due to low confidence.
| export const achieve = async (plugin: O2Plugin, settings: O2PluginSettings) => { | ||
| if (!settings.isAutoAchieve) { | ||
| return; | ||
| } | ||
|
|
||
| // move files to achieve folder | ||
| const readyFiles = getFilesInReady(plugin); | ||
| readyFiles.forEach((file: TFile) => { | ||
| return plugin.app.vault.copy(file, file.path.replace(settings.readyFolder, settings.achieveFolder)); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Ensure Proper Error Handling in achieve
The function currently does not handle errors that might occur during the file copy for achieving. Adding error handling can prevent silent failures and improve reliability.
+ try {
return plugin.app.vault.copy(file, file.path.replace(settings.readyFolder, settings.achieveFolder));
+ } catch (error) {
+ console.error(`Failed to achieve file ${file.name}:`, error);
+ new Notice('Failed to achieve file, see console for more information.');
+ }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.
| export const achieve = async (plugin: O2Plugin, settings: O2PluginSettings) => { | |
| if (!settings.isAutoAchieve) { | |
| return; | |
| } | |
| // move files to achieve folder | |
| const readyFiles = getFilesInReady(plugin); | |
| readyFiles.forEach((file: TFile) => { | |
| return plugin.app.vault.copy(file, file.path.replace(settings.readyFolder, settings.achieveFolder)); | |
| }); | |
| }; | |
| export const achieve = async (plugin: O2Plugin, settings: O2PluginSettings) => { | |
| if (!settings.isAutoAchieve) { | |
| return; | |
| } | |
| // move files to achieve folder | |
| const readyFiles = getFilesInReady(plugin); | |
| readyFiles.forEach((file: TFile) => { | |
| try { | |
| return plugin.app.vault.copy(file, file.path.replace(settings.readyFolder, settings.achieveFolder)); | |
| } catch (error) { | |
| console.error(`Failed to achieve file ${file.name}:`, error); | |
| new Notice('Failed to achieve file, see console for more information.'); | |
| } | |
| }); | |
| }; |
| export async function backupOriginalNotes(plugin: O2Plugin) { | ||
| const readyFiles = getFilesInReady(plugin); | ||
| const backupFolder = plugin.jekyll.achieveFolder; | ||
| const readyFolder = plugin.jekyll.readyFolder; | ||
| readyFiles.forEach((file: TFile) => { | ||
| return plugin.app.vault.copy(file, file.path.replace(readyFolder, backupFolder)); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Ensure Proper Error Handling in backupOriginalNotes
The function currently does not handle errors that might occur during the file copy for backup. Adding error handling can prevent silent failures and improve reliability.
+ try {
return plugin.app.vault.copy(file, file.path.replace(readyFolder, backupFolder));
+ } catch (error) {
+ console.error(`Failed to backup file ${file.name}:`, error);
+ new Notice('Failed to backup file, see console for more information.');
+ }Committable suggestion was skipped due to low confidence.
| export const copyMarkdownFile = async (plugin: O2Plugin): Promise<TFile[]> => { | ||
| const dateString = Temporal.Now.plainDateISO().toString(); | ||
| const markdownFiles = getFilesInReady(plugin); | ||
| for (const file of markdownFiles) { | ||
| const newFileName = TEMP_PREFIX + dateString + '-' + file.name; | ||
| const newPath = file.path | ||
| .replace(file.name, newFileName) | ||
| .replace(/,+/g, '') | ||
| .replace(/\s/g, '-'); | ||
|
|
||
| await plugin.app.vault.copy(file, newPath) | ||
| .catch((error) => { | ||
| console.error(error); | ||
| new Notice('Failed to copy file, see console for more information.'); | ||
| }); | ||
| } | ||
|
|
||
| // collect copied files | ||
| return plugin.app.vault.getMarkdownFiles() | ||
| .filter((file: TFile) => file.path.includes(TEMP_PREFIX)); | ||
| }; |
There was a problem hiding this comment.
Review Error Handling in copyMarkdownFile
The function attempts to handle errors during the file copy operation, but it returns a promise that might not be properly awaited by the callers. Ensure that the error handling is consistent and that callers are aware they need to handle potential rejections.
+ try {
await plugin.app.vault.copy(file, newPath);
+ } catch (error) {
+ console.error(error);
+ new Notice('Failed to copy file, see console for more information.');
+ }Committable suggestion was skipped due to low confidence.
| export const cleanUp = async (plugin: O2Plugin) => { | ||
| // remove temp files | ||
| const markdownFiles = plugin.app.vault.getMarkdownFiles() | ||
| .filter((file) => file.path.includes(TEMP_PREFIX)); | ||
|
|
||
| for (const file of markdownFiles) { | ||
| await plugin.app.vault.delete(file) | ||
| .then(() => { | ||
| console.log(`Deleted temp file: ${file.path}`); | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Review Error Handling in cleanUp
The function attempts to handle errors during the file deletion operation, but it returns a promise that might not be properly awaited by the callers. Ensure that the error handling is consistent and that callers are aware they need to handle potential rejections.
+ try {
await plugin.app.vault.delete(file);
+ } catch (error) {
+ console.error(`Failed to delete file ${file.name}:`, error);
+ new Notice('Failed to delete file, see console for more information.');
+ }Committable suggestion was skipped due to low confidence.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
README.md (1)
Line range hint
94-94: Reduce excessive exclamation marks.Using many exclamation marks might seem excessive. Consider reducing the number of exclamation marks for better style.
- If you ever want to buy me a coffee, don't hesitate. + If you ever want to buy me a coffee, don't hesitate!
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- README.md (2 hunks)
Additional context used
LanguageTool
README.md
[uncategorized] ~38-~38: Possible missing article found.
Context: ... via obsidian'scmd + pshortcut. if exception occurs, you can see the original note i...(AI_HYDRA_LEO_MISSING_AN)
[style] ~94-~94: Using many exclamation marks might seem excessive (in this case: 13 exclamation marks for a text that’s 4247 characters long)
Context: ...B0/) Welcome to write articles about O2! ## License This project is published ...(EN_EXCESSIVE_EXCLAMATION)
Markdownlint
README.md
106-106: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
Additional comments not posted (3)
README.md (3)
56-57: Approved: Hyphen addition for "Auto-create folders".The hyphen improves readability and consistency.
82-83: Approved: Sentence break fix.The fix improves the clarity of the sentence.
63-68: Verify syntax examples for accuracy.Ensure that the syntax examples for image resizing and embedding are correct.
Verification successful
Syntax examples are accurate.
The syntax examples for image resizing and embedding in the README.md file are correct and match the expected patterns.
- Image resizing (width):
{: width="100" }- Image resizing (width & height):
{: width="100" height="200" }- Embeds:
![[Obsidian#This is Obsidian!]]Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify syntax examples for image resizing and embedding. # Test: Search for syntax examples in the codebase. Expect: Correct syntax examples. rg --type markdown 'image.*\{\: width="100" \}' && rg --type markdown 'image.*\{\: width="100" height="200" \}' && rg --type markdown 'Obsidian'Length of output: 151
Script:
#!/bin/bash # Description: Verify syntax examples for image resizing and embedding. # Test: Search for syntax examples in the codebase. Expect: Correct syntax examples. # Correct the regex patterns to avoid escape sequence errors rg --type markdown 'image.*\{: width="100" \}' && rg --type markdown 'image.*\{: width="100" height="200" \}' && rg --type markdown 'Obsidian'Length of output: 1864
There was a problem hiding this comment.
Actionable comments posted: 7
Outside diff range and nitpick comments (4)
src/FilenameConverter.ts (1)
1-5: Ensure consistent handling of file extensions.The
convertFileNamefunction currently only removes.mdextensions. Consider making this more flexible to handle other extensions if needed.// Example: export const convertFileName = (filename: string): string => filename .replace(/\.[^/.]+$/, '') // Removes any file extension .replace(/\s/g, '-') .replace(/[^a-zA-Z0-9-\uAC00-\uD7A3]/g, '');src/CommentsConverter.ts (1)
1-2: Consider removing redundant import statements.The
Converterimport is not used in this file. Consider removing it to clean up the code.import { ObsidianRegex } from './core/ObsidianRegex';src/WikiLinkConverter.ts (1)
1-2: Consider removing redundant import statements.The
Converterimport is not used in this file. Consider removing it to clean up the code.import { ObsidianRegex } from './core/ObsidianRegex';src/ResourceLinkConverter.ts (1)
Line range hint
46-107: Improve error handling and use async/await for file operations.The current implementation uses synchronous file operations and logs errors to the console. Consider using async/await for non-blocking operations and improve error handling.
import fs from 'fs/promises'; export class ResourceLinkConverter implements Converter { private readonly fileName: string; private readonly resourcePath: string; private readonly absolutePath: string; private readonly attachmentsFolder: string; private readonly relativeResourcePath: string; constructor(fileName: string, resourcePath: string, absolutePath: string, attachmentsFolder: string, relativeResourcePath: string) { this.fileName = fileName; this.resourcePath = resourcePath; this.absolutePath = absolutePath; this.attachmentsFolder = attachmentsFolder; this.relativeResourcePath = relativeResourcePath; } async convert(input: string): Promise<string> { const sanitizedFileName = removeTempPrefix(this.fileName); const resourcePath = `${this.resourcePath}/${sanitizedFileName}`; const resourceNames = extractResourceNames(input); if (resourceNames && resourceNames.length > 0) { await fs.mkdir(resourcePath, { recursive: true }); } if (resourceNames) { for (const resourceName of resourceNames) { try { await fs.copyFile( `${this.absolutePath}/${this.attachmentsFolder}/${resourceName}`, `${resourcePath}/${resourceName.replace(/\s/g, '-')}` ); } catch (err) { console.error(err); new Notice(err.message); } } } const replacer = (match: string, contents: string, suffix: string, width: string | undefined, height: string | undefined, space: string | undefined, caption: string | undefined) => `}.${suffix})` + `${convertImageSize(width, height)}` + `${convertImageCaption(caption)}`; return input.replace(ObsidianRegex.ATTACHMENT_LINK, replacer); } }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (22)
- src/CalloutConverter.ts (1 hunks)
- src/CommentsConverter.ts (1 hunks)
- src/CurlyBraceConverter.ts (2 hunks)
- src/EmbedsConverter.ts (1 hunks)
- src/FilenameConverter.ts (1 hunks)
- src/FootnotesConverter.ts (1 hunks)
- src/FrontMatterConverter.ts (1 hunks)
- src/ResourceLinkConverter.ts (3 hunks)
- src/WikiLinkConverter.ts (1 hunks)
- src/core/ObsidianRegex.ts (1 hunks)
- src/docusaurus/docusaurus.ts (1 hunks)
- src/jekyll/chirpy.ts (2 hunks)
- src/tests/CalloutConverter.test.ts (3 hunks)
- src/tests/CommentsConverter.test.ts (1 hunks)
- src/tests/CurlyBraceConverter.test.ts (1 hunks)
- src/tests/EmbedsConverter.test.ts (1 hunks)
- src/tests/FilenameConverter.test.ts (2 hunks)
- src/tests/FootnotesConverter.test.ts (2 hunks)
- src/tests/FrontMatterConverter.test.ts (3 hunks)
- src/tests/ObsidianRegex.test.ts (1 hunks)
- src/tests/ResourceLinkConverter.test.ts (1 hunks)
- src/tests/WikiLinkConverter.test.ts (1 hunks)
Files skipped from review due to trivial changes (7)
- src/EmbedsConverter.ts
- src/tests/CommentsConverter.test.ts
- src/tests/CurlyBraceConverter.test.ts
- src/tests/EmbedsConverter.test.ts
- src/tests/ObsidianRegex.test.ts
- src/tests/ResourceLinkConverter.test.ts
- src/tests/WikiLinkConverter.test.ts
Files skipped from review as they are similar to previous changes (5)
- src/docusaurus/docusaurus.ts
- src/tests/CalloutConverter.test.ts
- src/tests/FilenameConverter.test.ts
- src/tests/FootnotesConverter.test.ts
- src/tests/FrontMatterConverter.test.ts
Additional context used
Biome
src/FrontMatterConverter.ts
[error] 145-145: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 158-158: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 161-161: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 162-162: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Additional comments not posted (18)
src/core/ObsidianRegex.ts (1)
5-5: Verify the correctness of the updatedCALLOUTregex pattern.The updated
CALLOUTregex pattern includes an additional capturing group. Ensure this change is intended and correctly handled in the code that uses this regex.Verification successful
The updated
CALLOUTregex pattern is correctly handled by thejekyllReplaceranddocusaurusReplacerfunctions.
- The
jekyllReplaceranddocusaurusReplacerfunctions insrc/CalloutConverter.tsare designed to handle three capturing groups, which aligns with the updatedCALLOUTregex pattern.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the updated CALLOUT regex pattern. # Test: Search for the usage of ObsidianRegex.CALLOUT. Expect: Correct handling of capturing groups. rg --type typescript 'ObsidianRegex.CALLOUT'Length of output: 79
Script:
#!/bin/bash # Description: Verify the usage of the updated CALLOUT regex pattern. # Test: Search for the usage of ObsidianRegex.CALLOUT. Expect: Correct handling of capturing groups. rg --type ts 'ObsidianRegex.CALLOUT'Length of output: 315
Script:
#!/bin/bash # Description: Verify the handling of capturing groups in the updated CALLOUT regex pattern. # Search for the definitions of jekyllReplacer and docusaurusReplacer in the codebase. rg --type ts 'jekyllReplacer' rg --type ts 'docusaurusReplacer'Length of output: 596
src/CommentsConverter.ts (1)
4-9: Ensure consistency between class method and standalone function.The
convertmethod inCommentsConverterclass and the standaloneconvertCommentsfunction perform the same operation. Ensure consistency in their implementations.src/WikiLinkConverter.ts (1)
4-8: Ensure consistency between class method and standalone function.The
convertmethod inWikiLinkConverterclass and the standaloneconvertWikiLinkfunction perform the same operation. Ensure consistency in their implementations.src/FootnotesConverter.ts (2)
1-11: LGTM!The
FootnotesConverterclass correctly implements theConverterinterface and uses a regex pattern to transform footnotes.
13-17: LGTM!The
convertFootnotesfunction correctly uses a regex pattern to transform footnotes.src/CurlyBraceConverter.ts (2)
Line range hint
1-18: LGTM!The
CurlyBraceConverterclass correctly implements theConverterinterface and uses a regex pattern to transform content based on theisEnableflag.
20-25: LGTM!The
convertCurlyBracefunction correctly uses a regex pattern to transform content based on theisEnableflag.src/CalloutConverter.ts (5)
1-8: LGTM!The
CalloutConverterclass correctly implements theConverterinterface and uses a helper function to transform callout syntax.
10-12: LGTM!The
convertCalloutSyntaxToChirpyfunction correctly uses a regex pattern and a replacer function to transform callout syntax.
44-49: LGTM!The
convertDocusaurusCalloutfunction correctly uses a regex pattern and a replacer function to transform callout syntax for Docusaurus.
51-52: LGTM!The
replaceDocusaurusKeywordfunction correctly maps callout keywords to Docusaurus-specific keywords.
81-82: LGTM!The
replaceDocusaurusContentsfunction correctly transforms the contents of a callout.src/jekyll/chirpy.ts (1)
Line range hint
55-96: LGTM!The
convertToChirpyfunction correctly performs multiple operations including file reads, transformations, and writes. Error handling and logging are included.src/ResourceLinkConverter.ts (3)
2-7: Update import paths for consistency.Ensure that all import paths are consistent and correct. The changes seem to align with the new directory structure.
Line range hint
108-115: LGTM!
Line range hint
116-137: LGTM!src/FrontMatterConverter.ts (2)
1-4: Update import paths for consistency.Ensure that all import paths are consistent and correct. The changes seem to align with the new directory structure.
32-39: LGTM!
| export const removeTempPrefix = (filename: string): string => | ||
| filename.replace('o2-temp', ''); |
There was a problem hiding this comment.
Consider making the prefix to remove configurable.
The removeTempPrefix function currently removes a hardcoded prefix. Making this configurable can enhance flexibility.
export const removeTempPrefix = (filename: string, prefix: string = 'o2-temp'): string =>
filename.replace(prefix, '');| export const convertJekyllResourceLink = ( | ||
| input: string, | ||
| fileName: string, | ||
| absolutePath: string, | ||
| settings: JekyllSettings, | ||
| ) => { | ||
| const resourcePath = `${settings.resourcePath}/${fileName}`; | ||
| const resourceNames = extractResourceNames(input); | ||
| if (!(resourceNames === undefined || resourceNames.length === 0)) { | ||
| fs.mkdirSync(resourcePath, { recursive: true }); | ||
| } | ||
| resourceNames?.forEach((resourceName) => { | ||
| fs.copyFile( | ||
| `${absolutePath}/${settings.attachmentsFolder}/${resourceName}`, | ||
| `${resourcePath}/${(resourceName.replace(/\s/g, '-'))}`, | ||
| (err) => { | ||
| if (err) { | ||
| // ignore error | ||
| console.error(err); | ||
| new Notice(err.message); | ||
| } | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| const replacer = (match: string, | ||
| contents: string, | ||
| suffix: string, | ||
| width: string | undefined, | ||
| height: string | undefined, | ||
| space: string | undefined, | ||
| caption: string | undefined) => | ||
| `}.${suffix})` | ||
| + `${convertImageSize(width, height)}` | ||
| + `${convertImageCaption(caption)}`; | ||
|
|
||
| return input.replace(ObsidianRegex.ATTACHMENT_LINK, replacer); | ||
| }; |
There was a problem hiding this comment.
Improve error handling and use async/await for file operations.
The current implementation uses synchronous file operations and logs errors to the console. Consider using async/await for non-blocking operations and improve error handling.
import fs from 'fs/promises';
export const convertJekyllResourceLink = async (
input: string,
fileName: string,
absolutePath: string,
settings: JekyllSettings,
) => {
const resourcePath = `${settings.resourcePath}/${fileName}`;
const resourceNames = extractResourceNames(input);
if (resourceNames && resourceNames.length > 0) {
await fs.mkdir(resourcePath, { recursive: true });
}
if (resourceNames) {
for (const resourceName of resourceNames) {
try {
await fs.copyFile(
`${absolutePath}/${settings.attachmentsFolder}/${resourceName}`,
`${resourcePath}/${resourceName.replace(/\s/g, '-')}`
);
} catch (err) {
console.error(err);
new Notice(err.message);
}
}
}
const replacer = (match: string, contents: string, suffix: string, width: string | undefined, height: string | undefined, space: string | undefined, caption: string | undefined) =>
`}.${suffix})`
+ `${convertImageSize(width, height)}`
+ `${convertImageCaption(caption)}`;
return input.replace(ObsidianRegex.ATTACHMENT_LINK, replacer);
};| // WIP | ||
| // TODO: implement this function | ||
| export const convertToChirpyV2 = async (plugin: O2Plugin) => { | ||
| const settings = plugin.jekyll as JekyllSettings; | ||
| const markdownFiles = await copyMarkdownFile(plugin); | ||
| for (const file of markdownFiles) { | ||
| const fileName = convertFileName(file.name); | ||
| const contents: Contents = await plugin.app.vault.read(file); | ||
|
|
||
| const filenameConverter = new FilenameConverter(); | ||
| const result = | ||
| convertCurlyBrace( | ||
| settings.isEnableCurlyBraceConvertMode, | ||
| convertFrontMatter( | ||
| convertJekyllResourceLink( | ||
| contents, | ||
| fileName, | ||
| vaultAbsolutePath(plugin), | ||
| settings, | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| await plugin.app.vault.modify(file, result) | ||
| .then(() => { | ||
| new Notice('Converted to Chirpy successfully.', 5000); | ||
| }); | ||
|
|
||
| // move files to chirpy folder | ||
| await moveFiles( | ||
| `${vaultAbsolutePath(plugin)}/${settings.readyFolder}`, | ||
| settings.targetPath(), | ||
| settings.pathReplacer, | ||
| ) | ||
| .then(() => new Notice('Moved files to Chirpy successfully.', 5000)); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Implement convertToChirpyV2 function and enhance error handling and logging.
The function is currently a WIP and has a TODO comment. Enhance error handling and add more descriptive logging to help in troubleshooting and maintaining the code.
+ try {
const settings = plugin.jekyll as JekyllSettings;
const markdownFiles = await copyMarkdownFile(plugin);
for (const file of markdownFiles) {
const fileName = convertFileName(file.name);
const contents: Contents = await plugin.app.vault.read(file);
const result =
convertCurlyBrace(
settings.isEnableCurlyBraceConvertMode,
convertFrontMatter(
convertJekyllResourceLink(
contents,
fileName,
vaultAbsolutePath(plugin),
settings,
),
),
);
await plugin.app.vault.modify(file, result)
.then(() => {
new Notice('Converted to Chirpy successfully.', 5000);
});
// move files to chirpy folder
await moveFiles(
`${vaultAbsolutePath(plugin)}/${settings.readyFolder}`,
settings.targetPath(),
settings.pathReplacer,
)
.then(() => new Notice('Moved files to Chirpy successfully.', 5000));
}
+ } catch (error) {
+ console.error('Error during Chirpy conversion:', error);
+ new Notice('Chirpy conversion failed, see console for more information.');
+ }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.
| // WIP | |
| // TODO: implement this function | |
| export const convertToChirpyV2 = async (plugin: O2Plugin) => { | |
| const settings = plugin.jekyll as JekyllSettings; | |
| const markdownFiles = await copyMarkdownFile(plugin); | |
| for (const file of markdownFiles) { | |
| const fileName = convertFileName(file.name); | |
| const contents: Contents = await plugin.app.vault.read(file); | |
| const filenameConverter = new FilenameConverter(); | |
| const result = | |
| convertCurlyBrace( | |
| settings.isEnableCurlyBraceConvertMode, | |
| convertFrontMatter( | |
| convertJekyllResourceLink( | |
| contents, | |
| fileName, | |
| vaultAbsolutePath(plugin), | |
| settings, | |
| ), | |
| ), | |
| ); | |
| await plugin.app.vault.modify(file, result) | |
| .then(() => { | |
| new Notice('Converted to Chirpy successfully.', 5000); | |
| }); | |
| // move files to chirpy folder | |
| await moveFiles( | |
| `${vaultAbsolutePath(plugin)}/${settings.readyFolder}`, | |
| settings.targetPath(), | |
| settings.pathReplacer, | |
| ) | |
| .then(() => new Notice('Moved files to Chirpy successfully.', 5000)); | |
| } | |
| }; | |
| // WIP | |
| // TODO: implement this function | |
| export const convertToChirpyV2 = async (plugin: O2Plugin) => { | |
| try { | |
| const settings = plugin.jekyll as JekyllSettings; | |
| const markdownFiles = await copyMarkdownFile(plugin); | |
| for (const file of markdownFiles) { | |
| const fileName = convertFileName(file.name); | |
| const contents: Contents = await plugin.app.vault.read(file); | |
| const result = | |
| convertCurlyBrace( | |
| settings.isEnableCurlyBraceConvertMode, | |
| convertFrontMatter( | |
| convertJekyllResourceLink( | |
| contents, | |
| fileName, | |
| vaultAbsolutePath(plugin), | |
| settings, | |
| ), | |
| ), | |
| ); | |
| await plugin.app.vault.modify(file, result) | |
| .then(() => { | |
| new Notice('Converted to Chirpy successfully.', 5000); | |
| }); | |
| // move files to chirpy folder | |
| await moveFiles( | |
| `${vaultAbsolutePath(plugin)}/${settings.readyFolder}`, | |
| settings.targetPath(), | |
| settings.pathReplacer, | |
| ) | |
| .then(() => new Notice('Moved files to Chirpy successfully.', 5000)); | |
| } | |
| } catch (error) { | |
| console.error('Error during Chirpy conversion:', error); | |
| new Notice('Chirpy conversion failed, see console for more information.'); | |
| } | |
| }; |
| const parseFrontMatter = (content: string): [FrontMatter, string] => { | ||
| if (!content.startsWith('---')) { | ||
| return [{}, content]; | ||
| } | ||
|
|
||
| // for define front matter boundary | ||
| const endOfFrontMatter = content.indexOf('---', 3); | ||
| if (endOfFrontMatter === -1) { | ||
| return [{}, content]; | ||
| } | ||
|
|
||
| const frontMatterLines = content.substring(3, endOfFrontMatter); | ||
| const body = content.substring(endOfFrontMatter + 3).trimStart(); | ||
|
|
||
| try { | ||
| const frontMatter = yaml.load(frontMatterLines) as FrontMatter; | ||
| return [frontMatter, body]; | ||
| } catch (e) { | ||
| console.error(e); | ||
| return [{}, content]; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Improve error handling in parseFrontMatter.
Provide more context in the error message to make debugging easier.
const parseFrontMatter = (content: string): [FrontMatter, string] => {
if (!content.startsWith('---')) {
return [{}, content];
}
// for define front matter boundary
const endOfFrontMatter = content.indexOf('---', 3);
if (endOfFrontMatter === -1) {
return [{}, content];
}
const frontMatterLines = content.substring(3, endOfFrontMatter);
const body = content.substring(endOfFrontMatter + 3).trimStart();
try {
const frontMatter = yaml.load(frontMatterLines) as FrontMatter;
return [frontMatter, body];
} catch (e) {
console.error(`Error parsing front matter: ${e.message}`);
return [{}, content];
}
};
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (2)
- .github/workflows/auto-release.yml (2 hunks)
- .github/workflows/ci.yml (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- .github/workflows/auto-release.yml
- .github/workflows/ci.yml
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #19 #346
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
CalloutConverterclass for converting callout syntax.CommentsConverter,CurlyBraceConverter,EmbedsConverter, andFootnotesConverterclasses for specific syntax conversions.FilenameConverterandFrontMatterConverter.README.mdwith new syntax adjustments and formatting changes.v3tov4with additional permissions.ltsin configuration files.