feat: add explicit multi-select output formats - #1658
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds explicit ChangesMulti-select formatting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CaptureChoiceEngine
participant FieldSuggestionParser
participant formatter
participant TemplateOutput
CaptureChoiceEngine->>FieldSuggestionParser: Parse multi-select format
FieldSuggestionParser-->>CaptureChoiceEngine: Return multiFormat
CaptureChoiceEngine->>formatter: Format selected values
formatter->>TemplateOutput: Emit inline, YAML, or Markdown output
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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: |
fb8bd9a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://172ec2d1.quickadd.pages.dev |
| Branch Preview URL: | https://1649-feature-request-formatt.quickadd.pages.dev |
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/engine/CaptureChoiceEngine.ts`:
- Around line 113-123: Update MULTI_SELECT_TOKEN_REGEX so the multi flag accepts
optional whitespace before its terminator, matching FieldSuggestionParser.parse
behavior for forms such as `|multi }`. Add a regression case in the
CaptureChoiceEngine audit tests that verifies this token still triggers the
contextual multi-select warning.
In `@src/utils/FieldSuggestionParser.ts`:
- Around line 314-319: Update the format parsing in the FIELD/FILE flow around
the multiSelect and multiFormat check to track whether a format directive was
explicitly provided, rather than relying on the resolved format value; warn and
reset to auto whenever format:auto is supplied without multi. Apply the same
correction in the VALUE-token parser in valueSyntax.ts, and add regression tests
covering FIELD, FILE, and VALUE tokens with format:auto but no multi.
🪄 Autofix
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 Plus
Run ID: dae05a55-e963-4b29-834c-29bb9a640f1a
📒 Files selected for processing (17)
docs/src/content/docs/docs/FormatSyntax.mdsrc/engine/CaptureChoiceEngine.audit-capture.test.tssrc/engine/CaptureChoiceEngine.tssrc/formatters/completeFormatter.test.tssrc/formatters/formatter-field-title-regression.test.tssrc/formatters/formatter-file.test.tssrc/formatters/formatter.tssrc/utils/FieldSuggestionParser-1564-unknown-filter.test.tssrc/utils/FieldSuggestionParser.test.tssrc/utils/FieldSuggestionParser.tssrc/utils/fileSyntax.test.tssrc/utils/fileSyntax.tssrc/utils/multiValueFormat.test.tssrc/utils/multiValueFormat.tssrc/utils/valueSyntax.test.tssrc/utils/valueSyntax.tstests/e2e/multi-select-formatting.test.ts
The degradation-warning regex missed a trimmed '|multi ' flag, and an explicit |format:auto without |multi was a silent no-op instead of the 'needs |multi' warning the other format values get.
Multi-select output shape was inferred from context: picks became a real YAML list only when capturing into a brand-new note's front matter without a template; every other shape wrote comma-separated text. Users who capture into template-backed notes (the most common setup for people who want list properties) could not get a list at all, and nobody could get a vertical Markdown list in a note body (#1649).
This adds an explicit, opt-in
|format:option to all three multi-select tokens ({{VALUE:...|multi}}, {{FILE:...|multi}}, {{FIELD:...|multi}}):|format:yaml- quoted YAML flow sequence (topics: ["Alpha", "Beta"]); Obsidian reads it as a native list in any front matter, including template-backed captures.|format:markdown- vertical bullet list, preserving the line's indentation.|format:inline- the existing comma-separated text, now nameable.|format:auto(default) - the legacy context-sensitive behavior, byte-for-byte unchanged. Nothing changes for existing setups.It composes with the existing item options (
|multi:linklist,|link,|path,|text:,|custom), and|format:without|multiwarns and is ignored. The capture-time warning that previously just said "this writes comma-separated text" now points at the new flags.Before (capture into a template-backed note -
topicsis a single string):After (
|format:yamlin front matter,|format:markdownin the body):Includes docs (with "Available in the next release" callouts), unit coverage for the parser/renderer/formatter paths, and a live e2e test (
tests/e2e/multi-select-formatting.test.ts) that drives a template-backed capture and asserts Obsidian's metadata cache reads a native list. Also verified interactively in Obsidian 1.13.4 (screenshots above).pnpm test(4883 tests),pnpm build-with-lintpass.Fixes #1649
Summary by CodeRabbit