feat(format): date snap options |startof:/|endof: for {{DATE}} and {{VDATE}} (#511)#1371
Merged
Merged
Conversation
Add a generic "snap a date to the start/end of a period" capability to the
{{DATE}} and {{VDATE}} tokens via pipe options, e.g.
`{{DATE:gggg.MM.[Wk]w|startof:week}}`. The formatted output then reflects the
period boundary instead of the exact instant, so a weekly-note filename whose
week crosses a month boundary resolves the month of the *week* (May) rather than
today's calendar month (June) — the issue #511 scenario — while the in-note
heading can stay day-actual with a plain {{DATE}}/{{VDATE}}.
Units: year, quarter, month, week, isoweek, day (case-insensitive). The
locale-vs-ISO week choice is an explicit unit (`week` = locale first-day,
`isoweek` = Monday), so there is no implicit anchor guessing. The existing
`+N` day offset is unchanged and applied before the snap.
Design notes:
- New token? No. This is a generic, composable property of the existing date
tokens — not a near-duplicate {{WEEK}} token.
- Backward compatible: the formatted-DATE regex peels only a trailing
`|startof:`/`|endof:` segment (keyword-anchored, lazy format slot), so any
other literal `|` in a format still renders verbatim ({{DATE:YYYY|MM}} →
2023|06). Verified no literal-pipe DATE formats exist in repo.
- VDATE snap-extraction is folded into parseVDateOptions, so all four call
sites (runtime, preflight scan, both preview overrides) strip it from the
default value automatically; the snap is applied per-occurrence at format
time on the raw stored @Date:ISO, so one picked date can render snapped in a
filename and day-actual in a heading.
- Unknown units throw a self-correcting error (moment's startOf silently
no-ops on a bad unit otherwise).
- InsertAfterFields ordered-log date auto-detect tolerates the |option suffix.
- moment@2.29.4 added as a dev-only dependency (matches obsidian's pin) so the
snap math can be unit-tested against real moment; production reads
window.moment.
Verified end-to-end in an isolated Obsidian vault for both {{DATE}} and
{{VDATE}} (incl. the dual-context recipe, ISO weeks, endof, offset+snap,
byte-identical literal pipe, and bad-unit errors).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesDate Snap Feature
Sequence Diagram(s)sequenceDiagram
participant Template as Template String
participant Formatter as Formatter
participant getDate as getDate
participant applyDateSnap as applyDateSnap
rect rgba(100, 150, 255, 0.5)
note over Template,Formatter: {{DATE:YYYY-MM|startof:month}}
Template->>Formatter: replaceDateInString
Formatter->>Formatter: DATE_REGEX_FORMATTED match → { format, offset, snapSegment }
Formatter->>Formatter: parseDateSnapSegment(snapSegment) → DateSnap
Formatter->>getDate: { format, offset, snap }
getDate->>getDate: moment().add(offset, "days")
getDate->>applyDateSnap: (moment, snap)
applyDateSnap-->>getDate: moment.startOf("month")
getDate-->>Formatter: "2025-06-01"
end
rect rgba(100, 220, 150, 0.5)
note over Template,Formatter: {{VDATE:YYYY-MM|startof:month}}
Template->>Formatter: replaceDateVariableInString
Formatter->>Formatter: parseVDateOptions → { snap, dateFormat, ... }
Formatter->>applyDateSnap: (window.moment(`@date`:...), snap)
applyDateSnap-->>Formatter: snapped Moment
Formatter-->>Template: formatted string
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying quickadd with
|
| Latest commit: |
52595c2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9bbaa2fc.quickadd.pages.dev |
| Branch Preview URL: | https://chhoumann-511-date-week-of-m.quickadd.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a74c26fd1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // (issue #511); the moment sort-parse format is the part before the first | ||
| // `|`, mirroring the VDATE handling below. | ||
| const date = after.match(/\{\{DATE:([^}\n\r+]*)(?:\+-?\d+)?(?:\|[^}\n\r]*)?\}\}/i); | ||
| const dateFormat = date?.[1]?.split("|")[0]?.trim(); |
There was a problem hiding this comment.
When an ordered capture's after text uses a DATE format with a literal pipe, such as {{DATE:YYYY|MM}} or {{DATE:YYYY|MM|startof:month}}, DATE syntax still treats non-snap pipes as part of the Moment format, but this split seeds orderBy.dateFormat as only YYYY. The runtime ordered-placement path then parses headings with that truncated format, so headings for different months in the same year compare as the same date and new sections can be inserted in the wrong order.
Useful? React with 👍 / 👎.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/gui/ChoiceBuilder/components/InsertAfterFields.svelte`:
- Around line 131-133: The `detectDateFormatFromAfter` function incorrectly
splits the captured date format string on pipes and only retains the first
segment using `.split("|")[0]`, which truncates valid formats containing literal
pipes like `{{DATE:YYYY|MM}}` into just `YYYY`. Remove the `.split("|")[0]`
operation from the dateFormat assignment so that the complete captured format
string (which is already captured by the regex) is preserved and returned
intact.
In `@src/utils/dateModifiers.test.ts`:
- Around line 2-13: The beforeAll hook in dateModifiers.test.ts changes the
global moment locale to "en" but never restores it, which can leak this state to
other tests. Modify the beforeAll hook to capture the current moment locale
before setting it to "en", then add a corresponding afterAll hook that restores
the previously captured locale using moment.locale(). This ensures the global
moment locale is not mutated for tests outside this suite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd1f6dc2-97a4-4d2c-bf3e-d8a20e186f31
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
docs/docs/FormatSyntax.mdpackage.jsonsrc/constants.tssrc/formatters/formatter.tssrc/gui/ChoiceBuilder/components/InsertAfterFields.sveltesrc/utils/dateModifiers.test.tssrc/utils/dateModifiers.tssrc/utils/dates.snap.test.tssrc/utils/dates.tssrc/utils/vdateSyntax.snap.test.tssrc/utils/vdateSyntax.ts
…re test locale
Address PR-bot review (CodeRabbit + Codex):
- InsertAfterFields.detectDateFormatFromAfter used `.split("|")[0]`, which
truncated a literal-pipe sort format ({{DATE:YYYY|MM}} -> "YYYY") and could
order headings wrongly. Use the same keyword-anchored regex as
DATE_REGEX_FORMATTED so a literal `|` is kept and only |startof:/|endof: is
excluded.
- Restore the global moment locale in afterAll for the two snap test suites so
pinning en doesn't leak into other suites.
…ring tests
Adversarial review (ultracode + 2 Codex) of the implementation found a
self-inflicted regression and coverage gaps; this addresses them:
- ReDoS: the lazy format slot overlapped the snap alternation, giving O(n^2)
backtracking on a malformed unclosed token ({{DATE: + |startof: x N, n=30000
-> ~3.2s). Replace with a deterministic, mutually-exclusive format arm
`\|(?!(?:startof|endof):[a-z])` and anchor the snap to letters
(`|startof:<letters>`). Now linear (n=30000 -> ~1ms).
- Bracket-literal abort: {{DATE:[label |startof: x ]YYYY-MM-DD|startof:month}}
previously peeled the FIRST |startof: (inside the [] literal) and threw
"Unknown date unit", aborting the whole format. Now only a trailing
|startof:<letters> is the snap; a |startof: followed by a space/non-letter
stays literal. Verified it renders correctly end-to-end.
- A malformed double-snap ({{DATE:..|startof:week|endof:month}}) and a literal
'+' format ({{DATE:YYYY+MM}}) now stay literal (no throw), matching master.
Tests:
- Extract detectDateFormatFromAfter to src/utils/insertAfterDateFormat.ts and
table-test it (the load-bearing ordered-log change had zero coverage; this is
the exact PR-bot-found literal-pipe regression).
- New formatter-datesnap.test.ts exercises the snap THROUGH replaceDateInString
and replaceDateVariableInString with real moment + a frozen clock (the wiring
was untested: mutating the snap capture group kept the old suite green).
- Replace the no-op ReDoS test (`'a|'.repeat` never entered the alternation)
with a real `|startof:`xN linear-time assertion; add bracket-literal,
double-snap, and literal-'+' grammar guards.
Owner
Author
|
Thanks bots 🤖 — addressed in
Known limitation (accepted): the VDATE builder preview shows the un-snapped date (the preview uses a hand-rolled, moment-less generator that already can't render |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/utils/insertAfterDateFormat.test.ts (1)
40-44: ⚡ Quick winAdd a regression case for multi-token headings returning
undefined.This suite should also lock the documented unsupported multi-token behavior to prevent future drift.
Suggested test case
it("returns undefined for a bare {{DATE}} or no token", () => { expect(detectDateFormatFromAfter("{{DATE}}")).toBeUndefined(); expect(detectDateFormatFromAfter("# Today")).toBeUndefined(); }); + + it("returns undefined for multi-token headings", () => { + expect( + detectDateFormatFromAfter("## {{DATE:YYYY-MM-DD}} :: {{VDATE:d,HH:mm}}"), + ).toBeUndefined(); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/insertAfterDateFormat.test.ts` around lines 40 - 44, Add a regression test case within the existing "returns undefined for a bare {{DATE}} or no token" test in the detectDateFormatFromAfter test suite to verify that multi-token headings also return undefined. This test case should document the unsupported behavior for headings containing multiple tokens (such as "# Today is {{DATE}}") by asserting that detectDateFormatFromAfter returns undefined for such inputs, which will prevent future drift from this documented unsupported case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/insertAfterDateFormat.ts`:
- Around line 12-24: The function detectDateFormatFromAfter currently extracts
and returns a date format without first checking whether the heading is
multi-token, which violates the function contract that multi-token headings
should return undefined. Add a check at the beginning of the function to detect
if the after parameter contains multiple tokens (multiple words/segments) and
return undefined early if it does, before proceeding with the existing DATE and
VDATE format extraction logic. This ensures ambiguous multi-token headings are
handled as unsupported rather than auto-seeding potentially incorrect dateFormat
values.
---
Nitpick comments:
In `@src/utils/insertAfterDateFormat.test.ts`:
- Around line 40-44: Add a regression test case within the existing "returns
undefined for a bare {{DATE}} or no token" test in the detectDateFormatFromAfter
test suite to verify that multi-token headings also return undefined. This test
case should document the unsupported behavior for headings containing multiple
tokens (such as "# Today is {{DATE}}") by asserting that
detectDateFormatFromAfter returns undefined for such inputs, which will prevent
future drift from this documented unsupported case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc7c9be8-f3c5-48fa-bf91-ebb078101efa
📒 Files selected for processing (6)
src/constants.tssrc/formatters/formatter-datesnap.test.tssrc/gui/ChoiceBuilder/components/InsertAfterFields.sveltesrc/utils/dates.snap.test.tssrc/utils/insertAfterDateFormat.test.tssrc/utils/insertAfterDateFormat.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/dates.snap.test.ts
Comment on lines
+12
to
+24
| export function detectDateFormatFromAfter(after: string): string | undefined { | ||
| const date = after.match( | ||
| /\{\{DATE:((?:[^}\n\r+|]|\|(?!(?:startof|endof):[a-z]))*)(?:\+-?\d+)?(?:\|(?:startof|endof):[a-z]+)?\}\}/i, | ||
| ); | ||
| const dateFormat = date?.[1]?.trim(); | ||
| if (dateFormat) return dateFormat; | ||
| // VDATE is {{VDATE:name,format}} or {{VDATE:name,format|default}} — drop the | ||
| // "|default"/"|startof:..." segment so it doesn't leak into the moment parse | ||
| // format (the VDATE format itself never contains a literal pipe). | ||
| const vdate = after.match(/\{\{VDATE:[^,}]+,([^}\n\r]*)\}\}/i); | ||
| const vdateFormat = vdate?.[1]?.split("|")[0]?.trim(); | ||
| if (vdateFormat) return vdateFormat; | ||
| return undefined; |
There was a problem hiding this comment.
Handle multi-token headings as unsupported before extracting a format.
The function contract says multi-token headings should return undefined, but current logic returns the first matching DATE/VDATE format. That can auto-seed an ambiguous orderBy.dateFormat.
Suggested fix
export function detectDateFormatFromAfter(after: string): string | undefined {
+ // Ambiguous input: do not infer one sort format from multiple date tokens.
+ const tokenCount = (after.match(/\{\{(?:DATE(?::|}})|VDATE:)/gi) ?? []).length;
+ if (tokenCount > 1) return undefined;
+
const date = after.match(
/\{\{DATE:((?:[^}\n\r+|]|\|(?!(?:startof|endof):[a-z]))*)(?:\+-?\d+)?(?:\|(?:startof|endof):[a-z]+)?\}\}/i,
);📝 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.
Suggested change
| export function detectDateFormatFromAfter(after: string): string | undefined { | |
| const date = after.match( | |
| /\{\{DATE:((?:[^}\n\r+|]|\|(?!(?:startof|endof):[a-z]))*)(?:\+-?\d+)?(?:\|(?:startof|endof):[a-z]+)?\}\}/i, | |
| ); | |
| const dateFormat = date?.[1]?.trim(); | |
| if (dateFormat) return dateFormat; | |
| // VDATE is {{VDATE:name,format}} or {{VDATE:name,format|default}} — drop the | |
| // "|default"/"|startof:..." segment so it doesn't leak into the moment parse | |
| // format (the VDATE format itself never contains a literal pipe). | |
| const vdate = after.match(/\{\{VDATE:[^,}]+,([^}\n\r]*)\}\}/i); | |
| const vdateFormat = vdate?.[1]?.split("|")[0]?.trim(); | |
| if (vdateFormat) return vdateFormat; | |
| return undefined; | |
| export function detectDateFormatFromAfter(after: string): string | undefined { | |
| // Ambiguous input: do not infer one sort format from multiple date tokens. | |
| const tokenCount = (after.match(/\{\{(?:DATE(?::|}})|VDATE:)/gi) ?? []).length; | |
| if (tokenCount > 1) return undefined; | |
| const date = after.match( | |
| /\{\{DATE:((?:[^}\n\r+|]|\|(?!(?:startof|endof):[a-z]))*)(?:\+-?\d+)?(?:\|(?:startof|endof):[a-z]+)?\}\}/i, | |
| ); | |
| const dateFormat = date?.[1]?.trim(); | |
| if (dateFormat) return dateFormat; | |
| // VDATE is {{VDATE:name,format}} or {{VDATE:name,format|default}} — drop the | |
| // "|default"/"|startof:..." segment so it doesn't leak into the moment parse | |
| // format (the VDATE format itself never contains a literal pipe). | |
| const vdate = after.match(/\{\{VDATE:[^,}]+,([^}\n\r]*)\}\}/i); | |
| const vdateFormat = vdate?.[1]?.split("|")[0]?.trim(); | |
| if (vdateFormat) return vdateFormat; | |
| return undefined; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/insertAfterDateFormat.ts` around lines 12 - 24, The function
detectDateFormatFromAfter currently extracts and returns a date format without
first checking whether the heading is multi-token, which violates the function
contract that multi-token headings should return undefined. Add a check at the
beginning of the function to detect if the after parameter contains multiple
tokens (multiple words/segments) and return undefined early if it does, before
proceeding with the existing DATE and VDATE format extraction logic. This
ensures ambiguous multi-token headings are handled as unsupported rather than
auto-seeding potentially incorrect dateFormat values.
…week-of-month # Conflicts: # src/gui/ChoiceBuilder/components/InsertAfterFields.svelte
Owner
Author
|
Latest CodeRabbit nitpick (multi-token heading coverage) addressed in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #511.
Problem
A weekly planner named
gggg.MM.[Wk]wshould file the week of Thursday 2023-06-01 under2023.05.Wk22.md(the week belongs to May), but{{DATE:gggg.MM.[Wk]w}}renders2023.06.Wk22— moment has no month-of-week token, and QuickAdd passes the format slot to a single barewindow.moment(). The reporter also wants the in-note heading (# 6.01 Thursday) to keep the actual day, so one capture needs two date contexts.Solution — a generic date property, not a new token
Instead of a
{{WEEK}}clone, this adds a composable capability to the existing date tokens: snap a date to the start/end of a period before formatting, via pipe options consistent with{{VALUE|...}}/{{VDATE|...}}/{{FILE|...}}:<unit>∈year · quarter · month · week · isoweek · day(case-insensitive). The locale-vs-ISO week choice is an explicit unit (week= locale first-day,isoweek= Monday) — no implicit anchor guessing. The+Nday offset is unchanged and applied before the snap.#511 recipe (single picked date, dual context):
{{VDATE:d,gggg.MM.[Wk]w|startof:week}}→2023.05.Wk22(filename) and{{VDATE:d,M.DD dddd}}→6.01 Thursday(heading).Why this shape
{{DATE}}/{{VDATE}}passes already wired into all 5 formatter entry points (runtime ×2, both display previews, preflight).|startof:/|endof:segment (lazy format slot), so every other literal|still renders verbatim:{{DATE:YYYY|MM}}→2023|06. Verified there are zero literal-pipe DATE formats in the repo.parseVDateOptions, so all four call sites strip it from the default value automatically; the snap is applied at format time on the raw stored@date:ISO, so the same variable can render snapped in one place and day-actual in another.startOfsilently no-ops otherwise); a mistyped key (|starof:) is left literal, byte-identical to today.Surfaces touched
src/utils/dateModifiers.ts(new shared kernel),dates.ts(getDatesnap),constants.ts(regexes + syntax lists),vdateSyntax.ts,formatters/formatter.ts,InsertAfterFields.svelte(ordered-log date auto-detect tolerates the option), docs (FormatSyntax.md, Next only).Process
Designed, then reviewed with an ultracode multi-lens pass plus two opposing-model (Codex) adversarial reviewers on the design before implementation; their must-fixes (the broken
[|]escape claim, the VDATE preview overrides, the fourparseVDateOptionscall sites, the test seam, a wrong offset example) are all folded in.moment@2.29.4added as a dev-only dependency (matches obsidian's pin) so the snap math is unit-tested against real moment; production still readswindow.moment.Testing
dateModifiers,dates.snap(the deterministic [FEATURE REQUEST] Option for Week-Month in {{DATE:}} #511 guard + year-boundary + units + offset-then-snap + regex grammar incl. backward-compat & no-ReDoS),vdateSyntax.snap(snap extraction + legacy round-trip preservation). Full suite: 2458 passed, 0 failures.pnpm run build(tsc + esbuild) andpnpm run lintgreen;docsbuild green;pnpm install --frozen-lockfileclean.{{DATE}}and{{VDATE}}: week/month/quarter/isoweek snaps,endof:month, offset+snap, the dual-context recipe (2023.05.Wk22 :: 6.01 Thursday),{{DATE:YYYY|MM}}→2023|06, and bad-unit errors.Summary by CodeRabbit
Release Notes
New Features
|startof:<unit>and|endof:<unit>for{{DATE}}and{{VDATE}}, applying snapping after any+/-<N>offset (with locale vs ISO week behavior).Documentation
Bug Fixes
Tests