feat: ✨ show every locale's value in the translation hover - #287
Merged
mikebronner merged 1 commit intoAug 25, 2026
Merged
Conversation
The translation hover hardcoded locale "en": a project maintaining
de + en catalogues only ever saw the English value, and a key defined
solely in a non-English locale rendered as "not found".
Discover the locales that could define the key — the locale directories
(and {locale}.json catalogues) of the key's lang dir(s): the published
vendor override plus the registered namespace dir for namespaced keys,
the project lang dirs otherwise — resolve the key per locale, and render
one value line per defining locale, each with its own source link:
`failed_title`
**de** — “Analyse fehlgeschlagen”
[lang/de/contract.php]
**en** — “Analysis failed”
[lang/en/contract.php]
Locales that don't define the key are skipped; when none does, the
existing not-found trailer renders unchanged.
Contributor
|
I like this idea, although I'm worried if apps have 10s of different translations, I guess its ok if the hover is scrollable, but that's really up to Zed, and I believe the code hover for other areas already scrolls if there is a lot of content. I'll give this a once-over before merging, as well. |
8 tasks
mikebronner
approved these changes
Aug 25, 2026
mikebronner
left a comment
mikebronner
left a comment
Contributor
There was a problem hiding this comment.
Approved — thanks for this. Hardcoding en was a real gap, and a de + en project seeing both values at once is a clear improvement.
Verified locally: full suite green (2,927 tests), cargo clippy --all-targets clean, cargo fmt --check clean. The no-root fallback, the vendor exclusion, the locale dedup, and the unpublished-vendor-map path all check out.
I'm merging this as the hover half of the change and tracking the remainder in #288. The main one worth flagging: go-to-definition (main.rs:16057) and diagnostics (main.rs:15032) still hardcode en, so a key defined only in de now shows in the hover but still won't navigate, and still gets flagged as missing. Not a regression from this PR — it just means the underlying problem is fixed in one of three places. #288 covers that plus a few smaller items (test coverage on the locale discovery, the "default locale" trailer wording, and resources/lang being discovered but never read by the resolver).
No changes needed here.
mikebronner
approved these changes
Aug 25, 2026
This was referenced Aug 25, 2026
mikebronner
added a commit
that referenced
this pull request
Aug 25, 2026
#294) * fix: 🐛 Resolve translations against every locale, not a hardcoded "en". #287 made the hover show every locale that defines a key, but go-to-definition and diagnostics kept resolving against a hardcoded "en" across all three key shapes. A key defined only in `de` therefore rendered in the hover, refused to navigate, and was still reported missing — and the comment claiming "navigation, diagnostics, and hover all agree on where a key lives" was simply false. All three now resolve against translation_lookup::available_locales, a single shared helper that returns every locale which could define the key, ordered with the project's APP_LOCALE first and the rest alphabetically. Sharing the ordering matters as much as sharing the set: when several locales define a key, goto lands on the same one the hover leads with. Resolution is now by reading the key rather than probing for the file. The dotted and text branches previously accepted a lang file's existence as proof the key existed, so a file that had been created but never populated made diagnostics report a missing key as present and sent goto to a file that never contained it. resources/lang is wired into the resolver itself (resolve_dotted, resolve_namespaced, resolve_text_key) rather than only into discovery. Diagnostics had always checked both lang roots while the resolver checked only lang/, so on a Laravel-8-style project hover could find nothing while diagnostics resolved happily — the same divergence in the opposite direction. APP_LOCALE is read through config::read_env_value, extracted from DatabaseConfigResolver::resolve_env. That reader is deliberately horizontal to stop one variable's value swallowing the next line, a past credential-leak bug; a second hand-rolled regex would risk reintroducing it, so there is now one reader and database.rs delegates to it. The multi-locale card is rebuilt on the HoverContent template with a structured per-locale field instead of a hand-built markdown string. Source links render inline on each locale's line rather than as separate paragraphs, and when exactly one locale *resolves* the key the card collapses to the dense single-block form — most projects ship one locale and a lone value under a heading costs a line to say nothing. A distinct trailer states the key was found in no locale at all, leaving the single-locale card's "default locale" wording intact where it is still accurate. get_all_translation_keys deliberately keeps its first-wins directory selection: completion asks "what keys exist at all", which any single locale answers, and unioning every catalogue per request would read the whole project to produce the same list. Per-hover I/O is still outside Salsa and is now multiplied by locale count; that trade is tracked in #293. Fixes: #288 * fix: 🔒️ Fence locale discovery inside the project root. `available_locales` enumerated the unpublished vendor lang directory without a containment check. That directory is built from a `loadTranslationsFrom` argument in project/vendor source — untrusted input that can point anywhere — so a namespaced key could `read_dir` a tree outside the project and render whatever subdirectories it found there as that key's locale list. `resolve_namespaced_in_dir` has fenced its own read since issue #248; this enumeration is a read site too and was the one that missed the guard. It now takes the same fail-closed `path_within_root` check. Verified by removing the guard: out-of-root `ja`/`ko` directories leak straight into the returned locales. Also corrects the module header, which this branch made false: all three key shapes now resolve under both `lang/` and `resources/lang/`, and the claim that the resolver "only checks the published path" had been untrue since the vendor_map fallback landed. Drops a leftover bare block in `create_translation_location_from_salsa`. * fix: 🔒️ Fence every translation read against the project root. The `vendor::` namespace of a translation key is lifted verbatim out of parsed PHP/Blade source — including from indexed `vendor/**.php`, so a dependency can choose it. Two sites joined it into a path with no containment check: `resolve_namespaced` read `lang/vendor/{namespace}/{locale}/{file}.php`, and `available_locales` enumerated `lang/vendor/{namespace}`. An absolute or `../`-bearing namespace escaped the tree, and a hit surfaced in the hover tooltip and as a goto-definition target. Rather than patch the two reported sites, route every file read in the module through one `read_in_root` choke point that fail-closes on a path it cannot prove inside the root. That covers the dotted-key and JSON text-key reads too, which had the same shape, and leaves one place a future read site can be forgotten. `resolve_namespaced_in_dir` drops its own inline guard, now redundant. Enumeration takes the same guard, moved to the `read_dir` loop so it applies to every directory rather than only the `vendor_map` one. Five tests pin the guard, each verified to redden when it is removed: absolute and traversing namespaces in the published path, dotted-key and text-key reads through a symlink that escapes the root, and the enumeration twin of the published-path case. * refactor: ♻️ Pair a locale's translation value with its source link. translation_card_locales took each locale as (locale, Option<value>, Option<link>), which admitted a locale that resolved a value but carried no link. That state cannot arise: a value exists only because a file was read to produce it, and that file's path is exactly what the link is built from — source_link returns a String unconditionally, falling back to a backtick-quoted display path when the URL conversion fails. Pair them as (locale, Option<(value, link)>) so the shape says so. The compiler now rejects the impossible combination instead of a test pretending to cover it. Drops the fixture that exercised the valued-but-linkless case, which was documenting a scenario the resolver has no way to produce.
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.
The translation hover hardcodes locale
"en". A project maintainingde+encatalogues only ever sees the English value, and a key defined solely in a non-English locale renders as not found.The hover now discovers which locales could define the key — the locale directories (and
{locale}.jsoncatalogues) of the key's lang dir(s): the published vendor override plus the registered namespace dir for namespaced keys, the project lang dirs otherwise — resolves the key per locale, and renders one line per defining locale with its own source link:Locales that don't define the key are skipped; when none does, the existing not-found trailer renders unchanged, as does the no-root fallback.
Full test suite green.