Skip to content

chore: delete the code nothing reaches, and narrow the exports nothing imports - #506

Merged
PathGao merged 2 commits into
masterfrom
chore/delete-unreachable-code
Aug 7, 2026
Merged

chore: delete the code nothing reaches, and narrow the exports nothing imports#506
PathGao merged 2 commits into
masterfrom
chore/delete-unreachable-code

Conversation

@PathGao

@PathGao PathGao commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What this is

Deletions only, no behaviour change: four functions in MarkdownViewer.svelte that nothing calls, 24 exports nothing outside their own file imports, one alias that only forwards, one wrapper with no production caller, and one FrontMatterField property that is a duplicate of the property beside it. Net -162 lines, no dependency change.

Three of the four dead functions are what makes this worth a PR rather than a tidy-up — they are stale copies of render steps that still run somewhere else:

dead copy in MarkdownViewer.svelte live implementation what the live one has that the copy doesn't
processTaskItems src/lib/utils/markdown.ts:368 the data-task-checkbox guard, paragraph re-parenting, whitespace-node handling, the inputIdx === -1 guard
processBlockIds src/lib/utils/markdown.ts:252 — same logic, just a second copy
processHighlights Rust, src-tauri/src/lib.rs:967 code-span and fence protection (#228, #371); the copy is a bare text.replace(/==([^=\n]+)==/g, …)

getSplitTransition is simply unreachable.

Mechanism

b46a283 (2026-03-20) added the three process* steps to MarkdownViewer.svelte. 09fa88f (2026-03-27) moved the two DOM ones into markdown.ts, and the ==highlight== rewrite moved into the Rust renderer. Each migration re-pointed the call site and left the old function body in place.

Nothing was ever going to complain. A module-level function in a .svelte <script> that nobody calls is legal TypeScript; svelte-check reports it as neither error nor warning, and cargo's dead_code lint doesn't reach across the language boundary. So the copies have sat there since March looking like helpers.

That is also the gap in scripts/singleImplementationConvention.test.ts: its rules ask whether a marker appears in more than one allowed file, which an unreachable second copy passes. The bug class that suite describes — "it looks like a reusable shared helper, so the next person to reuse or sync it silently reverts a merged fix" — is live here. Reusing processTaskItems from MarkdownViewer.svelte would reintroduce the pre-data-task-checkbox behaviour.

The rest is smaller. editableValue is worth one line: it computes the identical string as displayValue in every branch (the array branch is the same expression spelled twice), and nothing type-checks that they stay equal, so it is a copy waiting to drift. The one consumer now reads displayValue.

Scope

Found and deliberately left alone:

  • A guard rule for the deleted copies. Cleanup without a gate can regress, but the obvious markers don't qualify: processTaskItems and processBlockIds are private in markdown.ts, and that suite states a marker "must never be a private identifier". The one that could be pinned by defect shape is the highlight rewrite — marker: /replace\([^)]*<mark>/g with allowed: [], matching the three existing empty-allowed rules. Say the word and I'll add it here so cleanup and gate land together.
  • 13 copy-on-write updates on $state records in MarkdownViewer.svelte (x = { ...x, [k]: v }, and copy-then-delete). Svelte 5's $state proxy already makes plain mutation reactive, so these are Svelte-4-era habit, but no test covers that component's front-matter tag editor and neither svelte-check nor the suite verifies reactivity. Not a change to make blind.
  • The 22 near-identical checkbox blocks in Settings.svelte and the toggleX() methods behind them. The methods are not boilerplate: most have 2–3 call sites and several map a boolean onto 'on'/'off' or run real logic (toggleZenMode is 29 lines). Deleting them would copy that logic outward, not remove it.
  • 93 en i18n keys with no static reference. The native macOS menu is built in Rust and titlebarToolbar.ts resolves keys indirectly through labelKey, so a static scan can't separate genuinely dead keys from indirectly referenced ones. Acting on it would be a ~2400-line diff across 26 locales on a guess; it needs runtime evidence first.

Tests

None added, and the honest reason is that no test applies. Every deletion here is either unreachable code or a name nothing imports, so there is no behaviour to pin — a test written for any of it would pass identically with and without the change. The falsification check the template asks for degenerates for the same reason: putting the four functions back leaves the suite green, which is exactly the claim.

scripts/frontMatter.test.ts changed for a different reason: addFrontMatterListItem had no production caller, only that test, so the three assertions now call addFrontMatterListItems with a one-element array. Same inputs, same expectations.

What actually verifies this change is the reachability evidence, not a test: every deleted symbol has zero references across src/ and scripts/ (the four functions have exactly one occurrence each — their own definition), checked over .ts, .svelte and the Svelte markup, and svelte-check compiles the tree afterwards.

Verification

npm audit      → found 0 vulnerabilities
npm run check  → 653 FILES 0 ERRORS 0 WARNINGS 0 FILES_WITH_PROBLEMS
npm test       → tests 778, pass 778, fail 0
cargo test     → 125 passed; 0 failed

Not verified: I did not run this on Windows or Linux, and I did not launch the app. The deletions contain no platform branch (#[cfg], osType, or otherwise) and no deleted symbol has a caller, so there is no runtime path left to exercise — but that reasoning is what I'm offering, not an observation.

PathGao and others added 2 commits August 7, 2026 00:47
…g imports

Four functions in MarkdownViewer.svelte have no caller. Three of them are
stale copies of render steps that moved elsewhere and kept evolving there:
processTaskItems and processBlockIds moved into markdown.ts in 09fa88f, and
the ==highlight== rewrite moved into the Rust renderer, where it later grew
code-span protection (#228, #371). Each migration re-pointed the call site
and left the old body behind, so the copies have been sitting there since
b46a283 looking like reusable helpers. getSplitTransition is plain dead.

Also removed: 24 exports nothing outside their own file imports, the
escapeHtmlText alias that only forwards to escapeHtml, an addFrontMatterList-
Item wrapper with no production caller, the toPlainRecord guard that
isFrontMatterMapping already makes unreachable, and FrontMatterField's
editableValue, which computes the same string as displayValue in every
branch with nothing to keep the two from drifting apart.

No behaviour change. The front matter tag test now calls the plural helper.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same sweep as the previous commit, finished in scripts/. Twenty-two names in
keymapHarness.ts, renderProtocolDom.ts, renderProtocolFixtures.ts,
windowTagEditor.ts and scrollSyncBlockMapping.test.ts are only used inside
their own file, so the export widened the contract for nothing.
RenderFixtureName had no use anywhere, including its own file, and is gone.

tsconfig.json puts scripts/ inside `npm run check`, so a name another file
still imports would fail the type check rather than pass silently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant