feat: Replace {{date}}, {{time}} and {{status}} placeholders in custom templates
#23
Conversation
📝 WalkthroughWalkthroughRefactors date/time handling in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)src/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
tests/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-12-28T16:39:43.487ZApplied to files:
🧬 Code graph analysis (1)tests/file-writer.test.ts (2)
🔇 Additional comments (6)
Comment |
![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
🧹 Nitpick comments (1)
src/file-writer.ts (1)
773-792: Fix comment: time format is 24-hour (HH:mm), not 12-hour (hh:mm).The helper methods are well-implemented and correctly format date/time values. However, the comment on line 785 says "(hh:mm)" which typically denotes 12-hour format, but the code produces 24-hour format. The comment should be updated for accuracy.
🔎 Proposed fix for the comment
/** - * Format a time for Flow frontmatter (hh:mm) + * Format a time for Flow frontmatter (HH:mm) */ private formatTime(date: Date): string {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/file-writer.tssrc/settings-tab.ts
🔇 Additional comments (4)
src/settings-tab.ts (1)
354-354: LGTM! Documentation accurately reflects new template variables.The updated description correctly documents the new
{{date}},{{time}}, and{{status}}placeholders alongside the existing ones, matching the implementation infile-writer.ts.src/file-writer.ts (3)
372-375: LGTM! Clean date/time computation.The granular date/time variables are well-structured and provide the necessary flexibility for template replacement. The separation of
date,time, anddateTimealigns with the PR objectives to support both the Obsidian Templates plugin format and custom use cases.
389-392: LGTM! Template placeholder replacements are correctly implemented.The regex patterns properly handle optional whitespace around placeholder names, and the global flag ensures all occurrences are replaced. The
{{status}}replacement usingdefaultStatusaligns with the PR objectives.
496-497: LGTM! Fallback path maintains consistency.The fallback implementation correctly uses the same date/time formatting approach as the template-based path, ensuring consistent behavior regardless of whether a custom template exists.
Also applies to: 516-516
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/file-writer.ts (1)
400-403: The misleading comment about 12-hour format handling remains.As previously noted, the comment on line 400 claims to "Handle both 12-hour (hh:mm) and 24-hour (HH:mm) formats," but both Templater placeholders are replaced with
dateTime, which uses 24-hour format exclusively.If 24-hour standardization is intentional, remove or update the comment to reflect this. Otherwise, implement actual 12-hour conversion for the
hh:mmplaceholder.
🧹 Nitpick comments (1)
src/file-writer.ts (1)
439-450: Consider extracting checkbox creation logic to reduce duplication.The logic for creating checkboxes and formatting action text (handling
isDone,isWaiting, completion dates) is duplicated across multiple methods:buildProjectContent,buildProjectContentFallback,addToNextActionsFile,addActionToSection, andcreateSectionWithAction.Extracting this to a helper method would improve maintainability and ensure consistency across all action creation points.
Example helper method
/** * Format an action item with appropriate checkbox and metadata */ private formatActionItem( action: string, isWaiting: boolean = false, isDone: boolean = false, dueDate?: string ): string { let checkbox: string; let actionText = action; if (isDone) { checkbox = "- [x]"; const completionDate = new Date().toISOString().split("T")[0]; actionText = `${action} ✅ ${completionDate}`; } else if (isWaiting) { checkbox = "- [w]"; } else { checkbox = "- [ ]"; } const dueDateSuffix = dueDate ? ` 📅 ${dueDate}` : ""; return `${checkbox} ${actionText}${dueDateSuffix}`; }Then replace duplicated logic with calls to
this.formatActionItem(action, isWaiting, isDone, dueDate).Also applies to: 456-470, 543-561, 564-578
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/file-writer.ts
🔇 Additional comments (4)
src/file-writer.ts (4)
372-375: LGTM! Well-structured date/time handling.The separation of
dateTime,date, andtimevariables provides clear semantics:dateTimefor frontmatter timestamps anddate/timefor template placeholders. This aligns well with the PR objective to support the Templates plugin's standard placeholders.
389-392: LGTM! Template placeholder replacements implemented correctly.The new
{{date}},{{time}}, and{{status}}placeholders are properly replaced with appropriate values. The regex patterns correctly handle optional whitespace, and the status placeholder correctly references the default status setting.
496-497: LGTM! Fallback method correctly uses dateTime for frontmatter.The fallback method appropriately uses
dateTimefor thecreation-datefrontmatter field, consistent with the template-based approach. Since the fallback builds content directly without template replacements, it doesn't need separatedateandtimevariables.Also applies to: 516-516
760-792: LGTM! Well-structured date/time formatting methods.The three formatting methods are cleanly separated and correctly implement their respective formats:
formatDateTime: "YYYY-MM-DD HH:mm" for frontmatter timestampsformatDate: "YYYY-MM-DD" for date-only placeholdersformatTime: "HH:mm" for time-only placeholdersThe JSDoc has been corrected to accurately reflect the 24-hour format output.
- Add tests for {{date}} placeholder (YYYY-MM-DD format)
- Add tests for {{time}} placeholder (HH:mm format)
- Add tests for {{status}} placeholder (uses defaultStatus setting)
- Clarify comment about Templater date syntax handling
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/file-writer.ts (1)
372-392: File is missing required ABOUTME comments.Per coding guidelines, all
.tsfiles must start with two ABOUTME comments describing file purpose. The file currently begins with imports instead.The implementation at lines 372-392 is clean: the single
nowvariable ensures consistency across all date/time formatters, and the regex patterns properly handle whitespace. However,defaultStatushas a defined default value of"live"insrc/types/settings.ts, so no additional validation is needed.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/file-writer.tstests/file-writer.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: All source files must start with two ABOUTME comments describing the file purpose
Use PascalCase for class names
Use camelCase for function and variable names
Use UPPER_SNAKE_CASE for exported constants
Files:
src/file-writer.ts
tests/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.ts: Use Jest with ts-jest preset for testing with an 80% code coverage threshold
Use generateDeterministicFakeApiKey() from tests/test-utils.ts for test API keys to avoid security scanner false positives
Files:
tests/file-writer.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-28T16:39:43.487Z
Learnt from: CR
Repo: tavva/flow PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T16:39:43.487Z
Learning: Applies to tests/**/*.ts : Use generateDeterministicFakeApiKey() from tests/test-utils.ts for test API keys to avoid security scanner false positives
Applied to files:
tests/file-writer.test.ts
🧬 Code graph analysis (1)
tests/file-writer.test.ts (2)
src/types/gtd.ts (1)
GTDProcessingResult(16-33)tests/__mocks__/obsidian.ts (1)
TFile(3-11)
🔇 Additional comments (6)
tests/file-writer.test.ts (3)
429-477: LGTM! Well-structured test for{{date}}placeholder.The test properly validates that the
{{date}}placeholder is replaced with YYYY-MM-DD format (date only, no time component). The negative assertion ensures the time component is not included.
479-527: LGTM! Well-structured test for{{time}}placeholder.The test properly validates that the
{{time}}placeholder is replaced with HH:mm format (24-hour time only).
529-576: LGTM! Well-structured test for{{status}}placeholder.The test properly validates that the
{{status}}placeholder is replaced with thedefaultStatusvalue from settings.src/file-writer.ts (3)
400-403: LGTM! Templater syntax handling clarified.The updated comment accurately reflects that both HH:mm and hh:mm patterns are replaced with 24-hour format. This addresses the previous review concern about misleading documentation.
496-497: LGTM! Correct usage in fallback method.The fallback method correctly creates only
dateTimesince it generates content directly without processing template placeholders.
760-792: LGTM! Well-implemented formatter helper methods.The three new private helper methods (
formatDateTime,formatDate,formatTime) are cleanly implemented with:
- Clear JSDoc documentation
- Consistent formatting logic
- Proper zero-padding
- Appropriate method visibility (private)
|
Thanks @cr7pt0gr4ph7! |
2 participants
This adds support for the standard
{{date}}and{{time}}placeholders of the offical Templates plugin, as well as support for{{status}}so custom templates can also access theDefault Project Statussetting.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.