Skip to content

fix(desktop): add German (de) desktop locale with review fixes - #71161

Open
Karaboja98 wants to merge 3 commits into
NousResearch:mainfrom
Karaboja98:fix/de-locale-clean
Open

Karaboja98 wants to merge 3 commits into
NousResearch:mainfrom
Karaboja98:fix/de-locale-clean

Conversation

@Karaboja98

Copy link
Copy Markdown

What does this PR do?

Adds German (de) as a sixth Hermes Desktop locale. German is now available in Settings → Appearance → Language and maps to display.language: de.

The locale uses defineLocale() so any missing desktop-only keys fall back to English while remaining type-checked against the shared Translations contract.

Related Issue

Supersedes #67077 (same author, cleaned up scope and additional fixes).

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)

Changes Made

New files

  • apps/desktop/src/i18n/de.ts — German desktop UI copy (2613 lines, defineLocale())
  • apps/desktop/src/i18n/de.test.ts — corruption guard + semantic regression tests

Registration

  • types.ts — added de to Locale union
  • languages.ts — added LOCALE_OPTIONS entry + aliases: de, de-de, de_de, de-at, de_at, de-ch, de_ch, deutsch, german
  • catalog.ts — imported and registered de in TRANSLATIONS

Tests

  • languages.test.ts — normalization coverage for all de aliases including de-AT/de-CH
  • context.test.tsxloads de from display.language config test
  • de.test.ts — 10 tests covering boot copy, keybind labels, import/export collision, Starmap import semantics (5 distinct values), singular/plural resume, corruption guard, plus regression guards for the 3 copy-paste bugs found during review

Documentation

  • DESIGN.md — updated locale list to five, noted defineLocale() fallback behavior

Review fixes (from #67077 sweeper + follow-up audit)

Issue Before After
Starmap import: 5x Kopiert! all mapped to same string 5 distinct German semantics
resumeWhenBackgroundDone(1) literal ${count} in quotes count-free singular form
de-AT / de-CH aliases missing, fell back to English registered in LOCALE_ALIASES
settings.config.imported copy-paste of autosaveFailed Konfiguration importiert
settings.appearance.importedBadge copy-paste of removeTheme Importiert
about.justNowSuffix untranslated English gerade eben
composer.queue missing (fell back to English) Nachricht in Warteschlange
petdex capitalization lowercase in unreachable Petdex (proper noun)
backendOutOfDateMessage ambiguous pronoun die Versionen
Trailing newline missing in de.ts added

How to Test

  1. cd apps/desktop && npx tsc --noEmit -p tsconfig.json → 0 errors
  2. cd apps/desktop && npx vitest run src/i18n --environment jsdom → 39 tests pass
  3. cd apps/desktop && npm run build → exit 0
  4. Manually: Settings → Appearance → Language → Deutsch → verify overlays/UI render German

Verification performed

  • npx tsc --noEmit -p tsconfig.json — 0 errors
  • npx vitest run src/i18n --environment jsdom — 39/39 passed
  • npm run build — successful, assert-dist-built passed
  • git diff --stat origin/main — only 8 expected i18n files touched (clean scope isolation)

- Add missing composer.queue translation ('Nachricht in Warteschlange')
- Fix settings.config.imported: was 'Automatisches Speichern fehlgeschlagen', now 'Konfiguration importiert'
- Fix settings.appearance.importedBadge: was 'Design entfernen', now 'Importiert'
- Fix about.justNowSuffix: was '· just now', now '· gerade eben'
- Fix Petdex capitalization (proper noun) in appearance.pet.unreachable
- Fix backendOutOfDateMessage: clarify pronoun reference
- Add trailing newline to de.ts
- Add regression tests for all fixes in de.test.ts
- Update DESIGN.md: document five locales and defineLocale() fallback

Verification:
- tsc --noEmit: passed (0 errors)
- vitest src/i18n: 39/39 tests passed
- npm run build: successful

Closes NousResearch#67077
@Karaboja98

Copy link
Copy Markdown
Author

Replaces #67077 with a clean scope and additional fixes.

Scope isolation: only 8 i18n files touched (verified via git diff --stat origin/main). The original PR had 2101 files of drift from a stale branch — this branch was built fresh from origin/main.

Beyond the original sweeper fixes, this PR also addresses:

  • settings.config.imported was a copy-paste of autosaveFailed (both showed "Automatisches Speichern fehlgeschlagen" on successful config import)
  • settings.appearance.importedBadge was a copy-paste of removeTheme (both showed "Design entfernen")
  • about.justNowSuffix was left untranslated as · just now
  • composer.queue keybind action was missing (fell back to English "Queue message")
  • petdexPetdex (proper noun capitalization)
  • backendOutOfDateMessage: ambiguous pronoun "sie" → "die Versionen"
  • Trailing newline added to de.ts

Each fix has a regression test in de.test.ts so they cannot recur silently.

Verification (all green on this branch):

  • npx tsc --noEmit -p tsconfig.json → 0 errors
  • npx vitest run src/i18n --environment jsdom → 39/39 passed
  • npm run build → exit 0, assert-dist-built passed

Happy to adjust wording or split into separate commits if preferred.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) area/i18n Localization, locales, translations sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades duplicate This issue or pull request already exists labels Jul 25, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #51762: both open PRs add the same Desktop German locale, catalog registration, type union, and picker aliases. This PR's review-cleanup coverage is useful context for the canonical implementation; it also replaces the author's earlier #67077 attempt.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused locale addition and the registration/tests work. The feature is still absent from current main (apps/desktop/src/i18n/types.ts:8, catalog.ts:8-14), but a few visible strings remain untranslated.

Problems

  • apps/desktop/src/i18n/de.ts:1364-1366, :1380, and :1477-1478 are English profile/cron destructive-dialog fragments. They render directly in apps/desktop/src/app/profiles/delete-profile-dialog.tsx:30-34 and apps/desktop/src/app/cron/index.tsx:548-550.
  • apps/desktop/src/i18n/de.ts:1945 returns English installer-stage text. The current test walker in de.test.ts:72-90 does not execute function-valued translations, so it cannot catch this class.

Suggested changes

  • Translate those remaining fragments/functions while retaining their interpolation boundaries.
  • Add focused assertions for the assembled destructive descriptions and de.install.currentStage('…').

This is an automated hermes-sweeper review.

Comment thread apps/desktop/src/i18n/de.ts Outdated
Comment thread apps/desktop/src/i18n/de.ts Outdated
Comment thread apps/desktop/src/i18n/de.ts Outdated
Comment thread apps/desktop/src/i18n/de.ts Outdated
@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 30, 2026
Address teknium1 review comments on PR NousResearch#71161:

- Translate profiles.deleteDescPrefix/Mid/Suffix (delete-profile-dialog)
- Translate profiles.renameDescPrefix (rename dialog)
- Translate cron.deleteDescPrefix/Suffix (cron delete dialog)
- Translate install.currentStage() function (installer stage text)
- Add focused regression tests for all translated fragments,
  including direct assertions for assembled destructive descriptions
  and de.install.currentStage() — the generic leaf walker does not
  execute function-valued translations.

Verification:
- tsc --noEmit: 0 errors
- vitest src/i18n: 43/43 tests passed (4 new tests added)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/i18n Localization, locales, translations comp/desktop Electron desktop app (apps/desktop/*) duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants