Repository navigation
fix(picker): render plain choice names without MarkdownRenderer - #2252
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Shared rendering behavior src/gui/choiceList/renderChoiceName.ts, src/gui/choiceList/renderChoiceName.test.ts, tests/e2e/choice-name-render.test.ts |
The helper uses an owned paragraph and textContent for qualifying plain names. Other names go through awaited MarkdownRenderer.render. Unit and end-to-end tests cover both paths and compare rendered output. |
Picker integration src/gui/suggesters/choiceSuggester.ts, src/gui/suggesters/choiceSuggester.test.ts, src/gui/choiceList/*, src/quickAddSettingsTab.test.ts |
ChoiceSuggester now uses the shared helper. Choice-list callers and settings tests explicitly discard the helper's return value where needed. Tests check plain and Markdown-styled names. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant ChoiceSuggester
participant renderChoiceName
participant MarkdownRenderer
participant ChoiceRow
ChoiceSuggester->>renderChoiceName: pass choice name and rendering context
alt plain-text criteria match
renderChoiceName->>ChoiceRow: set textContent in owned paragraph
else markdown rendering required
renderChoiceName->>MarkdownRenderer: render choice name
MarkdownRenderer-->>renderChoiceName: return rendered content
end
Merge Risk: 
75736
-
The rendering change is ready to merge after normal checks; no actionable issue remains identified.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "fix(picker): render plain choice names without MarkdownRenderer" matches the main change. The PR writes eligible plain choice names directly into a paragraph in the picker and settings list… Linked Issues check Issue #2251requires the QuickAdd picker to open near-instantly with Iconic enabled. The change routes picker rows throughrenderChoiceName, writes eligible plain names directly to<p dir="auto">,…Out of Scope Changes check The changes remain connected to #2251. The shared helper, settings-list integration, rejection fallback, unit tests, and end-to-end rendering test support the same rendering and performance change. No…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 6 files. (2 skipped: 2 … Full details: Title check
-
Explanation
The title "fix(picker): render plain choice names without MarkdownRenderer" matches the main change. The PR writes eligible plain choice names directly into a paragraph in the picker and settings list. Names with Markdown or HTML constructs still use MarkdownRenderer. The title names the picker scope and the bypass of MarkdownRenderer, so a reviewer can see the primary change. It uses the conventional commit prefix and is 63 characters long, within a reasonable limit.
Full details: Linked Issues check
-
Explanation
Issue
#2251requires the QuickAdd picker to open near-instantly with Iconic enabled. The change routes picker rows throughrenderChoiceName, writes eligible plain names directly to<p dir="auto">, and keeps Markdown rendering for names that need it. This removes the repeated Markdown post-processor calls for ordinary choice names. Unit, suggester, and end-to-end tests cover the classifier and compare picker output with Obsidian's renderer.
Full details: Out of Scope Changes check
-
Explanation
The changes remain connected to
#2251. The shared helper, settings-list integration, rejection fallback, unit tests, and end-to-end rendering test support the same rendering and performance change. No unrelated product behavior or unrelated files appear in the reviewed diff.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 6 files. (2 skipped: 2 unsupported.)
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit hops past names in flight
Plain words land as text just right
Bold marks take the markdown trail
Tests check each rendered detail
The picker rows now greet the light
Comment @coderabbitai help to get the list of available commands.
|
Independent verification: PASS (an agent that did not write the change ran it in Obsidian 1.13.7, with a 28k-note vault and Iconic 1.1.11 enabled).
The verdict ran on |
Fixes #2251.
Why
With Iconic enabled, Run QuickAdd took about a second to open on a large vault. The picker rendered every choice name with
MarkdownRenderer.render. That call runs every plugin's Markdown post processors. Iconic's post processor readsmetadataCache.getTags(), a scan of the whole vault, on every call before it looks for a tag. So 27 rows meant 27 vault scans per open, and more on each keystroke. The settings list had the same cost.What changed
renderChoiceNamenow writes a plain name straight into<p dir="auto">, which is the output Obsidian's renderer gives for such a name. Names that use Markdown or HTML still go through the renderer.renderChoiceName.,.'"?;()-, and it does not open a list, an indented code block, or awww.autolink.Scope
/as the link source path instead of"", the same as the settings list. Both render identically in Obsidian.Blast Radius
Every picker row and settings row with a plain name skips the Markdown renderer. If the gate accepted a name that Obsidian renders differently, that name would show as literal text. The fuzz and e2e checks below found no such name.
Verification
getTags()calls per open: 27 → 0. Settings tab: about 950 ms → 27-51 ms.tests/e2e/choice-name-render.test.tschecks the real picker against the renderer, and it fails ifdir="auto"is removed.Note
Render plain picker choice names as text without
MarkdownRendererAdds an
isPlainNameclassifier torenderChoiceNameso ordinary names render as an automatic-direction text paragraph, while names with Markdown or HTML constructs still go through the async Markdown renderer. RoutesChoiceSuggester.renderSuggestionthrough the shared helper, keeping the existing rejection fallback. Adds unit and end-to-end tests covering plain names, Markdown-sensitive names, and HTML comparison against Obsidian's renderer.renderChoiceNamein renderChoiceName.ts is now async and returns a promise; callers in ChoiceListItem.svelte, MultiChoiceListItem.svelte, and quickAddSettingsTab.test.ts discard that promise, so render failures no longer surface there.Macroscope summarized 7573699.
Note
Medium Risk
Misclassified plain names would show literally instead of formatted Markdown; settings list callers no longer handle render promise rejections, though the picker still does.
Overview
Fixes Run QuickAdd slowness (#2251) by skipping Obsidian’s
MarkdownRendererfor choice names that would render as plain text anyway, so third-party Markdown post-processors are not invoked once per row.renderChoiceNamenow classifies names withisPlainName(allowed punctuation/Unicode, excluding list openings andwww.autolinks) and writes them as<p dir="auto">text; Markdown/HTML names still useawait MarkdownRenderer.renderwith source path/. The function is async, and settings list plus ChoiceSuggester both route through this helper (the picker keeps its existing render-failure fallback).Tests add plain vs markdown matrices, a picker integration check that only styled names hit the renderer, and an e2e test that picker row HTML matches Obsidian’s renderer output.
Reviewed by Cursor Bugbot for commit 7573699. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit