feat: Add Docusaurus authors support #397
Conversation
- Implement authors handling in FrontMatterConverter - Add authors field to DocusaurusSettings - Pass authors to convertFrontMatter in docusaurus.ts - Add UI setting for Docusaurus authors in settings.ts This commit adds support for specifying authors in Docusaurus front matter. Users can now set multiple authors in the plugin settings, which will be correctly formatted in the output. Single authors are handled as strings, while multiple authors are formatted as arrays in the front matter.
WalkthroughThe changes introduce enhancements to the handling of author information within the front matter of markdown files. A new optional parameter for authors is added to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant O2SettingTab
participant DocusaurusSettings
participant FrontMatterConverter
User->>O2SettingTab: Add authors setting
O2SettingTab->>DocusaurusSettings: Update authors
DocusaurusSettings->>O2SettingTab: Confirm authors updated
User->>FrontMatterConverter: Convert front matter with authors
FrontMatterConverter->>FrontMatterConverter: Process authors
FrontMatterConverter->>User: Return updated front matter
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
src/settings.ts (1)
70-81: LGTM with suggestions: New Docusaurus authors setting implemented correctly.The new
addDocusaurusAuthorsSetting()method is well-implemented and follows the existing patterns in the file. However, there are a few suggestions for improvement:
- Consider enhancing the placeholder text to be more descriptive, e.g., "e.g., John Doe, Jane Smith".
- Add input validation to ensure the correct format for multiple authors.
- Consider handling the case where multiple authors are entered incorrectly, perhaps by trimming whitespace and splitting the input.
Here's a suggested implementation incorporating these improvements:
private addDocusaurusAuthorsSetting() { const docusaurus = this.plugin.docusaurus as DocusaurusSettings; new Setting(this.containerEl) .setName('Docusaurus authors') .setDesc('Author(s) for Docusaurus front matter. For multiple authors, separate with commas.') .addText(text => text .setPlaceholder('e.g., John Doe, Jane Smith') .setValue(docusaurus.authors) .onChange(async (value) => { // Trim whitespace and split by comma const authors = value.split(',').map(author => author.trim()).filter(author => author !== ''); docusaurus.authors = authors.join(', '); await this.plugin.saveSettings(); })); }This implementation trims whitespace, removes empty entries, and joins the authors back into a comma-separated string, ensuring a consistent format.
src/tests/FrontMatterConverter.test.ts (4)
398-416: Verify date formatting and content preservation in author addition testThe test case successfully verifies the addition of a single author to the front matter. However, there are two points to consider:
The expected output shows a formatted date (line 408), which differs from the input date format. Ensure this is the intended behavior of the
convertFrontMatterfunction.The test doesn't explicitly verify that other existing front matter content is preserved. Consider adding an additional field in the input and expected output to ensure the function doesn't inadvertently modify other content.
You could improve the test by adding an additional field and using a more specific date assertion:
const input = `--- title: "Test Post" date: 2021-01-01 categories: [test] --- Content here `; const expected = `--- title: "Test Post" date: 2021-01-01 categories: [test] authors: jmarcey --- Content here `; const result = convertFrontMatter(input, 'jmarcey'); expect(result).toBe(expected);
418-436: Address date formatting and enhance content preservation checkThis test case successfully verifies the addition of multiple authors as an array in the front matter. However, there are two areas for improvement:
The expected output shows a formatted date (line 428), which differs from the input date format. Ensure this is the intended behavior of the
convertFrontMatterfunction.The test doesn't explicitly verify that other existing front matter content is preserved. Consider adding an additional field in the input and expected output to ensure the function doesn't inadvertently modify other content.
You could enhance the test by adding an additional field and using a more specific date assertion:
const input = `--- title: "Test Post" date: 2021-01-01 categories: [test, multiple] --- Content here `; const expected = `--- title: "Test Post" date: 2021-01-01 categories: [test, multiple] authors: [jmarcey, slorber] --- Content here `; const result = convertFrontMatter(input, 'jmarcey, slorber'); expect(result).toBe(expected);
438-457: Enhance author replacement test and consider additional scenariosThis test case successfully verifies the replacement of existing authors with new ones in the front matter. However, there are several points to consider:
The expected output shows a formatted date (line 449), which differs from the input date format. Ensure this is the intended behavior of the
convertFrontMatterfunction.The test doesn't explicitly verify that other existing front matter content is preserved. Consider adding an additional field in the input and expected output to ensure the function doesn't inadvertently modify other content.
It might be beneficial to add another test case that verifies the replacement of multiple existing authors with a single new author, to cover all possible scenarios.
You could improve the test by addressing these points:
const input = `--- title: "Test Post" date: 2021-01-01 categories: [test] authors: oldauthor --- Content here `; const expected = `--- title: "Test Post" date: 2021-01-01 categories: [test] authors: [jmarcey, slorber] --- Content here `; const result = convertFrontMatter(input, 'jmarcey, slorber'); expect(result).toBe(expected); // Additional test case it('should replace multiple existing authors with a single author', () => { const input = `--- title: "Test Post" date: 2021-01-01 authors: [oldauthor1, oldauthor2] --- Content here `; const expected = `--- title: "Test Post" date: 2021-01-01 authors: newauthor --- Content here `; const result = convertFrontMatter(input, 'newauthor'); expect(result).toBe(expected); });
459-469: Ensure consistency in date formatting and enhance content preservation checkThis test case correctly verifies that no authors are added when none are provided. However, there are two points to consider:
The input date format (line 462) is inconsistent with the date formats used in previous test cases. For consistency, consider using the same date format across all test cases.
The test doesn't include other front matter fields. Adding more fields would help ensure that the function preserves all existing content when no authors are provided.
You could improve the test by addressing these points:
const input = `--- title: "Test Post" date: 2021-01-01 categories: [test] tags: [sample] --- Content here `; const result = convertFrontMatter(input); expect(result).toBe(input);This change would make the test more robust and consistent with the other test cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- src/FrontMatterConverter.ts (3 hunks)
- src/docusaurus/docusaurus.ts (1 hunks)
- src/docusaurus/settings/DocusaurusSettings.ts (1 hunks)
- src/settings.ts (1 hunks)
- src/tests/FrontMatterConverter.test.ts (1 hunks)
🧰 Additional context used
🪛 Biome
src/FrontMatterConverter.ts
[error] 170-170: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 171-171: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (6)
src/docusaurus/docusaurus.ts (1)
56-56: LGTM! Authors parameter added successfully.The addition of
plugin.docusaurus.authorsas a parameter toconvertFrontMatteris in line with the PR objective of adding Docusaurus authors support. The change is minimal and correctly integrated into the existing nested function calls.To ensure that
plugin.docusaurus.authorsis properly defined and used consistently, let's verify its definition and usage:src/settings.ts (2)
67-68: LGTM: Appropriate integration of the new setting.The new
addDocusaurusAuthorsSetting()method is correctly integrated into thedisplay()method under the Docusaurus section. This addition aligns with the PR objectives to add Docusaurus authors support.
Line range hint
1-81: Overall assessment: Well-implemented feature with minor improvement suggestions.The changes to add Docusaurus authors support in the settings are well-integrated and follow existing patterns. The new
addDocusaurusAuthorsSetting()method is correctly implemented and called in the appropriate place. Some minor suggestions were made to improve user experience and input handling, but these are not critical issues. The changes align well with the PR objectives to add Docusaurus authors support.src/FrontMatterConverter.ts (3)
53-56: Authors field is correctly handledThe implementation appropriately processes the
authorsfield by formatting it as an array when there are multiple authors, ensuring consistent front matter formatting.
155-155: Function signature updated successfullyThe
convertFrontMatterfunction now accepts an optionalauthorsparameter, enhancing flexibility in specifying authors programmatically.
169-173: Properly updating 'authors' and 'author' fieldsThe code accurately removes existing
authorsandauthorfields before setting the newauthorsvalue, ensuring the front matter reflects the updated authors without residual data.🧰 Tools
🪛 Biome
[error] 170-170: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 171-171: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
| @@ -4,6 +4,7 @@ import { DateExtractionPattern } from '../DateExtractionPattern'; | |||
| export default class DocusaurusSettings implements O2PluginSettings { | |||
| docusaurusPath: string; | |||
| dateExtractionPattern: string; | |||
| authors: string; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing the authors property implementation.
The addition of the authors property is a good step towards supporting Docusaurus authors. However, there are a few potential improvements to consider:
- The current type
stringmight not be flexible enough for multiple authors. Consider usingstring[]instead to support an array of authors. - Adding JSDoc comments would improve the documentation and provide more context about the property's usage.
- It might be beneficial to initialize the property with a default value (e.g., an empty array if using
string[]) to prevent potential null/undefined issues.
Here's a suggested implementation with these improvements:
/**
* The authors of the Docusaurus content.
* This can be a single author or multiple authors.
*/
authors: string[] = [];Could you please clarify if there's a specific reason for using string instead of string[] for the authors property? This would help in understanding if the current implementation aligns with the intended use case.
There was a problem hiding this comment.
The user will fill in the authors through the input window, so can this value be recognized as array? I think we should first receive the value and then convert it into array.
There was a problem hiding this comment.
@songkg7, thank you for the clarification.
You're absolutely right; since users fill in the authors through the input window as a string, keeping the authors property as a string in DocusaurusSettings makes sense. Converting it into an array during the front matter conversion process is a suitable approach to handle multiple authors. If you need any assistance with the conversion logic or ensuring that multiple authors are parsed correctly, please let me know.
(_/)
(o.o)?
(>💡<)
✏️ Learnings added
Learnt from: songkg7
PR: songkg7/o2#397
File: src/docusaurus/settings/DocusaurusSettings.ts:7-7
Timestamp: 2024-10-19T11:32:28.774Z
Learning: In `DocusaurusSettings.ts`, the `authors` property is a `string` because users input authors as a string through the settings UI. The string is then converted into an array during the front matter conversion to handle multiple authors.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| describe('convertFrontMatter with author settings', () => { | ||
| it('should add single author to front matter', () => { | ||
| const input = `--- | ||
| title: "Test Post" | ||
| date: 2021-01-01 | ||
| --- | ||
|
|
||
| Content here | ||
| `; | ||
| const expected = `--- | ||
| title: "Test Post" | ||
| date: Fri Jan 01 2021 09:00:00 GMT+0900 (Korean Standard Time) | ||
| authors: jmarcey | ||
| --- | ||
|
|
||
| Content here | ||
| `; | ||
| const result = convertFrontMatter(input, 'jmarcey'); | ||
| expect(result).toBe(expected); | ||
| }); | ||
|
|
||
| it('should add multiple authors to front matter', () => { | ||
| const input = `--- | ||
| title: "Test Post" | ||
| date: 2021-01-01 | ||
| --- | ||
|
|
||
| Content here | ||
| `; | ||
| const expected = `--- | ||
| title: "Test Post" | ||
| date: Fri Jan 01 2021 09:00:00 GMT+0900 (Korean Standard Time) | ||
| authors: [jmarcey, slorber] | ||
| --- | ||
|
|
||
| Content here | ||
| `; | ||
| const result = convertFrontMatter(input, 'jmarcey, slorber'); | ||
| expect(result).toBe(expected); | ||
| }); | ||
|
|
||
| it('should replace existing authors in front matter', () => { | ||
| const input = `--- | ||
| title: "Test Post" | ||
| date: 2021-01-01 | ||
| authors: oldauthor | ||
| --- | ||
|
|
||
| Content here | ||
| `; | ||
| const expected = `--- | ||
| title: "Test Post" | ||
| date: Fri Jan 01 2021 09:00:00 GMT+0900 (Korean Standard Time) | ||
| authors: [jmarcey, slorber] | ||
| --- | ||
|
|
||
| Content here | ||
| `; | ||
| const result = convertFrontMatter(input, 'jmarcey, slorber'); | ||
| expect(result).toBe(expected); | ||
| }); | ||
|
|
||
| it('should not add authors if not provided', () => { | ||
| const input = `--- | ||
| title: "Test Post" | ||
| date: Fri Jan 01 2021 09:00:00 GMT+0900 (Korean Standard Time) | ||
| --- | ||
|
|
||
| Content here | ||
| `; | ||
| const result = convertFrontMatter(input); | ||
| expect(result).toBe(input); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Overall assessment of new author handling test cases
The new test cases for author handling in the FrontMatterConverter are a valuable addition to the test suite. They cover the main scenarios: adding single authors, adding multiple authors, replacing existing authors, and not adding authors when none are provided. This comprehensive coverage helps ensure the reliability of the author handling feature.
However, there are some areas for improvement:
-
Date Formatting: There's inconsistency in date formatting across the test cases. Standardize the date format in both input and expected output across all tests.
-
Content Preservation: Enhance the tests to verify that other front matter content is preserved when authors are added or modified.
-
Additional Scenario: Consider adding a test case for replacing multiple existing authors with a single new author.
-
Consistency: Ensure all test cases use similar structures for input and expected output, including additional front matter fields like categories or tags.
To address these points, consider refactoring the test suite as follows:
- Standardize date formats across all tests.
- Add additional front matter fields (e.g., categories, tags) to all test cases.
- Ensure expected outputs match input formats for unchanged fields.
- Add a test case for replacing multiple authors with a single author.
These improvements will make the test suite more robust and consistent, providing better coverage for the author handling functionality.
- Update date format in convertFrontMatter tests with author settings - Change all instances to use "2021-01-01 12:00:00 +0900" format - Ensure consistency with other test cases in the file
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/tests/FrontMatterConverter.test.ts (1)
397-470: Approve new test cases for author handling with suggestions for improvementThe new test cases for author handling in the
convertFrontMatterfunction are a valuable addition to the test suite. They cover the main scenarios: adding single authors, adding multiple authors, replacing existing authors, and not adding authors when none are provided.However, there are some areas for improvement:
Date Formatting: There's inconsistency in date formatting across the test cases. Standardize the date format in both input and expected output across all tests.
Content Preservation: Enhance the tests to verify that other front matter content is preserved when authors are added or modified.
Additional Scenario: Consider adding a test case for replacing multiple existing authors with a single new author.
Consistency: Ensure all test cases use similar structures for input and expected output, including additional front matter fields like categories or tags.
To address these points, consider refactoring the test suite as follows:
describe('convertFrontMatter with author settings', () => { const baseInput = `--- title: "Test Post" date: 2021-01-01T12:00:00+09:00 categories: [test] tags: [sample] --- Content here `; it('should add single author to front matter', () => { const expected = `--- title: "Test Post" date: 2021-01-01T12:00:00+09:00 categories: [test] tags: [sample] authors: jmarcey --- Content here `; const result = convertFrontMatter(baseInput, 'jmarcey'); expect(result).toBe(expected); }); it('should add multiple authors to front matter', () => { const expected = `--- title: "Test Post" date: 2021-01-01T12:00:00+09:00 categories: [test] tags: [sample] authors: [jmarcey, slorber] --- Content here `; const result = convertFrontMatter(baseInput, 'jmarcey, slorber'); expect(result).toBe(expected); }); it('should replace existing single author with multiple authors in front matter', () => { const input = `--- title: "Test Post" date: 2021-01-01T12:00:00+09:00 categories: [test] tags: [sample] authors: oldauthor --- Content here `; const expected = `--- title: "Test Post" date: 2021-01-01T12:00:00+09:00 categories: [test] tags: [sample] authors: [jmarcey, slorber] --- Content here `; const result = convertFrontMatter(input, 'jmarcey, slorber'); expect(result).toBe(expected); }); it('should replace existing multiple authors with single author in front matter', () => { const input = `--- title: "Test Post" date: 2021-01-01T12:00:00+09:00 categories: [test] tags: [sample] authors: [oldauthor1, oldauthor2] --- Content here `; const expected = `--- title: "Test Post" date: 2021-01-01T12:00:00+09:00 categories: [test] tags: [sample] authors: newauthor --- Content here `; const result = convertFrontMatter(input, 'newauthor'); expect(result).toBe(expected); }); it('should not add authors if not provided', () => { const result = convertFrontMatter(baseInput); expect(result).toBe(baseInput); }); });These improvements will make the test suite more robust and consistent, providing better coverage for the author handling functionality.
This commit adds support for specifying authors in Docusaurus front matter.
Users can now set multiple authors in the plugin settings, which will be correctly formatted in the output. Single authors are handled as strings, while multiple authors are formatted as arrays in the front matter.
Pull Request
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?
Currently, there is no support for specifying authors in Docusaurus front matter through the plugin settings.
Issue Number: N/A
What is the new behavior?
Users can now specify authors in the plugin settings. The plugin will automatically format the authors in the front matter:
Single author:
authors: authorNameMultiple authors:
authors: [author1, author2]Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Tests