Repository navigation
Task/obsidian review cleanup - #14
Merged
Merged
Conversation
…eprecated APIs, CSS lint) - prefer-create-el: Obsidian helpers in noteProperty/NotepadView, narrow DetachedDoc helper in export/postprocess, harness uses its own shim (7 shim implementation sites remain as the documented exception) - TFile casts -> instanceof guards; unsafe-* -> typed guards + local NodeFs interface in noteStorage/umVault (no typeof import node:fs) - no-explicit-any (texto/core covariance) and empty Commands interfaces (declaration merging) suppressed pointwise with descriptions; eslint config aligned with review (empty interfaces no longer allowed) - deprecated APIs: messageEl fallback (Notice), caretPositionFromPoint with caretRangeFromPoint fallback, declarative getSettingDefinitions (display() kept as pre-1.13 fallback), window.requestAnimationFrame, window.crypto instead of globalThis; writeAssetDescriptors removed - CSS: all 24 !important refactored to specificity/order/exclusions (table borders inline styles now plain, drag-handle reveal rules carry :not(.is-grabbed)/:not(.is-dragging)), column-gap -> gap, standard scrollbar props dropped in favor of ::-webkit-scrollbar, intentional text-decoration/display:contents closed with described stylelint disables, note.css regenerated by scripts/generate-note-css.mjs with duplicate-property dedup - tooling: stylelint + lint:css gate (browserslist Chrome 124 = Obsidian 1.6.x), lint:src gate with --max-warnings 0 Gates: lint:src 0, lint:css 0, typecheck, 629 tests, build — pass
…rtant, dedup hardening) - note-property gap -> two-value (row-gap 0): the same node is a flex row for list values; single-value gap changed wrap spacing - img.ProseMirror-separator: !important back under a documented stylelint-disable (must beat unknown user-theme img rules, per PM's recommendation) - generate-note-css.mjs: importance-aware dedup key, url()-blocks and quote/brace crossings fail fast instead of mis-splitting - review nits: attrCode JSDoc placement, keyframes-name-pattern disable description, loadSettings comment wording, .inscriptum-log.jsonl name, dangling test title
…, CI gates, settings value read - browser-harness: describe the shim's own createEl/createDiv/createSpan implementation (the rule would flag its own shim) and type the class-list helpers; eslint.config.mts allows exactly obsidianmd/prefer-create-el disables, every other obsidianmd rule stays non-disableable - empty-interface suppressions now name both rule ids (the bot's rule name is not visible to us) and the local config keeps both enabled - main.ts: getControlValue reads the plugin's own settings so the 1.13+ declarative rows render the saved values instead of empty fields - ci.yml: run lint:src (--max-warnings 0) and lint:css on every push - scripts/debug/verify-review-css.mjs: live Chromium check of the CSS refactor (fold hidden, table border cascade incl. the No-borders preset, chevron reveal/hide states, note-property gap, hidden scrollbar) Gates: lint:src 0, lint:css 0, typecheck, 629 tests, build — pass
…Obsidian API) The shim iterated the argument as string[][] and spread each element — a multi-character class name was added character by character.
…ermaid 12.2.0, vite 8.3.5, obsidian 1.14.4 - prosemirror-view 1.42.2 -> 1.42.6: XSS in paste handling (GHSA-c8x8-7fp4-3x9w) — this library ships inside main.js, so the advisory was user-facing - prosemirror-model/-commands/-history/-dropcursor/-transform/-schema-basic: patch bumps - mermaid 11.17.2 -> 12.2.0: the published mermaid chunk carries katex 0.16.47 (GHSA-238p-pmpm-9mq7); 12.2.0 carries katex 0.18.11. An npm override would only silence the audit, not the bundled code - vite 8.2.0 -> 8.3.5 (dev): three launch-editor advisories on Windows - obsidian 1.13.1 -> 1.14.4 + override (eslint-plugin-obsidianmd pins 1.12.3 -> moment 2.29.4, GHSA-4p3w-j4w9-5jqw) - eslint-plugin-obsidianmd 0.4.1 -> 0.4.2 Residual: braces 3.0.3 (CVE-2026-93687) via stylelint -> micromatch -> braces. Dev-only, not shipped, no upstream patch — dismiss with rationale. Bundle: main.js 4,359 kB -> 6,091 kB (gzip 1,275 -> 1,800 kB); the delta is mermaid 12's new ELK layout chunk. Gates: lint:src 0, lint:css 0, typecheck, 629 tests, build — pass Live: mermaid renders on desktop (1.14.4) and Android, pasted markup is sanitized (no onerror/script), editor unaffected
mermaid-clean-check.note was a throwaway typing fixture tracked in the repo root; local/ is the gitignored scratch area of the dev vault.
 [Medium impact] The PR appears safe to merge; both previous findings are addressed and no new blocking issue was found. SummaryThis PR tightens lint checks, cleans up types and CSS, updates dependencies, and adds searchable settings. Changes since the previous review address both reported issues:
No new actionable issues were found. Tests were inspected but not run. Reviews (2) · Last reviewed commit: "fix: PR review findings — harness mounts..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… respects quoted values Greptile P1 (src/NotepadView.ts:1392): the new 'instanceof TFile' guard (the community review forbids TFile casts) rejected the harness' plain object for v.file, so the notepad editors never mounted and 22 e2e specs timed out. The harness now builds a real TFile instance. Also fixed the sibling break the failing e2e run exposed: the note-property extension now uses Obsidian's createSvg helper (prefer-create-el), which the harness shim did not provide — the notepad editor failed to init with 'createSvg is not defined'. The shim now implements createSvg (SVG namespace, class via setAttribute — SVG className is read-only). Greptile P2 (scripts/generate-note-css.mjs:101): dedupProperties() split declaration blocks on every ';', including semicolons inside quoted values — a value like content:"x;color:red" was cut in half and the generated stylesheet lost a fragment. The splitter is now quote-aware, fails fast on unbalanced quotes, and the pure helpers moved to scripts/generate-note-css-lib.mjs with a regression test (tests/generate-note-css/generate-note-css.test.ts). Gates: lint:src 0, lint:css 0, typecheck, 643 tests, 48/48 e2e — pass
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
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.
No description provided.