Implement all three markdown post-processors (Features 1-4)#3
Open
YellowKidokc wants to merge 1 commit intomainfrom
Open
Implement all three markdown post-processors (Features 1-4)#3YellowKidokc wants to merge 1 commit intomainfrom
YellowKidokc wants to merge 1 commit intomainfrom
Conversation
Feature 2: Inline JSON annotations (%%{...}%%)
- Parse %%{...}%% from raw markdown source via getSectionInfo()
- Render colored badges: blue (laws), gold (axioms), gray (direction)
- Click any badge to expand/collapse annotation detail panel
- Respects badgeSize setting (small/medium/hidden)
Feature 3: Deep drill expand blocks (%%[expand-ID]...%%])
- Parse %%{ref:"expand-ID"}%% markers from source
- Match to %%[expand-ID]...%%] content blocks in full document
- Render ▶ trigger icon at end of sentence
- Click ▶ to expand/collapse styled annotation panel with content
- Basic markdown rendering (bold, italic, code) in expand content
Features 1 & 4: Math callouts ([!math-interlinear] and [!math-stack])
- Detect Obsidian callouts with data-callout attribute
- math-interlinear: parse equation/terms/english code blocks,
render as styled container with symbol-meaning grid and hover tooltips
- math-stack: parse pipe-delimited rows, render as column-aligned
equation + term grid with divider and English summary
- Supports compact mode (equation-only) with click-to-expand
https://claude.ai/code/session_01FUXUtLTZwDCycrvCcQWbGj
📝 WalkthroughWalkthroughThe plugin implements three major markdown parsing and rendering features: inline annotation detection and badge rendering, expandable block parsing with reference markers, and custom rendering for math-related callouts. Changes include keyboard shortcut documentation updates and approximately 425 net lines of new parsing and DOM manipulation logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/View
participant Plugin as TheophysicsLayersPlugin
participant Parser as Markdown Parser
participant DOM as DOM Renderer
User->>Plugin: Trigger markdown re-render
Plugin->>Parser: Extract section raw markdown
Parser->>Parser: Detect %%{...}%% patterns
Parser->>Parser: Parse key-value pairs (relaxed)
Parser->>DOM: Create annotation badge elements
DOM->>User: Render clickable badges
Plugin->>Parser: Parse full document source
Parser->>Parser: Detect %%[expand-ID]...%%] blocks
Parser->>Parser: Match %%{...ref:"expand-ID"...}%% markers
Parser->>DOM: Create expandable panel triggers
DOM->>User: Render "▶" toggle buttons
Plugin->>Parser: Scan rendered HTML for callouts
Parser->>Parser: Identify math-interlinear/math-stack
Parser->>Parser: Parse pipe-delimited row content
Parser->>DOM: Build custom math container/grid
DOM->>User: Display formatted math layout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
main.ts (2)
168-182: Consider consolidating post-processors.Three separate
registerMarkdownPostProcessorcalls iterate the DOM independently. For performance on large documents, consider combining them into a single processor that handles all three feature checks.💡 Consolidated processor example
- // 1. Parse %%{...}%% inline JSON annotations - this.registerMarkdownPostProcessor((el, ctx) => { - if (!this.physicistMode || !this.settings.parseInlineJson) return; - this.processInlineAnnotations(el, ctx); - }); - - // 2. Parse %%[expand-ID]...%%] deep drill blocks - this.registerMarkdownPostProcessor((el, ctx) => { - if (!this.physicistMode || !this.settings.parseExpandBlocks) return; - this.processExpandBlocks(el, ctx); - }); - - // 3. Render [!math-stack] and [!math-interlinear] callouts - this.registerMarkdownPostProcessor((el, ctx) => { - this.processMathCallouts(el, ctx); - }); + // Combined markdown post-processor for all features + this.registerMarkdownPostProcessor((el, ctx) => { + if (this.physicistMode) { + if (this.settings.parseInlineJson) this.processInlineAnnotations(el, ctx); + if (this.settings.parseExpandBlocks) this.processExpandBlocks(el, ctx); + } + this.processMathCallouts(el, ctx); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.ts` around lines 168 - 182, Multiple independent registerMarkdownPostProcessor registrations cause redundant DOM iteration; consolidate them into a single registerMarkdownPostProcessor that checks this.physicistMode and the relevant settings flags and then calls this.processInlineAnnotations(el, ctx), this.processExpandBlocks(el, ctx), and this.processMathCallouts(el, ctx) as needed. Locate the three calls to registerMarkdownPostProcessor and replace with one callback that: returns early if !this.physicistMode, then if this.settings.parseInlineJson invoke processInlineAnnotations, if this.settings.parseExpandBlocks invoke processExpandBlocks, and always invoke processMathCallouts (or conditionally if desired), preserving the existing method names and behavior.
424-427: Click handler on container may interfere with text selection.When
showMathInterlinearis"equation-only", the entire container becomes clickable. This can make it difficult for users to select text within the equation for copying.💡 Consider adding a dedicated expand button instead
if (this.settings.showMathInterlinear === "equation-only") { container.classList.add("compact"); - container.addEventListener("click", () => container.classList.toggle("expanded")); + const expandBtn = document.createElement("button"); + expandBtn.className = "math-interlinear-expand-btn"; + expandBtn.textContent = "▶"; + expandBtn.addEventListener("click", (e) => { + e.stopPropagation(); + container.classList.toggle("expanded"); + }); + container.appendChild(expandBtn); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.ts` around lines 424 - 427, The container-level click handler added when this.settings.showMathInterlinear === "equation-only" interferes with text selection; instead remove the container.addEventListener("click", ...) and add a dedicated expand toggle button (e.g., create an expandButton element, append it to the same container after container.classList.add("compact")), attach the click handler to that button to toggle the container's "expanded" class, and ensure the button has appropriate accessible labels (aria-expanded) so selecting text inside the equation is not blocked by a global container click.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@main.ts`:
- Around line 261-266: The panel injection uses panel.innerHTML with unsanitized
annotation fields (annotation.note, annotation.law, annotation.axioms,
annotation.direction) which allows XSS; fix by building DOM nodes instead of
interpolating HTML: create elements (e.g., document.createElement('strong') for
labels and text nodes for values) or sanitize/escape values before inserting,
push plain text via textContent or appendChild on panel rather than using
innerHTML and content.join — update the code paths that populate the content
array and replace the innerHTML assignment on panel with safe DOM construction
or escaping logic.
- Around line 340-361: The renderExpandContent method currently applies
markdown-like replacements without escaping HTML, enabling XSS; update
renderExpandContent to first HTML-escape the input (e.g., escape &, <, >, ", ')
for each line (use or add a helper like escapeHtml) before performing
bold/italic/inline-code replacements, and ensure the inline code replacement
encodes inner content or runs against the escaped string so `<script>` cannot be
interpreted; keep the rest of the transformation logic but operate on the
escaped text (reference: renderExpandContent function and any new escapeHtml
helper).
- Around line 84-88: The regex that injects quotes around unquoted keys (used on
the variable raw to build jsonStr before JSON.parse) can corrupt values
containing colons; replace this brittle transformation by using a relaxed JSON
parser (e.g., add and import JSON5 and call JSON5.parse on the raw-wrapped
object) or, if you prefer not to add a dependency, implement a safer manual
parser that tokenizes top-level key/value pairs (use the symbols raw, match,
jsonStr, and the parsing site in the try block) to avoid matching colons inside
values; ensure the code removes the replace(/(\w+)\s*:/g, '"$1":') step and uses
JSON5.parse or the new safe-splitting logic so JSON with colons in values is
handled correctly while keeping the project target ES6.
---
Nitpick comments:
In `@main.ts`:
- Around line 168-182: Multiple independent registerMarkdownPostProcessor
registrations cause redundant DOM iteration; consolidate them into a single
registerMarkdownPostProcessor that checks this.physicistMode and the relevant
settings flags and then calls this.processInlineAnnotations(el, ctx),
this.processExpandBlocks(el, ctx), and this.processMathCallouts(el, ctx) as
needed. Locate the three calls to registerMarkdownPostProcessor and replace with
one callback that: returns early if !this.physicistMode, then if
this.settings.parseInlineJson invoke processInlineAnnotations, if
this.settings.parseExpandBlocks invoke processExpandBlocks, and always invoke
processMathCallouts (or conditionally if desired), preserving the existing
method names and behavior.
- Around line 424-427: The container-level click handler added when
this.settings.showMathInterlinear === "equation-only" interferes with text
selection; instead remove the container.addEventListener("click", ...) and add a
dedicated expand toggle button (e.g., create an expandButton element, append it
to the same container after container.classList.add("compact")), attach the
click handler to that button to toggle the container's "expanded" class, and
ensure the button has appropriate accessible labels (aria-expanded) so selecting
text inside the equation is not blocked by a global container click.
Comment on lines
+84
to
+88
| try { | ||
| const raw = match[1]; | ||
| // Handle both strict JSON and relaxed (unquoted keys) formats | ||
| const jsonStr = "{" + raw.replace(/(\w+)\s*:/g, '"$1":').replace(/'/g, '"') + "}"; | ||
| const parsed = JSON.parse(jsonStr); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does TypeScript support lookbehind in regular expressions?
💡 Result:
Yes, TypeScript supports lookbehind in regular expressions (?<=...) and (?<!...) as part of ECMAScript 2018 features. TypeScript parses and type-checks these syntax elements when the target is ES2018 or later (error TS1499 otherwise). At runtime, support depends on the JavaScript engine, which has near-universal support in modern browsers and Node.js since 2018.
Citations:
- 1: Type Inference for Regular Expressions microsoft/TypeScript#60249
- 2: Regular Expression finds microsoft/TypeScript#58287
- 3: https://stackoverflow.com/questions/60014768/how-to-make-typescript-ts-node-accept-the-lookbehind-assertion-an-ecmascript-20
- 4: Provide Syntax Checking for Regular Expressions microsoft/TypeScript#55600
- 5: FR: Add TS config option to disable "<some regex feature> is only available when targeting 'es2018' or later.". microsoft/TypeScript#59066
- 6: https://stackoverflow.com/questions/49185764/which-es2018-features-are-implemented-in-typescript
🏁 Script executed:
find . -name "main.ts" -type fRepository: YellowKidokc/THEOPHYSICS_LAYERS
Length of output: 84
🏁 Script executed:
# Check tsconfig to understand the target
find . -name "tsconfig.json" -type fRepository: YellowKidokc/THEOPHYSICS_LAYERS
Length of output: 90
🏁 Script executed:
cat -n main.ts | sed -n '80,95p'Repository: YellowKidokc/THEOPHYSICS_LAYERS
Length of output: 776
🏁 Script executed:
cat tsconfig.jsonRepository: YellowKidokc/THEOPHYSICS_LAYERS
Length of output: 458
🏁 Script executed:
node <<'EOF'
// Test the regex transformation
const testCases = [
"note:see:this",
"law:'test',axioms:5",
"note:'url:with:colons'",
"key:value",
];
testCases.forEach(raw => {
const jsonStr = "{" + raw.replace(/(\w+)\s*:/g, '"$1":').replace(/'/g, '"') + "}";
console.log(`Input: ${raw}`);
console.log(`Output: ${jsonStr}`);
try {
JSON.parse(jsonStr);
console.log("✓ Valid JSON\n");
} catch (e) {
console.log(`✗ Invalid JSON: ${e.message}\n`);
}
});
EOFRepository: YellowKidokc/THEOPHYSICS_LAYERS
Length of output: 583
Regex for unquoted keys breaks JSON with colons in values.
The transformation raw.replace(/(\w+)\s*:/g, '"$1":') matches colon patterns anywhere, including inside values:
note:see:this→{"note":"see":this}(invalid JSON)note:'url:with:colons'→ broken transformation of quoted strings
However, the proposed lookbehind fix requires ES2018+ (error TS1499), but this project targets ES6 in tsconfig.json.
Consider instead:
- Using a JSON5 library for relaxed JSON parsing
- Restructuring input to require quoted values:
{note:"value",axioms:[]} - Manual object parsing with safer string splitting
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main.ts` around lines 84 - 88, The regex that injects quotes around unquoted
keys (used on the variable raw to build jsonStr before JSON.parse) can corrupt
values containing colons; replace this brittle transformation by using a relaxed
JSON parser (e.g., add and import JSON5 and call JSON5.parse on the raw-wrapped
object) or, if you prefer not to add a dependency, implement a safer manual
parser that tokenizes top-level key/value pairs (use the symbols raw, match,
jsonStr, and the parsing site in the try block) to avoid matching colons inside
values; ensure the code removes the replace(/(\w+)\s*:/g, '"$1":') step and uses
JSON5.parse or the new safe-splitting logic so JSON with colons in values is
handled correctly while keeping the project target ES6.
Comment on lines
+261
to
+266
| const content: string[] = []; | ||
| if (annotation.law.length) content.push(`<strong>Laws:</strong> ${annotation.law.join(", ")}`); | ||
| if (annotation.axioms.length) content.push(`<strong>Axioms:</strong> ${annotation.axioms.join(", ")}`); | ||
| if (annotation.direction) content.push(`<strong>Direction:</strong> ${annotation.direction}`); | ||
| if (annotation.note) content.push(`<strong>Note:</strong> ${annotation.note}`); | ||
| panel.innerHTML = content.join("<br>"); |
There was a problem hiding this comment.
XSS vulnerability: annotation fields are not sanitized before HTML injection.
The annotation.note and other fields are interpolated directly into HTML without escaping. A malicious annotation like %%{note:"<img src=x onerror=alert(1)>"}%% would execute JavaScript.
While Obsidian plugins typically process trusted user content, shared vaults or synced files could introduce risks.
🛡️ Proposed fix using textContent for safe rendering
- const content: string[] = [];
- if (annotation.law.length) content.push(`<strong>Laws:</strong> ${annotation.law.join(", ")}`);
- if (annotation.axioms.length) content.push(`<strong>Axioms:</strong> ${annotation.axioms.join(", ")}`);
- if (annotation.direction) content.push(`<strong>Direction:</strong> ${annotation.direction}`);
- if (annotation.note) content.push(`<strong>Note:</strong> ${annotation.note}`);
- panel.innerHTML = content.join("<br>");
+ const addField = (label: string, value: string) => {
+ const row = document.createElement("div");
+ const strong = document.createElement("strong");
+ strong.textContent = `${label}: `;
+ row.appendChild(strong);
+ row.appendChild(document.createTextNode(value));
+ panel.appendChild(row);
+ };
+ if (annotation.law.length) addField("Laws", annotation.law.join(", "));
+ if (annotation.axioms.length) addField("Axioms", annotation.axioms.join(", "));
+ if (annotation.direction) addField("Direction", annotation.direction);
+ if (annotation.note) addField("Note", annotation.note);🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 265-265: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: panel.innerHTML = content.join("
")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 265-265: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: panel.innerHTML = content.join("
")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main.ts` around lines 261 - 266, The panel injection uses panel.innerHTML
with unsanitized annotation fields (annotation.note, annotation.law,
annotation.axioms, annotation.direction) which allows XSS; fix by building DOM
nodes instead of interpolating HTML: create elements (e.g.,
document.createElement('strong') for labels and text nodes for values) or
sanitize/escape values before inserting, push plain text via textContent or
appendChild on panel rather than using innerHTML and content.join — update the
code paths that populate the content array and replace the innerHTML assignment
on panel with safe DOM construction or escaping logic.
Comment on lines
+340
to
+361
| private renderExpandContent(content: string): string { | ||
| const lines = content.split("\n"); | ||
| const htmlParts: string[] = []; | ||
|
|
||
| for (const line of lines) { | ||
| // Strip leading > markers | ||
| let cleaned = line.replace(/^>\s?/, "").replace(/^>\s?/, ""); | ||
| if (!cleaned.trim()) { | ||
| htmlParts.push("<br>"); | ||
| continue; | ||
| } | ||
| // Bold markers | ||
| cleaned = cleaned.replace(/\*\*(.+?)\*\*/g, "<strong>$1</strong>"); | ||
| // Italic markers | ||
| cleaned = cleaned.replace(/\*(.+?)\*/g, "<em>$1</em>"); | ||
| // Inline code | ||
| cleaned = cleaned.replace(/`(.+?)`/g, "<code>$1</code>"); | ||
| htmlParts.push(cleaned); | ||
| } | ||
|
|
||
| return htmlParts.join("<br>"); | ||
| } |
There was a problem hiding this comment.
XSS vulnerability: expand block content is rendered without HTML escaping.
The renderExpandContent method applies markdown transformations (bold, italic, code) but doesn't escape HTML entities first. Content like <script>alert(1)</script> in an expand block would execute.
🛡️ Proposed fix: escape HTML before applying markdown transforms
private renderExpandContent(content: string): string {
const lines = content.split("\n");
const htmlParts: string[] = [];
+
+ const escapeHtml = (str: string) =>
+ str.replace(/&/g, "&").replace(/</g, "<").replace(/>/g, ">").replace(/"/g, """);
for (const line of lines) {
// Strip leading > markers
let cleaned = line.replace(/^>\s?/, "").replace(/^>\s?/, "");
if (!cleaned.trim()) {
htmlParts.push("<br>");
continue;
}
+ // Escape HTML entities first
+ cleaned = escapeHtml(cleaned);
// Bold markers
cleaned = cleaned.replace(/\*\*(.+?)\*\*/g, "<strong>$1</strong>");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main.ts` around lines 340 - 361, The renderExpandContent method currently
applies markdown-like replacements without escaping HTML, enabling XSS; update
renderExpandContent to first HTML-escape the input (e.g., escape &, <, >, ", ')
for each line (use or add a helper like escapeHtml) before performing
bold/italic/inline-code replacements, and ensure the inline code replacement
encodes inner content or runs against the escaped string so `<script>` cannot be
interpreted; keep the rest of the transformation logic but operate on the
escaped text (reference: renderExpandContent function and any new escapeHtml
helper).
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
2 participants
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.
Feature 2: Inline JSON annotations (%%{...}%%)
Feature 3: Deep drill expand blocks (%%[expand-ID]...%%])
Features 1 & 4: Math callouts ([!math-interlinear] and [!math-stack])
https://claude.ai/code/session_01FUXUtLTZwDCycrvCcQWbGj
Summary by CodeRabbit
New Features
Chores