fix(builder): make the live format preview inert#1560
Merged
Conversation
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPreview formatting now routes parser warnings and errors into deduplicated diagnostics instead of runtime notices. Template previews avoid runtime execution, the UI displays delayed diagnostics safely, filename previews use filename semantics, and preflight scans remain silent. ChangesPreview diagnostics and warning routing
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: |
6d043dd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7da74a3f.quickadd.pages.dev |
| Branch Preview URL: | https://chhoumann-1558-preview-notic.quickadd.pages.dev |
chhoumann
force-pushed
the
chhoumann/1558-preview-notices
branch
from
July 26, 2026 21:56
37c4629 to
f9f28b4
Compare
This was referenced Jul 26, 2026
[BUG] {{TEMPLATE:}} in a file-name field previews as a literal token, but resolves at run time
#1563
Closed
chhoumann
marked this pull request as ready for review
July 26, 2026 22:02
Owner
Author
|
Design and diff both went through adversarial multi-agent review before this was marked ready (4 independent lenses → per-finding refuters → synthesis; 11 design findings and 4 diff findings survived refutation and are folded in). The diff review found no behavioural defect, but it did catch a real class of problem worth naming here: three justification comments asserted something stronger than what the code does, and two of those wrong claims had already propagated into commit messages, a test name and this PR's body.
All three comments, both commit messages, the test names and this PR body are corrected in Follow-ups filed rather than folded in: #1563 ( |
![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
🤖 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/formatters/previewDiagnostics.ts`:
- Around line 74-79: Update describePreviewFailure to run isCancellationError on
the original unknown error before rejecting non-Error values, returning null for
raw string cancellations such as “cancelled” and “no input given.” Preserve
PREVIEW_FAILED_MESSAGE for other non-Error failures and retain the existing
Error-message handling.
🪄 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 Plus
Run ID: f99316e7-04d1-43a2-94d5-209d394762da
📒 Files selected for processing (20)
src/formatters/displayFormatter-1558-inert.test.tssrc/formatters/fileNameDisplayFormatter.tssrc/formatters/formatDisplayFormatter-1558-template.test.tssrc/formatters/formatDisplayFormatter.tssrc/formatters/formatter-named-suggester.test.tssrc/formatters/formatter.tssrc/formatters/previewDiagnostics.tssrc/gui/ChoiceBuilder/components/FormatPreviewField.sveltesrc/gui/ChoiceBuilder/components/FormatPreviewField.test.tssrc/preflight/RequirementCollector.silence.test.tssrc/preflight/RequirementCollector.tssrc/preflight/runOnePagePreflight.filenamePreview.test.tssrc/preflight/runOnePagePreflight.tssrc/styles.csssrc/utils/FieldSuggestionParser.tssrc/utils/valueSyntax.audit-formatter-core.test.tssrc/utils/valueSyntax.test.tssrc/utils/valueSyntax.tssrc/utils/valueSyntax.warnSink.test.tssrc/utils/warnSink.ts
chhoumann
added a commit
that referenced
this pull request
Jul 26, 2026
…d of reporting it
`describePreviewFailure` gated on `instanceof Error` before consulting
`isCancellationError`. But QuickAdd's modals reject with a bare STRING
(`GenericInputPrompt.ts:293` -> `rejectPromise("No input given.")`), and
`isCancellationError` only ever matches strings (`errorUtils.ts:77` returns false
for anything that is not one). So the early return fired first and the exact case
the comment described - a nested resolver that prompts and is cancelled - would
have surfaced as "Preview unavailable" under the field.
Check the raw value first. An Error whose message is a cancellation string is
still covered on the way through.
Caught by CodeRabbit on #1560. Test verified to fail without the fix.
`parseAnonymousValueOptions` accepted a `quiet` flag and honoured it for the
`|case:` warning, but then called `resolveInputType` without it and
`resolveNumericInput` with a hardcoded `false`. So the deliberately-silent
prompt-context pre-pass in `Formatter.getValuePromptContext` warned anyway, and
every anonymous `{{VALUE|type:...}}` / `|min:` / `|max:` / `|step:` / slider typo
produced two identical Notices per parse instead of one.
Fix the class of bug, not just the instance: replace the optional `quiet:
boolean` with a `WarnSink` - a `(message: string) => void` the caller supplies -
and make it a REQUIRED parameter on `parseOptions`, `resolveInputType`,
`buildNumericConfig` and `resolveNumericInput`. Forgetting to forward it is now
a compile error rather than a silent duplicate Notice. A sink also expresses
the three audiences a boolean cannot: loud (the real run), silent (pre-passes)
and, next commit, collecting (the builders' live preview).
`FieldSuggestionParser.parse` gains the same sink. `warnUnknown` stays as-is: it
gates WHICH grammar may warn, the sink decides who hears it.
Drive-by, on a line already being edited: the empty `|name:` warning was the one
message here that never named its token, at run time as well as in preview.
Refs #1558
Every authoring warning the token parsers produce went straight to
`log.logWarning`, i.e. straight to an Obsidian Notice, no matter who was
parsing. `Formatter` now owns two overridable reporting hooks - `warn` and
`reportProblem` - and hands the parsers a pre-bound `warnSink`. `CompleteFormatter`
overrides neither, so the run-time contract is unchanged and stays pinned by
formatter.audit-formatter-core.test.ts. The next commits give the preview and
the preflight scan the overrides they should always have had.
The sink is deliberately a pre-bound property rather than the `warn` method
itself: the parsers call it detached, so `{ warn: this.warn }` would run a
collecting override with `this === undefined`. Neither tsc (strict off) nor
eslint (no type-aware config, so `unbound-method` is unavailable) would catch it,
and the resulting TypeError is swallowed by `format()`'s catch.
Also removes the bare `console.warn` beside the named-value conflict warning. Its
comment claimed it was "retained for the diagnostic log/trail", but
`ConsoleErrorLogger.logWarning` already console.warns every message
(src/logger/consoleErrorLogger.ts:49), and it bypassed the hook - so once the
preview stops firing Notices it would still have written one devtools line per
keystroke. The three tests that asserted on it now assert on `log.logWarning`,
which is the channel that actually reaches the user.
Refs #1558
The choice builders re-run the whole format pipeline on every keystroke to draw
the "Preview:" row, using FormatDisplayFormatter / FileNameDisplayFormatter.
Those override every RESOLUTION method so nothing prompts, but they inherited
the base Formatter's parsing verbatim - warnings included - and the autocomplete
inserts `{{VALUE:}}` with the closing braces already there, so every prefix of an
argument you type is a complete, INVALID token. Typing `pascal` into
`{{VALUE:title|case:}}` stacked five Obsidian Notices across half the window.
A preview is a speculative evaluation of incomplete input; it must not have the
run's side effects. Both display formatters now override the reporting hooks to
collect into a `PreviewDiagnostics` list, replaced at the start of every
`format()`. The real run is unchanged and still owns the user-facing Notice.
Two things fall out of having a channel at all:
- Thrown failures join it. Both formatters wrapped the pipeline in
`catch { return input; }`, so `{{VALUE:a,b|text:x}}` - and the half-typed
`{{VALUE:}}` the autocomplete leaves behind - silently echoed the raw text
back, which is exactly the state a confused author is already in. The catch is
untyped, so only QuickAdd-authored messages pass through; anything else
degrades to a generic line, and a cancelled prompt is dropped.
- Messages lose their redundant `QuickAdd: ` prefix on collection. GuiLogger
already prepends `QuickAdd: (Warning) `, so the Notice read
"QuickAdd: (WARNING) QuickAdd: Unsupported ..." and inline under a QuickAdd
field the brand is pure noise.
Tests re-run every row of the measurement in the issue against the real
formatters and assert zero Notices, plus the collected content - a collecting
override that silently failed would otherwise read as success.
Refs #1558
… reports
RequirementCollector is a scan, not a run: it documents itself as never
executing anything and returning inert replacements. But it extends Formatter,
so it emitted every authoring warning itself - twice per token, because it
parses each one in both a pre-pass and a main pass - and the run that followed
emitted the same message a third time.
Verified live on Obsidian 1.13.0: with one-page inputs enabled, one
`{{VALUE:title|case:pasc}}` typo showed two identical Notices before the input
form even opened, then another on submit.
The run owns the warning. Overriding both hooks covers the inherited paths; the
direct `parseValueToken` call in `scanVariableTokens` needs the sink passed
explicitly, since hook overrides do not reach a module-level call.
The preflight-only diagnostics in collectChoiceRequirements and
runOnePagePreflight are untouched and stay loud - those report things only the
scan can know.
Refs #1558
The worst thing found while fixing #1558, and not in the issue. `FormatDisplayFormatter.getTemplateContent` built a `SingleTemplateEngine`, whose base `TemplateEngine` constructs a real `CompleteFormatter`. So the builder's live preview ran the RUN-TIME formatter over the included template, on every keystroke. Verified live in an isolated Obsidian 1.13.0 vault: typing `{{TEMPLATE:PreviewProbe.md}}` into the Capture format field, with that template containing `{{VALUE:whoAreYou}}`, popped a real blocking "whoAreYou / Ok / Cancel" modal on top of the settings window. A `|case:` typo inside the template fired the run's warning Notice. `{{MACRO:...}}` reached the macro engine, where it threw because the preview passes no choice executor - and the throw was swallowed into "Template (not found): <path>", which was simply untrue. The preview now reads the template file and resolves it through ITSELF, so an included body previews exactly like the field it is embedded in: no prompts, no macro engine, no inline JS, and no engine import in this file at all. Two details worth calling out: - `format()` splits into a `formatInternal(input, { expandLinebreakEscapes })`. An included body must NOT have `\n` escapes expanded - the runtime treats them as format-template material and never applies them to substituted content (#527), so recursing through the public `format()` would corrupt a template containing `\nabla` or `C:\Users\nadia`. - The inclusion depth counter is incremented HERE, not in the shared `Formatter.replaceTemplateInString`. At run time `CompleteFormatter.getTemplateContent` hands its child engine a copy of the state with `depth + 1`; counting in the shared method too would advance the runtime by two per level and halve its inclusion limit. Preview and runtime both still cap at 10 nested levels. Cycle and depth accounting now spans the preview level itself, and one budget is shared across a field instead of restarting per top-level `{{TEMPLATE:}}` token - previously the preview handed the engine no inclusion state, so the first nested level always started over. (Nothing was unbounded before: inside the engine subtree `visited` was shared by reference, so a self-including template still terminated with a cycle report.) The "not found" / "failed" placeholders are now distinct, so the preview stops blaming a missing file for a problem that was something else. Refs #1558
The one-page input form's live preview computes `out.fileName`, but built a
`FormatDisplayFormatter` - the note-CONTENT formatter. So it expanded `\n`
escapes into real linebreaks (not a thing in a path) and resolved
`{{LINKCURRENT}}` / `{{LINKSECTION}}`, both of which the run-time
`formatFileName` deliberately leaves alone.
`FileNameDisplayFormatter` is the class that mirrors the runtime file-name path,
and the one the builder's own file-name preview already uses.
One deliberate divergence, unchanged by this commit and shared with the builder's
file-name field: `formatFileName` DOES resolve `{{TEMPLATE:}}` at run time
(`format()` -> `replaceTemplateInString`, with path prompt scope propagated), and
neither file-name preview does. Splicing a multi-line template body into a name
preview is worse than showing the token, and the only inert reader available here
returns a fabricated stub. Filed as a follow-up.
Found while auditing every site that constructs a display formatter for #1558 -
this is the only one that runs inside a real execution rather than in a builder.
Refs #1558
…ter per pass Completes #1558. Silencing the preview's Notices would have left the author with no feedback at all until they ran the choice, so the problems now land where the eye already is: one muted line under the preview row, coloured by severity, no glyph (matching .qa-folder-mode-warning), clamped with the full text in `title` because the longest of these runs past 250 characters. Two behaviours make that calm rather than a quieter kind of noise: - They are held back until the field has been still for 500ms. Every one of these messages quotes the partial token it is about, so shown live they would rewrite themselves on every keystroke. The preview TEXT stays live and un-debounced, as before. Lint-on-idle is the editor convention. - When nothing resolved, the row says "Unresolved:" instead of "Preview:". The value shown is the raw input, and labelling that "Preview" asserts it is what you will get. The formatter is now built per pass instead of memoized for the field's lifetime. That is a bug fix, not hygiene: the instance carries a `variables` map whose resolved keys short-circuit the next resolve, so editing the option list of `{{VALUE:alpha,beta|name:t}}` to `{{VALUE:gamma,delta|name:t}}` kept previewing "alpha" and tripped the "conflicting definitions" warn-once guard - which, before this PR, fired an on-screen Notice from a keystroke. It also scopes each pass's diagnostics, which an async pipeline sharing one instance could otherwise interleave. Text and diagnostics commit under the same staleness token, so the row can never show one pass's preview beside another's complaint. Verified live in an isolated Obsidian 1.13.0 vault: typing `pascal` into `{{VALUE:title|case:}}` now produces 0 Notices (was 5) and 0 inline complaints while typing, with the single explanation appearing once you stop. Closes #1558
A heredoc in the authoring pass wrote a literal NUL into a template literal, so git classified the file as binary and would have shown no diff for it in review.
Found by an adversarial review of the diff. No behaviour changes beyond two
small ones noted below; the point is that in a codebase that uses comments as
its design record, an inverted invariant is the expensive kind of wrong - it
survives into commit messages, test names and PR bodies, as all three did here.
- `runOnePagePreflight` claimed `formatFileName` "never includes templates". It
does: `format()` runs `replaceTemplateInString`, path prompt scope is
deliberately propagated into the child engine, and
`completeFormatter.promptContext.test.ts` pins it. The real reasons for the
swap are `\n` expansion and the link tokens, which run-time `formatFileName`
leaves literal - the second of which the old comment did not mention. The
`{{TEMPLATE:}}` behaviour is now documented as a deliberate divergence, shared
with the builder's own file-name field, with a follow-up filed.
- `formatDisplayFormatter` claimed the old preview "recursed unbounded". It did
not: only the preview -> engine boundary restarted, and inside the engine
subtree `visited` is shared by reference, so a self-including template still
terminated with a cycle report. What the preview-local depth counter actually
buys is accounting that spans the preview level itself, on one budget per
field. The load-bearing half of that comment - why the counter must NOT live
in the shared `replaceTemplateInString` - is unchanged and still correct.
- `describePreviewFailure` claimed "only QuickAdd-authored messages are passed
through". There is no allowlist, deliberately: most reachable throws are
authored but unbranded, and a genuine plugin bug's message is what makes a
report actionable. The comment now says that instead.
Two behaviour fixes in the same pass:
- The template cycle/depth reports arrive wrapped as `[QuickAdd: ... ]`, because
the identical string is also spliced into the output as a placeholder. The
brand strip did not match through the bracket, so the preview showed the same
sentence twice, twenty pixels apart. Unwrap the bracket first.
- `FileNameDisplayFormatter.getTemplateContent` returned a fabricated
`[<name> template content...]`. It is unreachable (the class never calls
`replaceTemplateInString`) but abstract on `Formatter`, so it has to exist -
and fabricated text that reads like part of a file name is the worst thing for
it to return if the pass is ever wired up. It now names what happened.
chhoumann
force-pushed
the
chhoumann/1558-preview-notices
branch
from
July 26, 2026 22:16
f9f28b4 to
54bee88
Compare
chhoumann
added a commit
that referenced
this pull request
Jul 26, 2026
…d of reporting it
`describePreviewFailure` gated on `instanceof Error` before consulting
`isCancellationError`. But QuickAdd's modals reject with a bare STRING
(`GenericInputPrompt.ts:293` -> `rejectPromise("No input given.")`), and
`isCancellationError` only ever matches strings (`errorUtils.ts:77` returns false
for anything that is not one). So the early return fired first and the exact case
the comment described - a nested resolver that prompts and is cancelled - would
have surfaced as "Preview unavailable" under the field.
Check the raw value first. An Error whose message is a cancellation string is
still covered on the way through.
Caught by CodeRabbit on #1560. Test verified to fail without the fix.
…d of reporting it
`describePreviewFailure` gated on `instanceof Error` before consulting
`isCancellationError`. But QuickAdd's modals reject with a bare STRING
(`GenericInputPrompt.ts:293` -> `rejectPromise("No input given.")`), and
`isCancellationError` only ever matches strings (`errorUtils.ts:77` returns false
for anything that is not one). So the early return fired first and the exact case
the comment described - a nested resolver that prompts and is cancelled - would
have surfaced as "Preview unavailable" under the field.
Check the raw value first. An Error whose message is a cancellation string is
still covered on the way through.
Caught by CodeRabbit on #1560. Test verified to fail without the fix.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/gui/ChoiceBuilder/components/FormatPreviewField.svelte (1)
70-93: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear diagnostics before starting every non-empty preview pass.
diagnosticsis reset only when the field is emptied (Line 73). If formatting takes longer than the 500 ms idle timer, the previous pass’s warning/error can be shown under the new input. The monotonic token rejects late writes but does not clear already committed diagnostics.Suggested fix
if (!current.trim()) { previewToken++; preview = ""; diagnostics = []; return; } + const token = ++previewToken; + diagnostics = []; const formatter = formatterKind === "fileName" ? new FileNameDisplayFormatter(app, plugin) : new FormatDisplayFormatter(app, plugin, undefined, { resolveActiveFolder: formatterKind !== "lineTarget", }); - const token = ++previewToken;Also applies to: 120-127
🤖 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/gui/ChoiceBuilder/components/FormatPreviewField.svelte` around lines 70 - 93, In the non-empty preview path of the field’s formatting handler, clear diagnostics at the start of every new pass before constructing the formatter and scheduling work. Keep the existing empty-input reset and previewToken stale-result protection unchanged, and update the corresponding repeated logic in the additional applicable section.
🤖 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/FormatPreviewField.svelte`:
- Around line 138-146: Update the diagnostic rendering in the diagnostics each
block to expose each diagnostic’s severity as visible text or an accessible
label, using “Warning” and “Error” based on diagnostic.severity. Keep the
existing severity color classes and diagnostic.message display so color remains
secondary information.
---
Outside diff comments:
In `@src/gui/ChoiceBuilder/components/FormatPreviewField.svelte`:
- Around line 70-93: In the non-empty preview path of the field’s formatting
handler, clear diagnostics at the start of every new pass before constructing
the formatter and scheduling work. Keep the existing empty-input reset and
previewToken stale-result protection unchanged, and update the corresponding
repeated logic in the additional applicable section.
🪄 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 Plus
Run ID: 08e7c447-7d7a-4ad5-a2e4-f041bbdc08db
📒 Files selected for processing (20)
src/formatters/displayFormatter-1558-inert.test.tssrc/formatters/fileNameDisplayFormatter.tssrc/formatters/formatDisplayFormatter-1558-template.test.tssrc/formatters/formatDisplayFormatter.tssrc/formatters/formatter-named-suggester.test.tssrc/formatters/formatter.tssrc/formatters/previewDiagnostics.tssrc/gui/ChoiceBuilder/components/FormatPreviewField.sveltesrc/gui/ChoiceBuilder/components/FormatPreviewField.test.tssrc/preflight/RequirementCollector.silence.test.tssrc/preflight/RequirementCollector.tssrc/preflight/runOnePagePreflight.filenamePreview.test.tssrc/preflight/runOnePagePreflight.tssrc/styles.csssrc/utils/FieldSuggestionParser.tssrc/utils/valueSyntax.audit-formatter-core.test.tssrc/utils/valueSyntax.test.tssrc/utils/valueSyntax.tssrc/utils/valueSyntax.warnSink.test.tssrc/utils/warnSink.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- src/utils/warnSink.ts
- src/formatters/formatDisplayFormatter-1558-template.test.ts
- src/preflight/runOnePagePreflight.filenamePreview.test.ts
- src/utils/valueSyntax.audit-formatter-core.test.ts
- src/formatters/fileNameDisplayFormatter.ts
- src/gui/ChoiceBuilder/components/FormatPreviewField.test.ts
- src/preflight/RequirementCollector.ts
- src/utils/valueSyntax.warnSink.test.ts
- src/utils/FieldSuggestionParser.ts
- src/utils/valueSyntax.test.ts
- src/formatters/previewDiagnostics.ts
- src/preflight/runOnePagePreflight.ts
- src/formatters/formatter-named-suggester.test.ts
- src/formatters/displayFormatter-1558-inert.test.ts
- src/preflight/RequirementCollector.silence.test.ts
- src/formatters/formatter.ts
- src/utils/valueSyntax.ts
- src/formatters/formatDisplayFormatter.ts
chhoumann
added a commit
that referenced
this pull request
Jul 26, 2026
The inline problems distinguished a warning from an error only by text colour (`--text-warning` vs `--text-error`), which is WCAG 1.4.1. A visually-hidden "Warning: " / "Error: " rather than a printed prefix: these messages already clamp at three lines, and the distinction a sighted user actually needs - did this format resolve at all - is carried by the row label, which reads "Unresolved:" instead of "Preview:" whenever an error is present. So the prefix would cost real estate to restate something already in text. Caught by CodeRabbit on #1560.
The inline problems distinguished a warning from an error only by text colour (`--text-warning` vs `--text-error`), which is WCAG 1.4.1. A visually-hidden "Warning: " / "Error: " rather than a printed prefix: these messages already clamp at three lines, and the distinction a sighted user actually needs - did this format resolve at all - is carried by the row label, which reads "Unresolved:" instead of "Preview:" whenever an error is present. So a visible prefix would cost real estate to restate something already in text. Caught by CodeRabbit on #1560.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/styles.css`:
- Around line 228-229: Remove the deprecated clip declaration from the CSS rule
containing clip-path: inset(50%), leaving the clip-path declaration as the
primary clipping behavior. Do not add CSS lint tooling; only introduce an
`@supports` fallback or compatibility note if legacy WebView support is explicitly
required.
🪄 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 Plus
Run ID: 8a5861b7-f125-4136-99ac-b26379b7548f
📒 Files selected for processing (3)
src/gui/ChoiceBuilder/components/FormatPreviewField.sveltesrc/gui/ChoiceBuilder/components/FormatPreviewField.test.tssrc/styles.css
🚧 Files skipped from review as they are similar to previous changes (2)
- src/gui/ChoiceBuilder/components/FormatPreviewField.test.ts
- src/gui/ChoiceBuilder/components/FormatPreviewField.svelte
…idden `clip: rect(0 0 0 0)` is the legacy half of the visually-hidden idiom and is deprecated; `clip-path: inset(50%)` alone does the clipping. `clip-path` has been supported since Chrome 55 and the plugin's minAppVersion is 1.13.0, so there is no Electron QuickAdd runs on that needs the fallback. Verified in the live vault after the change: the element is still 1x1, still absolutely positioned, computed `clip-path: inset(50%)`, and its "Warning: " text is present in the DOM without being visible. Caught by CodeRabbit's stylelint pass on #1560.
This was referenced Jul 27, 2026
chhoumann
added a commit
that referenced
this pull request
Jul 27, 2026
…1581) * fix(ai): stop previewing a substitution the system prompt never gets #1565 asked for four modals to be brought under the inert-preview mechanism #1560 built for the choice builders. Three of the four should not have a format preview at all: the AI system prompt is sent to the model VERBATIM. Only the prompt (or prompt template) goes through the formatter. Every path agrees: - `runAIAssistant` formats `targetPrompt` (AIAssistant.ts:295) and passes `systemPrompt` raw to `OpenAIRequest` (:308). - `Prompt` (:408/:418) and `ChunkedPrompt` (:936) do the same - and the chunked path sizes its budget with `estimateTokenCount(systemPrompt)` on the raw string (:897), which is the same admission. - `MacroChoiceEngine.executeAIAssistant` hands `command.systemPrompt` straight through (:668) while its formatter callback wraps only the prompt (:675). - `Agent.buildSeedMessages` pushes `system` raw and formats `prompt`. - `OpenAIRequest` then puts it on the wire unchanged (:198/:246/:312). The docs agree too: AIAssistant.md:56 says the prompt template "is a Markdown note in the prompt template folder, not raw prompt text", and nothing anywhere claims system prompts take tokens. Only the UI disagreed. All three system-prompt fields ran a live FormatDisplayFormatter and attached a `{{` token autocomplete, so `{{DATE}}` previewed as 2026-07-27 and then reached the model as eight literal characters. That is a worse defect than the one #1565 reports: the preview did not merely fail to warn, it asserted a substitution that will not happen. Mounting the GOOD preview implementation there would have made the lie better-typeset, inline-diagnosed and announced to screen readers. So the affordance goes instead. Removing it also closes #1568 outright: with the shipped token-free default prompt, that preview was a character-for-character duplicate of the textarea above it, at full body size, and read as the modal printing the prompt twice. There is nothing to label if there is nothing to show. What replaces it, for the authors who already have a token in there and would otherwise get no signal at all: one muted line under the field, shown only when the value contains `{{`, naming what happens and pointing at the prompt template - the thing that IS formatted. Muted rather than warning-coloured, matching `.qa-ai-token-note` directly above it in the same modals: like that line it states how the field behaves, and a `{{` a user wants the model to read literally is a legitimate thing to type here. Whether system prompts SHOULD be formattable is a real question, and a bigger one than these two issues - silent behaviour change for existing vaults, a prompting `{{VALUE:}}` inside an agent loop, and a chunk budget computed before the substitution. Filed as #1572. If it lands, the preview and the token autocomplete come back with it and this note is deleted. AIAssistant.systemPromptLiteral.test.ts pins the invariant in both directions - the system prompt arrives byte-identical AND the formatter is never called with it - so the person who changes the runtime is told to revisit the UI. Verified to fail against a one-line mutation that formats it. Drive-by: `addSlider` moves into the shared obsidian test stub, replacing the local shim AIAssistantInfiniteCommandSettingsModal.test.ts carried for it. Refs #1565 Closes #1568 * fix(user-script): give the format option the choice builders' preview The one field of the four #1565 names that really is a format field. A `type: "format"` option is the script author's own declaration, and the documented pattern is that the script resolves it itself with `quickAddApi.format()` (docs/public/scripts/EzImport.js:304) - so unlike the AI system prompt, previewing it is telling the truth. It just wasn't telling it well. `addFormatInput` built its preview span BEFORE the textarea, so it rendered above the field and read as its label - the exact placement #1543 fixed in the builders. It memoized one FormatDisplayFormatter for the modal's lifetime, whose resolved `variables` map short-circuits the next pass. It committed `await format(value)` with no staleness token, so a slow pass could overwrite a newer one. And it threw the collected diagnostics away, so after #1560 silenced the Notices a malformed token reported nothing at all: verified in an isolated Obsidian 1.13.0 vault before this change, typing `{{VALUE:title|case:pasc}}` produced 0 Notices and 0 inline complaints while the preview rendered "Example Title" as if all were well. Rather than reimplement labelling, placement, per-pass construction, staleness and diagnostics imperatively - ~80 lines of the drift that produced this issue - the Svelte FormatPreviewField is mounted. `mountFormatPreview` is a thin seam over the existing `mountComponent`, with a `$state`-backed props object (the documented way to feed reactive props to an imperatively mounted Svelte 5 component, mirroring `createCommandListProps`). It creates and owns its host element: `mount()` writes anchor comment nodes into whatever it is given, and this modal `createEl`s straight onto `contentEl`, so a shared target would interleave the anchors with later `new Setting(...)` rows. Lifecycle is the trap here. `display()` is re-entrant - the constructor calls it, and `migrateSecretSettings()` calls it again from a `void`ed promise that can land at any time - and it empties `contentEl`. Emptying does not stop a Svelte component; it just detaches it, so each re-render would orphan one per format option, each holding a live `$effect` and a 500ms timer. `destroyPreviews()` therefore runs as the first statement of `display()`, before `empty()`, and from a new `onClose()` (the class had none). The regression test for that asserts on the tracked handle list, not on the DOM: an orphan is detached along with contentEl's children, so it vanishes from every query while still running. The first version of this test asserted on `.qa-preview-row` and passed against a build with the teardown removed. Verified in an isolated Obsidian 1.13.0 vault: 0 Notices, "Preview: Logged on 2026-07-27 for Example Title" below the field, and the previously-silent warning inline underneath naming the eight supported |case styles. Opening and closing the modal three times leaves 0 preview rows behind. Closes #1565 * fix(user-script): open the settings modal when a format option has no default Follow-ups from an adversarial review of the two commits above. The first is a real regression the branch introduced; the rest harden what it shipped. **A format option with no `defaultValue` crashed the modal.** `initializeUserScriptSettings` deliberately skips options whose `defaultValue` is undefined, so `addFormatInput`'s `value: string` arrives as `undefined` from a third-party script - and `FormatPreviewField` calls `value.trim()` during mount. The throw propagates out of `display()` and out of the constructor, so `new UserScriptSettingsModal(...).open()` in CommandList never reaches `.open()` and the gear button silently does nothing. Same for a non-string stored value, e.g. a script that changed an option from `toggle` to `format` between versions. The old code only degraded (a textarea reading the literal text "undefined"), so this is new. Coerced at the boundary where untrusted script data enters typed code, which fixes the "undefined" wart too. Both cases pinned, both verified to fail without the coercion, and verified live: the modal now opens with an empty field and no dangling preview row. **`{{FOLDER}}` previewed a folder the runtime never produces.** The builders fall back to a "Folder/Name" placeholder because a choice HAS a configured target folder that no caller wires in yet. A user-script option has none at all, and `Formatter.replaceMacrosInString` resolves `{{FOLDER}}` to `this.targetFolderPath ?? ""` there. `targetFolderPath` now accepts an explicit `null` meaning "this host has no folder concept", distinct from `undefined` meaning "unknown", and `mountFormatPreview` passes it. The builders pass neither, so their behaviour is byte-identical. Verified live: `{{FOLDER}}/{{DATE}}` previewed `Folder/Name/2026-07-27`, now previews `/2026-07-27`. **The contract test covered two of the four paths its own comment cites.** Every existing test that names `runAIAssistant` mocks it away, so the macro AI command's runtime - the path behind the modal this branch strips - had no assertion on its `systemPrompt` argument anywhere in the suite. `runAIAssistant` and `Agent.buildSeedMessages` are now driven for real. All four cases verified to fail against a one-line mutation that formats the system prompt in that path. **A11y.** The note is now referenced by `aria-describedby` while it is shown (and only while, since an accessible description may include a directly-referenced hidden node). The four textareas that are appended to `contentEl` rather than their Setting's `controlEl` - so nothing associates them with the name above - get an explicit `aria-label`. **Copy.** "Use the prompt template for text that needs tokens" pointed at a control the global AI settings modal does not show; it now says which input IS formatted. The same sentence is added to the AI Assistant docs, so the answer is findable before you type the token rather than only after. **Polish.** The preview host takes a class and a negative top margin: the textareas carry `margin-bottom: 1em` for the gap to the NEXT setting, which left the preview row floating midway between the field it describes and the one below. Also: coverage for the AI modals' `reload()` path (the note must be rebuilt with its field, not hoisted), for `mountFormatPreview`'s destroy-then-setValue path, and a dead `beforeAll` removed - it justified itself with a false claim about the shared obsidian stub, which does define `onClose`. Refs #1565, #1568 * fix(user-script): show a non-string format value instead of blanking it Caught by Codex on #1581. The previous commit coerced a non-string stored value to "" so `FormatPreviewField`'s `value.trim()` could not throw out of the constructor. But `resolveUserScriptSettings` still forwards the original to the script, so a `toggle`-to-`format` type change across script versions left the field claiming the setting was empty while execution received `true`. Absent stays absent - `initializeUserScriptSettings` deliberately leaves an option with no `defaultValue` unset so the script can apply its own, and printing "undefined" would be its own lie. Anything present is stringified, which also makes this method consistent with its four siblings: they all render `value as string` unchanged. Persisting the normalization instead was rejected: for the absent case it would overwrite the deliberate undefined, and for the present case it means a disk write from a render pass. Refs #1565
chhoumann
added a commit
that referenced
this pull request
Jul 27, 2026
…warning readable (#1582) * fix(field): diagnose the mistyped FIELD filter instead of reciting the list The unknown-filter warning answered every typo by printing all thirteen supported keys: 256 characters, of which the last 180 were a list you had to read to the end to discover the answer was `folder` all along. As an Obsidian Notice it filled the corner of the screen; as the inline preview diagnostic added in #1560 it is the one message that needs a three-line clamp to fit. It now names the mistake: Unknown FIELD filter "fodler" - did you mean "folder"? Ignored in "{{FIELD:status|fodler:abc}}". The actionable clause comes before the echoed token so the clamp can never cut the fix off, and both user-controlled parts are length-bounded so an absurd key or a long token cannot re-inflate the message. The full vocabulary survives in the one case where it is still the most useful thing to say: when nothing is close enough to name. Three matching rules, in order - one edit away (Damerau, so `mutli` -> `multi` costs 1), then a shared prefix (`exclude` -> the three `exclude-*`), then two edits but only into a long key. That last bound is what stops `filter` being answered with `folder`, and `case-insensitive` is answered with a sentence rather than a guess, because obeying "did you mean case-sensitive?" would silently invert the request. Two supporting fixes in the same parser: - FIELD_FILTER_KEYS is now authoritative. Membership is tested before the switch dispatches, so a case added without an entry stops working instead of silently drifting away from what the warning advertises. - A pipe part with no colon used to be dropped in total silence, so `{{FIELD:status|mutli}}` quietly downgraded a multi-select prompt to a single-select one with nothing anywhere saying why. It warns now. Closes #1564 * fix(preview): put the file-name preview through the run's own normalizer Every generated name goes through `normalizeGeneratedFilePath` on the way to the vault - TemplateChoiceEngine, TemplateInsertEngine, templateNoteDiscovery, and the capture target via captureTargetResolution all call it - and the preview did not, so it asserted names the run would never create. `Meeting notes.` previewed with the dot the run strips; `Notes\2026\Log` previewed as one segment though the run writes it as a path (Obsidian's own normalizePath rewrites the backslash); a `.` or `..` segment previewed as an ordinary name though the run aborts the choice on it. It is also the answer to what a multi-line `{{TEMPLATE:}}` body should look like in a name preview (#1563): the normalizer already collapses a run of line breaks, with the spaces around it, into a single space. Doing the collapse here rather than inside the include means preview and run splice first and normalize second, in the same order, so the two agree byte for byte instead of differing by a space at the seam. The normalizer is refactored into a core that collects its rejections rather than throwing on sight. `normalizeGeneratedFilePath` keeps its exact behaviour by throwing the first one; the new `previewGeneratedFilePath` returns them, because a preview evaluates incomplete input on every keystroke and must not throw its way out of a keystroke (#1558). The rejections become error diagnostics, so a name the run would refuse now says so instead of looking fine. * fix(preview): resolve {{TEMPLATE:}} in the file-name preview `CompleteFormatter.formatFileName` resolves `{{TEMPLATE:...}}` - `format()` runs `replaceTemplateInString`, and path prompt scope is deliberately propagated into the child engine - but the file-name preview never called it. So the builder's "File name format" field said `{{TEMPLATE:Naming.md}}-My Note` while the run created `2026-07-27 My Note.md`. The one-page input form contradicted itself harder still: the preflight scan behind that modal already walks INTO the include for this same field, so it prompted for a variable it could only have found inside the template, then previewed the unresolved token beside the answer. The include is read through THIS formatter, which is what keeps the preview inert (#1558): the same substitutions the top level gets, no prompts, no macro engine, no inline JS. Building a SingleTemplateEngine here would construct a real CompleteFormatter and open blocking modals on every keystroke - the bug #1560 fixed on the content field. Four things this needed beyond the reader itself: - The template pass runs FIRST, where the run has it (before globals and before {{VALUE}}). Resolving it later would have made the preview splice a body in for a token that a global snippet produced, which the run leaves literal - trading one lie for another. - That ordering makes a global snippet holding a {{TEMPLATE:}} token re-arm the include loop with an unwound cycle set, so a per-pass include budget bounds it. It also caps the fan-out of a wide include tree on a field that resolves on every keystroke. - An included body resolves with CONTENT token semantics, because at run time it goes through the child engine's `formatFileContent`. Previewing it with the file-name pass list would leave literal exactly the tokens people put in templates ({{linkcurrent}}, {{linksection}}). - A missing template is an error diagnostic, not a quiet placeholder: the run throws there and the choice dies, so the row must read "Unresolved:" rather than presenting a name. Fixed in the content preview's reader too, where not-found was the odd branch out - cycle and max-depth already reported. A multi-line body is joined into one line by the normalizer, which is what the run does; the preview says so once, rather than presenting someone's whole note template as a run-on name with no explanation. It stays quiet about the trailing newline every well-formed one-line naming template has. Closes #1563 * fix(builder): clamp the file-name preview so an include cannot unfold it .qa-preview-row carried overflow-wrap and no clamp, which was fine while the row could only hold what you typed. Since #1563 it can hold a whole included template: a 40-line note previews as 468px of muted text under a single-line input, re-flowing the builder on every keystroke. Two lines now, with the full string in `title`, mirroring .qa-preview-issue - which was clamped for exactly this reason. The content field is left alone: a multi-line preview is the point there. Clamped on the row rather than the value span, because -webkit-box is block-level and would drop the value onto its own line under "Preview:". Also drops the quotes around the echoed {{FIELD:...}} token in the unknown-filter warning: the braces already delimit it, and in the builder the pair wrapped across the line break, leaving a dangling `"` at the end of the first line. * fix: three defects found reviewing the #1563/#1564 change All three are mine, from the commits above. 1. The FIELD hint table was an object literal indexed by whatever follows a pipe, so {{FIELD:x|constructor:y}} answered with a member of Object.prototype: "Unknown FIELD filter "constructor" - function Object() { [native code] }...". It is a Map now, which has no prototype chain to walk into. Reachable from both the inline preview and the run's Notice. 2. The new colon-less warning called a correctly spelled filter unknown. {{FIELD:status|folder}} - one keystroke short of `|folder:`, which a live preview sees constantly - produced "Unknown FIELD filter "folder" ... Supported filters: folder, tag, ...": it contradicted itself AND reprinted the 256-character dump #1564 exists to delete. Others got a confidently wrong redirect ("|inline" -> did you mean "inline-code-blocks"?), because the suggester excludes the exact match from its own pool. A recognised key without a value is a different mistake and now gets its own sentence: `FIELD filter "folder" needs a value - write "folder:value".` 3. The one-page input form's preview row was left unclamped. Since the commit above, a naming template's include resolves there too, so a 39-line template rendered 371px of joined text into a block that sits ABOVE every field and re-renders on each keystroke - pushing the inputs and Submit down and shifting them under the caret. Clamped to two lines with the full string in `title`, same rule as the builder row. Measured live: 371px -> 39px. * fix(preview): skip inline script fences when expanding includes Review catch. A fence is verbatim JavaScript source and the run consumes it BEFORE its template pass (replaceInlineJavascriptInString is the run's first pass), so a "{{TEMPLATE:N.md}}" written as a string literal inside a script is never an include at run time. The preview has no inline-JS pass at all - by design, it must not execute anything - so it read that path, spliced the body into the middle of the displayed source, and reported a "Template not found" ERROR for a format that is fine. Same helper and the same reason as expandLinebreakEscapesOutsideTokens (#1467), which protects fences from `\n` expansion two functions away.
chhoumann
added a commit
that referenced
this pull request
Jul 27, 2026
…#1595) * test: pin the file-name preview against the real formatter (#1580) fileNameDisplayFormatter.test.ts defined its own TestFileNameDisplayFormatter - a dozen hand-written regex replaces - and asserted that those regexes did what they said. Eleven green tests that never imported FileNameDisplayFormatter and could not fail for any change to it. They had also drifted into asserting behaviour the plugin cannot produce: {{TEMPLATE:daily-note}} was pinned to a fabricated '[daily-note template content...]' placeholder that #1560 deleted and #1563 replaced with a real inert read. Replaced with one case per token against the real class. Two of them pin CURRENT behaviour that the mock claimed otherwise for, each with the issue it is filed as: {{MATH:}} is left literal by the preview while the run prompts for it (#1587), and {{title}} previews as a name although formatFileName throws on it (#1588). * fix: the {{FIELD:}} preview names the field, not the filter syntax (#1579) Both preview formatters built the placeholder out of the token's whole inner text, filters included, so the more precisely you filtered the less the preview looked like a value: {{FIELD:status|folder:Work}} previewed 'status|folder:Work_field_value'. The field is status; at run time the token resolves to one of that property's values. replaceFieldVarInString already parsed the specifier one line above the call, so the parsed field name is handed to suggestForField. The variable KEY stays keyed on the whole specifier - two {{FIELD:status}} tokens with different filters are different prompts. Also fixes the fallback beside it: getVariableValue was called with the bare specifier instead of the FIELD-prefixed key, so on the one path where a suggester resolves undefined (a remote prompt provider can) it both missed the value that had been stored and cross-read the {{VALUE}} namespace - a {{VALUE:status}} answer could be served to a {{FIELD:status}} token. * fix: the file-name preview says when Obsidian will refuse the name (#1578) 'Bad: {{VALUE:title}}' previewed 'Bad: Example Title' in the ordinary 'Preview:' styling, and running the choice created nothing - the Notice was Obsidian's own: 'File name cannot contain any of the following characters: \\ / :'. #1563 made this row mirror the run's name normalizer; a character Obsidian refuses is the same class of truth and the last one missing. Measured against vault.create/createFolder on Obsidian 1.13.0 (macOS), one candidate character per name: ':' throws Obsidian's own guard for files AND folder segments; '* ? " < > | ^ [ ] #' and tab all create successfully, so copying the stricter set from TemplateEngine.validateFolderSegment would reject names Obsidian makes without complaint; '\\' and '/' are separators and QuickAdd creates the parent folder. The rule is ':' and only ':'. The check reads the FINISHED name rather than the format string. That is the only place all the sources meet: a colon the author typed, one {{TIME}} produced (it is HH:mm, and the token autocomplete offers it in this field), one a global snippet or an included template body carried in, and one left behind by a token that never matched - {{TEMPLATE:Naming}} without the extension is not a token, so the literal text goes to the vault, and a mask over {{...}}-shaped spans would be blind to exactly that, the most likely {{TEMPLATE:}} typo. Reading the finished name only works if the preview stops writing text that is not part of the name, so two stand-ins were corrected first: - the VDATE '(default: X)' / '(optional)' hints are gone from the file-name preview. The run splices in the formatted date and nothing else, so '2026-07-27 (default: tomorrow)' was already a name that could not exist - and the colon in it would have been blamed on the author. The hints stay on the body preview and in the run's own prompt placeholder. - an inline {{VALUE:a,b}} option list previews the option, without the body preview's ' (N options)' count. Three guards keep it from crying wolf: a pass that already reported an error says nothing more (all four [QuickAdd: ...] placeholders carry a colon and each already named its real problem); an unterminated '{{' means the author is mid-token with the format suggester open; and inline 'js quickadd' fences are excluded, since the run replaces a fence with what the script RETURNS while the preview must leave the source verbatim. Stand-ins that echo a token's own argument - a {{VALUE:}} prompt header, a macro name, a field name - degrade to a neutral placeholder when they would otherwise invent one of these characters. A stand-in is fiction either way; fiction that could not be a real file name is worse fiction. The run is deliberately unchanged. Failing fast at the create sinks would help - the folder is created and the template body formatted (prompts, macros, script fences) before Obsidian rejects the name - but a hard throw would also break appending to a colon-named file that already exists, which is legal on macOS/Linux. Filed separately. The formatter reports the colon for capture-target syntax like 'property:status=done' too, because it previews FILE NAMES and the same class previews a Template choice's file name, where that literal IS a path. CaptureTargetSetting renders no preview row for recognised picker syntax, and that gate is now pinned by a component test instead of a comment. * fix: fold in the adversarial review of the #1578 diagnostic Three lenses attacked the shipped diff and a verifier reproduced each finding. - The capture target's picker-syntax gate read the RAW field, but the run resolves the target's format tokens BEFORE parsing it. So a target written as {{GLOBAL_VAR:inbox}} expanding to 'property:type=draft' passed the gate, was previewed as a path, and got a red 'cannot contain ":"' for a capture that runs perfectly well. FormatPreviewField takes a hideWhen predicate over the RESOLVED text, and CaptureTargetSetting asks the same parser again. - The two message variants are one. Splitting on 'is the colon anywhere in the format string' sounds right and is not: every argument-bearing token carries a colon in its own syntax, so {{DATE:YYYY-MM-DD}} {{TIME}} - the shape where the hint is needed most - was told the author could see it, and only a format whose tokens take no argument at all reached the other variant. One sentence names both sources. - A half-typed 'js quickadd' fence now suppresses the diagnostic the way a half-typed {{ does. Until the closing backticks exist there is no span to strip, so a script holding {a: 1} or "HH:mm" turned the row red on every pause. hasUnterminatedInlineScriptFence lives beside findInlineScriptSpans and shares its opener rules. - The {{MATH:}} pin from the #1580 commit was itself the fiction #1580 exists to delete: {{MATH:...}} is not a token (MATH_VALUE_REGEX is /{{MVALUE}}/i), so preview and run agree on it. The case now pins {{MVALUE}}, which is the real divergence, and {{MATH:1+1}} is kept as plain text with the reason. Issue #1587 was corrected to match. - The picker-preview component test asserted an absence that could not fail (diagnostics wait for 500ms of stillness). It now advances the clock and carries a positive control, plus the token-expansion case above. - Added the {{TIME}}, {{FILE:}}, {{FILENAMECURRENT}} and {{GLOBAL_VAR:}} cases the rewritten test file's docstring claimed, and gave its formatter the target folder every real caller sets. Accepted and documented rather than fixed: a fence carried in by a {{GLOBAL_VAR:}} snippet is stripped from the scan although the run would keep it as literal text - mapping spans back through the passes is not worth it for a script inside a global variable inside a file name, and the failure is silence rather than a wrong accusation. * fix: two review findings on the illegal-character diagnostic - An existing file is never created, so Obsidian is never asked to accept its name. ':' is legal on macOS/Linux at the filesystem level, so a note made outside Obsidian really can carry one, and a capture pointed at it appends (CaptureChoiceEngine takes the fileExists branch and never reaches vault.create) - the diagnostic was marking a supported configuration broken. The vault lookup is defensive because it runs outside format()'s try/catch. - The capture target's hideWhen gate swallowed unrelated errors. A pass can fail and still leave text that parses as picker syntax: {{GLOBAL_VAR:inbox}}{{TEMPLATE:missing.md}} resolves to 'property:type=draft' followed by a not-found placeholder, and the run aborts on the missing template - hiding the row took the one message that explained it. Preview diagnostics now carry an optional kind, and 'path' marks the problems that say the RESULT is not a usable path (the normalizer's and this one). Those are the only ones a host that knows the field may not be a path at all is entitled to discard. #1594 wants the same split for the 'Unresolved:' label.
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 #1558.
The problem
The choice builders draw a live
Preview:row by re-running the whole formatpipeline on every keystroke. The display formatters override every resolution
method so nothing prompts, but they inherit the base
Formatter's parsingverbatim — warnings included — and
GuiLogger.logWarningis an unconditionalnew Notice(...).The autocomplete inserts
{{VALUE:}}with the closing braces already there, soevery prefix of an argument you type is a complete, invalid token. Typing
pascalinto{{VALUE:title|case:}}stacks five Notices across half the window.Two things I found while reproducing it are not in the issue, and one is worse
than the bug it was filed for.
The preview runs the real runtime formatter for
{{TEMPLATE:...}}FormatDisplayFormatter.getTemplateContentbuilt aSingleTemplateEngine,whose base
TemplateEngineconstructs a realCompleteFormatter. Verified livein an isolated Obsidian 1.13.0 vault: typing
{{TEMPLATE:PreviewProbe.md}}intothe Capture format field, where that template contains
{{VALUE:whoAreYou}},popped a real blocking input prompt modal on top of the settings window. A
|case:typo inside the template fired the run's Notice.{{MACRO:...}}reached the macro engine, threw (the preview passes no choice executor), and the
throw was swallowed into
Template (not found): <path>— which was simply untrue.The preflight scan reports the run's warnings before the run does
RequirementCollector extends Formatter. Verified live: with one-page inputsenabled, one
{{VALUE:title|case:pasc}}typo showed two identical Noticesbefore the input form opened, then a third on submit.
So the honest restatement is not "the preview emits Notices" but the preview
is not inert. Notices, blocking prompts and macro-engine entry are three
symptoms of one defect.
Before / after
pascalinto{{VALUE:title|case:}}{{TEMPLATE:…}}whose template asks for a{{VALUE}}5 Notices → 0, and a blocking modal → the template body previewed inline. In both
cases the problem is stated once, quietly, under the preview.
The design
A warn sink, not an
isPreviewflag. The issue proposedprotected get isPreview()threaded into three call sites. There are threeaudiences, not two — loud (the run), silent (pre-passes), collecting (the
preview) — and a boolean can silence but cannot capture, so passive surfacing
would be impossible.
WarnSinkis(message: string) => void, defaulting to aNotice.
Making it a required parameter on the internal helpers is what fixes the
bug class: the
parseAnonymousValueOptionsforwarding bug the issue documents(
|type:warning twice per parse) existed precisely becausequietwas anoptional boolean that one call site forgot and another hardcoded to
false.Inline instead of silent. Silencing alone would leave the author with no
feedback until they ran the choice. Problems now render as one muted line under
the preview, coloured by severity, held back until the field has been still
for 500 ms — every message quotes the partial token, so shown live they would
rewrite themselves per keystroke. The preview text itself stays live and
un-debounced, as it was.
Thrown failures joined the same channel. Both formatters wrapped the
pipeline in
catch { return input; }, so{{VALUE:a,b|text:x}}— and thehalf-typed
{{VALUE:}}the autocomplete leaves behind — silently echoed the rawtext back. That is the state a confused author is already in, so it is now the
most useful thing the preview says. When nothing resolved, the label reads
Unresolved:rather thanPreview:, which was asserting the raw text is theoutput.
Tradeoffs and things worth a reviewer's eye
CompleteFormatter.getTemplateContenthands the child engine a copy of thestate with
depth + 1, whilevisitedis shared by reference. Counting inthe shared
replaceTemplateInStringtoo would advance the runtime by two perlevel and halve its inclusion limit. Preview and runtime both still cap at 10
levels. What the local counter buys is accounting that spans the preview level
itself, on one budget per field — the old code handed the engine no inclusion
state, so the first nested level always restarted.
format()split intoformatInternal(input, { expandLinebreakEscapes }).An included body must not have
\nescapes expanded — the runtime neverapplies them to substituted content ([BUG] "\n" is missing after capture, i.e, "\nabla" becomes "abla" #527) — so recursing through the public
format()would corrupt a template containing\nablaorC:\Users\nadia.Pinned by a test.
warnmethod itself. Theparsers call it detached;
{ warn: this.warn }type-checks and lints clean(no type-aware eslint config, so
unbound-methodis unavailable) but wouldrun a collecting override with
this === undefined, and the TypeError isswallowed by
format()'s catch.fix: the instance's
variablesmap short-circuits an already-resolved key, soediting
{{VALUE:alpha,beta|name:t}}to{{VALUE:gamma,delta|name:t}}keptpreviewing
alphaand tripped the warn-once conflict guard — which, beforethis PR, fired a Notice from a keystroke. Regression test included.
console.warnwhose comment claimed it was kept "for thediagnostic trail".
ConsoleErrorLogger.logWarningalready console.warns everymessage, and it bypassed the hook — so the preview would still have written a
devtools line per keystroke.
runOnePagePreflightpreviewed a filename with
FormatDisplayFormatter(the note-content formatter), expanding\ninto linebreaks and resolving{{LINKCURRENT}}/{{LINKSECTION}}, both ofwhich run-time
formatFileNameleaves literal. It is the onlydisplay-formatter site that runs inside a real execution. Two tests, both
verified to fail without the change. Note run-time
formatFileNamedoesresolve
{{TEMPLATE:}}and neither file-name preview does — a pre-existing,now-documented divergence, filed as a follow-up.
Every display-formatter call site, accounted for
FormatPreviewField.svelte(5 builder fields)runOnePagePreflight.ts— inside a real runFileNameDisplayFormatter; silent (the run itself warns)InsertAfterFields.svelte\ntestDeclined
LogManager.loggersfor a no-op around the preview. Globalstatic, preview is async; a concurrent real run would lose its warnings.
isPreviewboolean. Silences but cannot capture.five live-confirmed
|caseNotices — every message interpolates the partialargument and the full token, so they are all distinct. It would only
collapse the anonymous-form duplicates, i.e. mask the forwarding bug this PR
fixes at the root. Worth filing as separate hardening.
the diagnostics are gated.
into two sets that must stay in sync forever. Stripping the redundant
QuickAdd:prefix (the logger already brands them) plus a line clamp gets thereadable result with no duplication.
.qa-field-hint.runValidatorclears that onevery value change, and it is the input's
aria-describedby— a screen readerwould read preview lint as the field's description on every focus.
no surface. Silence is strictly better than today's per-keystroke Notice
stacks, and each needs bespoke imperative DOM. Follow-up.
format()so one bad token does notblank the other tokens' previews. Real, but a class-level restructuring; the
Unresolved:label makes today's behaviour honest in the meantime. Follow-up.Known gaps, filed as follow-ups
{{TEMPLATE:}}in a file-name field previews as a literal token, while thereal run resolves it. Pre-existing on master, now documented rather than
papered over: splicing a multi-line body into a name preview is worse than
showing the token, and the only inert reader available there returns a stub.
inline surface, and still memoize their formatter. Strictly better than
today's per-keystroke Notice stacks; each needs bespoke imperative DOM.
aria-liveregion would talk over the preview's). The severity itself is in the DOM as
visually-hidden text, so it is readable on navigation — just not proactive.
Unresolved:labelmakes that honest, but per-token containment is a class-level restructuring.
Validation
pnpm run build,pnpm lint, full suite: 4038 passed, 0 failed.through the real formatters.
(
pnpm run obsidian:e2e), not just in tests: 0 Notices while typing (was 5),0 prompt modals from a
{{TEMPLATE:}}preview (was 1 blocking modal), theexplanation appearing once typing stops.