refactor: dedupe alias resolution, quote-stripping, error messages, and attribution drawing - #77
Merged
Merged
Conversation
cause instanceof Error ? cause.message : String(cause) was repeated verbatim across 10 call sites in 4 files. Pulled into a single util/errorMessage.ts, with a small test of its own.
…tripping idioms resolveFontAlias(x) ?? x was repeated at 7 call sites across autoload.ts, catalogue.ts, googleFonts.ts and install.ts; pulled into normalizeFontFamily(). part.trim().replace(/^["']|["']$/g, '') for a single CSS font-family token was duplicated identically in catalogue.ts, registry.ts and pipeline.ts; pulled into unquoteFontFamily(), both now exported from font/catalogue.ts alongside resolveFontAlias itself. Pure extraction, no behavior change.
…themes
font/catalogue.ts's FONT_ALIASES and theme/colorThemes.ts's
COLOR_THEME_ALIASES/CUSTOM_COLOR_THEME_ALIASES built the same alias -> key
record from a list of {key, alias} rows — one as an inline IIFE, the other
as its own aliasMap() function. Likewise CATALOGUE_BY_LOWERCASE and
KEYS_BY_NORMALIZED both built a Map<normalize(key), key> from the same kind
of rows, just with different normalize functions.
Pulled both into util/aliasCatalogue.ts as buildAliasMap/buildNormalizedKeyMap,
taking accessor functions so each caller's row shape and normalization stay
exactly as they were. resolveFontAlias() and resolveColorTheme() themselves
are untouched — colorThemes.ts's resolveColorTheme() normalizes an alias
lookup differently than a key lookup (stripping underscores only for the
latter), a real difference this doesn't collapse, just the identical
map-building boilerplate around it.
Both branches of drawAttribution did the same five steps on a LabelTheme (displayName and username share that exact type) — resolve the pixel size, set font and fill style, draw the prefixed text centred, advance y by the size — differing only in which theme field and which text. Pulled into drawAttributionLine(), which returns the y its own baseline landed on so displayName's caller can still add the extra gap before username. Pixel-comparing pipeline tests (draws something in the text area, etc.) all still pass, confirming identical rendered output.
…-resolution-and-drawing # Conflicts: # src/cli/commands.ts
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.
What does this change?
Four independent dedup/cleanup passes found during a codebase review, each its own commit. Pure extractions — no behavior change, confirmed by the existing (including pixel-comparing) test suite passing unchanged throughout.
errorMessage(cause):cause instanceof Error ? cause.message : String(cause)was repeated verbatim at 10 call sites across 4 files.normalizeFontFamily()/unquoteFontFamily():resolveFontAlias(x) ?? x(7 sites) and the quote-stripping regex for a CSS font-family token (3 sites, including one inpipeline.tsthe original review missed) were each duplicated identically.util/aliasCatalogue.ts:font/catalogue.ts'sFONT_ALIASES/CATALOGUE_BY_LOWERCASEandtheme/colorThemes.ts'sCOLOR_THEME_ALIASES/KEYS_BY_NORMALIZEDbuilt the same two maps from the same{key, alias}row shape — one inline, one as its own function. Shared via accessor functions so each caller's row shape and normalization (they differ subtly) stay exactly as they were;resolveFontAlias()/resolveColorTheme()themselves are untouched.drawAttributionLine():pipeline.ts'sdrawAttribution()did the same five-step draw (resolve size, set font/fill style, draw prefixed text, advance y) for bothdisplayNameandusername— they share the exact sameLabelThemetype.Checklist
npm run cipasses (Biome lint and format)npm run typecheckpassesnpm run testpassesnpm run build && npm run check:buildpasses