feat(themes): an Extensions panel for browsing, installing and updating themes from Open VSX - #169
Merged
Merged
Conversation
…cking The backend half of the theme registry browser. No UI yet -- this is the part that talks to the network and owns the disk, kept as its own commit because it is independently testable and the interesting decisions all live here. STRUCTURE `commands/themes.rs` shrank to a command surface and the work moved into a `themes/` module, following `pulse/` and `json_schemas/`. The vsix reader is now generic over its reader (`read_archive<R: Read + Seek>`) because it grew a second caller: importing from disk holds a `File`, installing from the registry holds bytes in memory and must not write them out just to read them back. A test asserts both paths produce the same payload. THE REGISTRY CLIENT Four properties of the API shaped it, all measured against the live service rather than assumed: - The `Themes` category covers icon themes, and the search response says nothing about what an extension contributes -- only `contributes.themes` in the manifest does. The manifest is served on its own (1-11 KB), so filtering costs a small fetch per candidate rather than a multi-megabyte download: Material Icon Theme is discarded for 11 KB instead of 6 MB. Verified live -- a search for "dracula" returns 10 colour themes out of ~51 hits. - Responses carry no `Cache-Control` and no `ETag`, so caching is entirely ours to own. Nothing to piggyback on. - Every version publishes a `sha256` that matches its `.vsix` byte for byte, so `download_vsix` verifies it. A mismatch is a hard failure; a version with no published digest still downloads, because refusing older extensions the registry serves happily would be gating on the check rather than strengthening with it. - The registry returns intermittent 503s, so `fetch_bytes` retries with backoff and stops early on a 4xx, which is a permanent answer. `AppError::Registry` is new, for the same reason `AppError::Inference` exists: `reqwest` raises no error for an HTTP status, so a 503 produces a perfectly successful `Response` the caller must still treat as a failure. It also keeps "the registry is having a bad afternoon" apart from a malformed package, which will never succeed however many times it is retried. TWO THINGS THE FRONTEND DOES NOT GET TO DECIDE The registry URL is read from preferences inside the command, never passed in -- gotcha #71's rule for the AI endpoint applies unchanged: a caller that can name the destination can bypass the kill-switch, and a preference only the UI honours is a suggestion. And a download URL, which arrives inside a search result rather than being derived, is checked against the configured registry's origin first; otherwise "install this theme" means "fetch whatever host this record happens to name", and records go stale. STORAGE `installed_themes.json` holds the editor themes and the install metadata; the palettes stay in localStorage. The split is by shape, not size: a palette is 30 hex values read synchronously before first paint to avoid a FOUC, while a Monaco theme is ~20 KB, is what localStorage should not accumulate, and tolerates arriving late because `registerImportedMonacoThemes` defines whatever it is given whenever it is given it. `palette_edited` is the field that makes updates safe. An update can always replace the editor theme -- nobody hand-edits 275 token rules -- but a derived palette the user has touched is theirs, so `upsert` preserves the flag even when the incoming record clears it. An update that silently overwrote someone's colour work would be the worst failure this feature could have. `ThemePrefs` defaults `registry_enabled` to **true**, against the project rule that a new flag starts off, and the exception is argued in the doc comment rather than assumed: those flags are per-connection and guard a database, while nothing of the user's leaves the machine here -- anonymous GETs for public packages, no credentials, no telemetry, no schema. VERSION COMPARISON, AND A BUG THE TESTS CAUGHT `is_newer` first split on `.`, `-` and `+` together, which makes two correct rules contradict each other: a missing component makes a version older (`1.2` < `1.2.1`) while a missing prerelease tail makes it newer (`1.2.0` > `1.2.0-beta`). The release-beats-prerelease case lost. Core and prerelease tail are now split first and compared under their own rules. It is deliberately not a strict semver parse: extension versions are `major.minor.patch` by convention, not by rule, and "cannot parse" must never resolve to "no update available", which is a silent failure nobody notices. TESTS 30 unit tests, plus four `#[ignore]`d live ones against open-vsx.org for the part only the real service can answer -- that the response still has the fields this module reads, that manifests are still served separately, and that the published digest still matches. Run them with: cargo test --lib themes::registry::tests::live -- --ignored All four pass today: 471 KB downloaded and verified, icon themes filtered out, an absent extension reported as absent rather than as an error. Authored by Alex Lopez (Alexfp28) <alexlopezdelafuente@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ng themes The user-facing half of the Open VSX browser, on top of the registry client from the previous commit. A PANEL, NOT A DIALOG -- WHICH WAS THE FIRST ATTEMPT AND WAS WRONG TWICE The browser started as a `workbench`-tier dialog opened from Settings > Appearance. Alex called it, and the objection holds on both counts: - Installing anything meant three stacked modals. The variant picker cannot be avoided -- an extension contributes several themes and nothing says which light and dark variants are counterparts -- so the stack was settings + browser + picker, which is a miserable way to press two buttons. - The only way in was a small `IconButton` in the theme list's header, the same shape of affordance that had already made the local `.vsix` import hard to find. I had even written that about the import button and then built the same mistake next to it. It is now the fourth occupant of the right dock, alongside Saved queries, Pulse and the AI panel, following that dock's existing contract exactly: deferred mount on first selection, kept mounted afterwards, and `active` -- not mountedness -- gates fetching, so someone who never opens it never issues a request. `RightPanelId` gains `extensions` and the activity bar a labelled button. The variant picker is now the only modal in the flow. The search runs once per session rather than on every activation: switching the dock back to a panel that already holds results would re-spend someone else's bandwidth to redraw what is on screen. WHAT THE PANEL DOES Search, with paging. Each row carries what actually decides an install: author, version, licence, download count, rating and how many variants the extension contributes. Installed themes are marked, the active one is ticked, and a theme already installed offers Apply as well as Reinstall. Updates are listed at the top when the registry has newer versions, and this is where `paletteEdited` earns its place: a theme whose derived palette you have edited has only its editor theme refreshed, and the panel says so before you press anything rather than after. Two visibly different outcomes, never one button that quietly behaves differently. STORAGE MOVED, BY SHAPE RATHER THAN SIZE `importedEditorThemes` leaves `partialize` and `installed_themes.json` becomes the source of truth. The palettes stay in localStorage because 30 hex values have to be read synchronously before first paint or the app flashes the wrong theme; a Monaco theme is ~20 KB, is what localStorage should not accumulate once a registry can install a dozen, and tolerates arriving late -- `registerImportedMonacoThemes` defines whatever it is handed whenever it is handed it, so an editor that mounted first is repainted rather than broken. `hydrateInstalledThemes` runs from an ordinary `useEffect` in `App.tsx` for that reason, and migrates anything a pre-split build left stranded in localStorage rather than letting it silently disappear. A MANIFEST CACHE, WHICH IS MANNERS RATHER THAN SPEED Filtering one page of results costs 25 requests, and the registry publishes neither `Cache-Control` nor `ETag`, so there is nothing to revalidate against and no shared cache doing this for anyone. Without a local one, paging back or retyping a query re-fetches everything from Eclipse's free infrastructure. Memory-only, 15-minute TTL, and failures are never cached -- the 503s are transient, and remembering one would turn a blip into fifteen minutes of a theme mysteriously missing from the results. `OnceLock` rather than `LazyLock`: the latter is stable since 1.80 and this crate's MSRV is 1.77, which `clippy::incompatible_msrv` caught. SETTINGS Settings > Appearance grows a Theme registry group: a switch and a URL. The URL is for the installs that are not on open-vsx.org -- a company's own Open VSX instance -- or not on any network, and it is recorded per installed theme too, so changing it never re-targets an existing theme's updates. Documented as ADR gotcha #88, including the panel-versus-dialog reasoning. ROADMAP item #9 closes; `CLAUDE.md`'s on-disk state map gains `installed_themes.json`. Both changelogs updated. Authored by Alex Lopez (Alexfp28) <alexlopezdelafuente@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…croll back Three things, all reported by Alex against the running app. CARDS The previous row was six facts on six equal-weight lines, which scans as a paragraph rather than as a list of choices. It is now a three-band card: identity (icon, name, publisher/version/licence), description at full width where it has room for two lines, then metrics and the action sharing the last band. In a ~340px panel everything competes for the same space, so deciding what reads first is the whole job. **The published icon is now shown**, which the registry has had all along and the card ignored -- `RegistryTheme.icon_url` was already populated. An extension that publishes none gets a monogram on a hue derived from its identifier, so the same extension is always the same colour; a fixed grey would have read as a broken image rather than as a deliberate placeholder. `onError` falls back to the same monogram, because the icon is a third-party URL and a 404 would otherwise leave a broken-image glyph in a list where every other row has artwork. State is carried by the border rather than by badges competing with the name: the active theme takes the brand accent (the app's own "this is live" colour), an installed one a quieter success border, and neither adds a row. DOWNLOAD COUNTS ARE COMPACT `842,877` is six characters of precision nobody acts on, sharing one line with a rating and a variant count. `Intl.NumberFormat` with `notation: "compact"` and `maximumSignificantDigits: 3` gives `843K` / `1.45M` in English and `843 mil` / `1,45 M` in Spanish, with each locale's own separators -- better than a hand-rolled K/M helper, which would have been English-only. APPEARANCE HAD NO SCROLL, AND I BROKE ITS PROPORTIONS Adding the Theme registry group made it three sections dividing one fixed height: a `flex-1` colour editor and two `shrink-0` groups. The editor collapsed to roughly the height of its own header, so the thing the page exists for became the smallest thing on it. It now scrolls, and the editor states a `24rem` floor. That the layout only worked with exactly two children was pre-existing and latent; adding the third is what surfaced it. A DOC CORRECTION WORTH ITS OWN NOTE While pulling real cards for a preview I found that **a search hit carries no `files.manifest`** -- only the per-extension metadata response does. The code is fine: the `unwrap_or_else` path to `/latest/file/package.json` is what every result has actually been taking, which is why the live tests pass. But the module header, the ADR and the ROADMAP all described `files.manifest` as the route, so anyone reading them would have believed the fallback was the rare case rather than the only case. Corrected in all three. Authored by Alex Lopez (Alexfp28) <alexlopezdelafuente@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ys in tests
The footer shipped referencing `common.previous` and `common.next`, which I
never added. i18next's fallback is to render the key, so the panel had two
buttons reading "common.previous" and "common.next", and the sentence beside
them truncated to "unos 1883 resulta…" because it never fit 340px either.
THE FOOTER
Chevrons and a page counter instead of two word buttons and a sentence:
[<] Página 2 de ~79 [>]
The `~` carries what the sentence was for. The registry's total counts the icon
themes this list filters out, so any count derived from it is an upper bound --
the approximation had to survive the rewrite, just not as thirteen words.
THE REAL PROBLEM, WHICH IS THAT NOTHING CAUGHT IT
A missing key is invisible to every check this repo has. TypeScript does not
type `t()`'s argument, no test mounts every component, and the fallback renders
the key rather than throwing -- so it ships as a word-shaped string in a button
and is found by whoever happens to look. That is how two of them reached a
build.
`src/lib/i18n/keys.test.ts` extracts every literal `t("…")` from the source and
asserts three things: the key resolves in English, it resolves in Spanish
(separately, so a half-translated addition names the language), and the two
locales carry the same key set. Plural keys resolve through their `_one` /
`_other` suffixes, and keys built from a variable are skipped -- they cannot be
resolved statically and guessing the suffixes would either miss cases or invent
them.
A fourth test asserts the extraction finds more than 500 call sites, because a
regex that silently stopped matching would make the other three pass by
checking nothing.
Verified by reintroducing the bug: with one invented key the suite fails and
names both the key and the file it is in.
Authored by Alex Lopez (Alexfp28) <alexlopezdelafuente@gmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three problems Alex hit installing a theme from the panel, of which the third is a real bug and the other two are a flow that asked for things it already knew. INSTALLING NO LONGER ASKS WHICH VARIANTS The variant picker was compulsory, which turned a one-click action into three steps to answer a question most extensions do not pose: the answer is "the light one and the dark one". `autoPairVariants` now takes the first contribution on each side and installs. First, because a manifest's order is the author's own -- `GitHub Light Default` precedes its high-contrast and colourblind siblings, `Dracula Theme` precedes `Dracula Theme Soft` -- so the first is what the author leads with. A side with no contribution stays undefined and `buildThemeImport` duplicates the other, which is the honest fallback for a dark-only extension. The picker is still one click away per row, shown only when there is more than one variant to choose between, for the person who wants Gruvbox Dark Hard rather than Medium. An update always goes through the recorded pairing rather than re-guessing, falling back to the automatic one when a variant no longer exists in the new version. THE NAME FIELD IS GONE The package already carries a name. Asking the user to restate a correct value before they can continue is a step that buys nothing, and renaming a theme is what Settings > Appearance is for. AN INSTALLED THEME NOW RECOGNISES ITSELF This one was a genuine bug, and one I had already flagged as fragile in the PR that introduced it -- which is the part worth noting: "documented as fragile" is not the same as "acceptable", and it should have been fixed then rather than described. The panel matched an installed family to a search row by comparing display names. That fails two independent ways: the name recorded at install time comes from the *package manifest* while the row shows the *registry's* `displayName`, and nothing requires those to be equal; and renaming a theme afterwards made it stop matching itself. So a freshly installed theme showed as not installed. The join key -- `namespace`/`name` -- was in `installed_themes.json` the whole time and simply was not reaching the frontend. `ImportedEditorThemes.source` now carries it, hydrated from disk alongside the editor themes. `findInstalledFamily` moved to `lib/vscodeTheme/` so it can be tested without mounting anything: it is a join between two stores, not a rendering concern. Eight new tests cover the pairing and the matching, including the two cases the old implementation got wrong (manifest name differing from registry name, and a renamed theme). Authored by Alex Lopez (Alexfp28) <alexlopezdelafuente@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sting A theme dragged in as a `.vsix` had no registry origin, so the panel offered it as if it were not installed and never checked it for updates. Both halves are now covered by recording what the package says about itself, independently of where it came from. TWO FIELDS, AND WHY THEY ARE NOT `ThemeSource` `InstalledTheme::identifier` holds the manifest's `publisher.name` and `InstalledTheme::version` the package version, for *every* install. They are separate from `source` because the two answer different questions: `source` is "where did this come from", which a file import cannot answer, while these are "which extension is this and which version" -- which the file answers perfectly well. Collapsing them would have meant writing a fake registry origin for a theme that has none. MEASURED BEFORE BEING RELIED ON The match only works if a manifest's `publisher.name` equals the registry's `namespace.name`. That is a convention, not a rule, so it was checked against the live registry across thirteen colour themes: identical every time, no case-only differences. Compared case-insensitively anyway -- a namespace like `GitHub` may be capitalised differently in one place than the other, and nothing enforces agreement. UPDATES, WHICH IS THE HALF THAT NEEDED THE VERSION Without a recorded version there is nothing to compare a registry's latest against, so a hand-imported theme would have shown as installed while silently never updating -- a worse state than not being recognised at all, because it looks handled. `check_theme_updates` now resolves each theme to a (registry, namespace, name, current version) tuple: a registry install answers all four from its own record, a local import answers the last three from its manifest and borrows the registry from preferences. "Not found" is an ordinary answer for a theme the configured registry does not carry, and is not counted as an error. `upsert` preserves both new fields when an incoming record does not know them, the same protection `palette_edited` already had: a caller that knows less must not erase what is stored. Four frontend tests and one Rust test cover it, including the near-miss the naive version would have accepted (`dracula-theme.theme` matching `dracula-theme.theme-dracula`). Authored by Alex Lopez (Alexfp28) <alexlopezdelafuente@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Authored by Alex López (Alexfp28)
<alexlopezdelafuente@gmail.com>.Fase 3 de la idea de David: el navegador de open-vsx dentro de la app. Cierra el item #9 del ROADMAP.
Un panel, no un diálogo
Lo construí primero como diálogo desde Ajustes → Apariencia y Alex lo corrigió a media implementación, con razón en los dos puntos:
IconButtonpequeño en la cabecera de la lista de temas — exactamente el tipo de botón que nadie encuentra. Yo mismo había escrito eso del botón de importar y construí el mismo error al lado.Ahora es el cuarto ocupante del dock derecho, junto a Consultas guardadas, Pulse y el panel de IA, siguiendo su contrato tal cual: montaje diferido en la primera selección, se mantiene montado después, y lo que controla las peticiones es
active, no estar montado. El selector de variantes pasa a ser el único modal del flujo.Qué hace
Búsqueda con paginación. Cada fila lleva lo que de verdad decide una instalación: autor, versión, licencia, descargas, valoración y cuántas variantes aporta. Los instalados se marcan, el activo lleva un tick, y uno ya instalado ofrece Aplicar además de Reinstalar.
Las actualizaciones salen arriba, y ahí es donde
paletteEditedse gana su sitio: a un tema cuya paleta hayas editado solo se le actualiza el tema del editor, y el panel lo dice antes de que pulses, no después.Decisiones que no son fontanería
Themesy su búsqueda no los distingue — solo el manifiesto. Se descarga ese manifiesto (1–11 KB), no el paquete: descartar Material Icon Theme cuesta 11 KB en vez de 6 MB. Por eso el recuento es aproximado y el panel dice «unos».sha256publicado. Un desajuste es fallo duro; una versión sin digest se instala igual, porque rechazarlas sería condicionar a la comprobación en vez de reforzar con ella.download_url, que viene dentro de un registro y no derivada, se valida contra el origen configurado (los casos trampaopen-vsx.org.evil.exampleynotopen-vsx.orgestán fijados con tests).ETagniCache-Control. Sin caché local, paginar hacia atrás re-descarga todo de la infraestructura gratuita de Eclipse. En memoria, TTL de 15 min, y nunca se cachea un fallo.localStorage(30 hex leídos síncronos antes del primer pintado, si no hay FOUC); los temas Monaco (~20 KB) van ainstalled_themes.json, porque toleran llegar tarde.Ajustes
Ajustes → Apariencia gana un grupo Registro de temas: un interruptor y una URL, para instalaciones con instancia propia de Open VSX o sin salida a internet. La URL se guarda además por tema instalado, así que cambiarla no redirige las actualizaciones de lo ya instalado.
registryEnabledviene activado por defecto, contra la convención del repo — argumentado en el ADR: esos flags son por conexión y protegen una base de datos, y aquí no sale nada del usuario (peticiones anónimas de paquetes públicos, sin credenciales, telemetría ni esquemas).Verificación
#[ignore]:cargo test --lib themes::registry::tests::live -- --ignored. Los cuatro pasan — 471 KB descargados y verificados, temas de iconos filtrados, extensión inexistente reportada como ausente.tsc --noEmitlimpio,cargo fmt+clippy -D warningslimpios.Documentado como ADR gotcha #88.
CLAUDE.md,ROADMAP.mdy ambos changelogs actualizados.🤖 Generated with Claude Code