fix: render list front matter as real properties out of the box (#662)#1338
Conversation
… of beta toggle (#662) The documented Movie & Series script broke since Obsidian's properties update: multi-value fields (cast/genre/director) were inlined as broken YAML, so the property showed "red" / failed to render. The "Template Property Types" handling that fixes this was gated behind a default-off beta flag whose string heuristic is also effectively dead in the real runtime (metadataTypeManager.getTypeInfo never returns null), so even enabling it didn't help. Decouple structured-value collection from the flag: container values (arrays/objects) from scripts are now always written as proper Obsidian properties through the native processFrontMatter serializer, even with the toggle off. The flag now only gates the opt-in string->type heuristic. Scalars (number/boolean/null) are left raw since they are already YAML-safe inlined, so valid front matter (and any comments) stays byte-identical. Also: - Fix a latent regex.lastIndex desync when a variable value is a non-string (coerce the raw replacement to a string). - movies.js: linkifyList returns a real array so cast/genre/director become List properties of links. - Rewrite the example template to YAML front matter with quoted scalar wikilinks and the plot in the note body (avoids quote/colon breakage). - Update the beta-toggle copy + docs to reflect that lists are always handled and the toggle controls string conversion. Verified flag-off end-to-end in the dev vault (lists render, comments preserved, plot with quotes/colon safe). Full suite: 1898 passed.
…ures (#662 follow-up) Editor-insertion capture actions (currentLine / newLineAbove / newLineBelow) write the capture into the note body at the cursor, but the formatting ran inside a template-property collection scope. After the #662 change made container collection flag-independent, a frontmatter-shaped capture with an array value would be replaced by a "[]" placeholder that is never applied (applyCapturePropertyVars only runs on the file-write branch) — silently dropping the value. Suppress front matter property collection for editor-insertion actions, since body insertion can't produce a real Obsidian property anyway. The value is substituted inline as text instead, matching the pre-#662 behaviour and eliminating the data loss. File-write capture paths (append/prepend/insert- after/insert-before/top/bottom and create) are unchanged — they correctly pair collect + apply. Adds a regression test exercising the real run() path: a newLineBelow capture of a frontmatter array inlines as text, leaves no placeholder, and never calls processFrontMatter.
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR fixes issue ChangesTemplate Property Types: Fix
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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: |
f3333fd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4566b40e.quickadd.pages.dev |
| Branch Preview URL: | https://chhoumann-662-movie-script-p.quickadd.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38a5931f05
ℹ️ 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".
| collectionActive, | ||
| heuristicEnabled: propertyTypesEnabled && collectionActive, |
There was a problem hiding this comment.
Avoid collecting body captures as frontmatter
When a Capture choice targets an existing file with append/bottom/insert-after and the capture format starts with frontmatter, onFileExists() runs the first formatting pass inside withTemplatePropertyCollection() before the snippet is placed in the file body. Since this now enables collection for arrays/objects regardless of the beta flag, cast: {{VALUE:cast}} is replaced by cast: []; later applyCapturePropertyVars() only rewrites the target note's frontmatter, so the inserted body snippet is left with the placeholder and the captured values appear in the wrong place. The new suppression only covers currentLine/newLineAbove/newLineBelow, so gate this to cases where the formatted snippet will actually become the file frontmatter.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in f3333fd. The editor-insertion-only suppression was too narrow. Collection is now gated to the single case where the capture content actually becomes the file's own front matter (a brand-new file created from the capture, no template). All existing-file insertions (append/bottom/insert-after/before), template-body embeds, and editor insertions suppress collection and inline the value as text — so no [] placeholder is stranded and nothing is written to the wrong note's front matter. Added a regression test covering append-into-an-existing-file.
…ile's own front matter Addresses Codex review on #1338: the editor-insertion suppression was too narrow. A frontmatter-shaped capture appended / inserted-after / written to the bottom of an EXISTING file also lands in the note body, so collecting it left a "[]" placeholder in the body while applyCapturePropertyVars wrote the values to the wrong note's real front matter. Gate collection to the only case where the capture content actually becomes the file's front matter: a brand-new file created from the capture with no template. All existing-file insertions, template-body embeds, and editor insertions now suppress collection and inline the value as text. Replaces the previous editor-insertion-only flag. Adds a regression test for append-into-an-existing-file: the array inlines in the body, leaves no placeholder, keeps the existing front matter, and never calls processFrontMatter.
Closes #662
Problem
Since Obsidian's properties update, the documented Movie & Series script produced invalid YAML front matter. Multi-value fields (
cast,genre,director) came from the script as a YAML-bullet-list-shaped string and were inlined into a quoted scalar, e.g.:…so the property stayed "red" / didn't render. The "Template Property Types" handling that should fix this was gated behind a default-off beta toggle, and its string→list heuristic is effectively dead in the real runtime (
metadataTypeManager.getTypeInfonever returnsnull), so even enabling the toggle didn't help.Fix
Decouple structured-value collection from the beta flag. Container values (arrays/objects) provided by scripts are now written as real Obsidian List/object properties through the native
processFrontMatterserializer — with the toggle off. The toggle now only gates the opt-in string → type heuristic.processFrontMatteronly runs when a value genuinely needs it.movies.js:linkifyListreturns a real array, socast/genre/directorbecome List properties of links.{{VALUE}}scanner doesn't desync (replacement.length).Follow-up (also in this PR): capture editor-insertion data-loss
Editor-insertion capture actions (
currentLine/newLineAbove/newLineBelow) write to the note body, where front matter property types are meaningless. They formatted inside a collection scope, so after the change above a frontmatter-shaped capture with an array value would be replaced by a[]placeholder that's never applied — silently dropping the value. Collection is now suppressed for these actions, so the value inlines as text (matching pre-change behaviour). File-write capture paths are unchanged.Verification
cast/genre/directoras List-of-link properties; scalars valid; a plot containing"and:is safe in the body; front-matter comments are preserved for scalar-only templates (no needless rewrite). Screenshot-confirmed.Formatterflag-off regressions (array → List + placeholder, scalars stay raw, same-line coercion), collectorgetTypeInfo→textcases, and a capture insertion-mode regression on the realrun()path. Updated the feature-flag mock tests to match the narrowed behaviour.tsc+eslintclean.Review
Two design rounds + an adversarial review (3 opposite-model lenses) on the diff. Their findings drove the narrowing-to-containers, the docs/setting-copy correction, the real-
Formattertests, and this capture follow-up.Known limitations (pre-existing, narrow — not changed here)
appendTop/appendBottom) concatenate the template into the note body, so a front-matter block there is body text (arrays inline as plain text). Append can't produce front-matter properties.processFrontMattercan't apply the collected list (logged warning). The example template + a docs tip show quoting scalar wikilinks.Release impact
fix:— patch release. No migration required; existing valid templates are unaffected (byte-identical). Users on the oldmovies.jsshould re-download the updated script.Summary by CodeRabbit
Release Notes
Documentation
Bug Fixes