Repository navigation
feat(i18n): persistir el idioma de la interfaz e inventario reproducible de textos (L1 #66) - #152
Conversation
Add `locale: "es" | "en"` to AppConfig as the first L1 step of the bilingual UI. Missing keys and clean starts default to `es`; values are trimmed and lowercased once, and anything outside es|en sanitizes to es. A null or non-string `locale` is read leniently so it can no longer make serde reject the whole config.json and drop the user's settings back to defaults.
…ev6#66) Generate docs/i18n-inventory.md from a git commit instead of keeping a hand-made list, so every path:line can be checked against that commit. React/TS is parsed with the TypeScript compiler; Rust with a lexical scanner that skips comments, attributes, log macros and #[cfg(test)] items. Inclusion is decided mostly by context (JSX text, visible attributes, meter units) rather than by the shape of the string. `--check` regenerates in memory and fails when the document is stale. The generator tests run as part of `npm test`.
egdev6
left a comment
There was a problem hiding this comment.
@egdev6 @danielgap — revisé el PR completo contra main @ 3864613. El cambio es sólido: verifiqué que el bug del deserializador tolerante es real (load() en config.rs:554 hace .ok() sobre from_str, así que un locale de tipo inesperado tiraba todo config.json a Default), los 6 tests de config.rs cubren la matriz completa incluyendo roundtrip con preservación de datos, los plurales declarados coinciden con el código (nine_router.rs:426, reset-credits.ts:9, Settings.tsx:1538), y el inventario reproducible cumple la condición path:line de la revisión de L0. También destaco la corrección honesta de las cifras de L0 (803/984 vs ~700/850) con el superconjunto y el gap conocido documentados.
Aprobado, con la petición de resolver estas observaciones menores antes del merge:
-
Regenerar el inventario justo antes de fusionar. El doc está generado desde
541fc6e(último commit del branch tocandosrc/), no desdemain. Al fusionar, el "último commit tocando src" cambia y--checkpodría marcar drift en el primer re-run. Regenerar el doc en la merge (o en un commit final del branch) para que el documento quede anclado a un commit verificable desdemain. -
Test de paridad del contrato de locales.
LOCALES/LOCALE_DEFAULT(Rust) yLocale/'es'(TS) duplican el contrato sin nada que los mantenga sincronizados. Para L1 es aceptable como está, pero pediría al menos un comentario cruzado en ambos lados apuntando al otro (Rust ← types.ts ya lo tiene; falta el inverso enconfig.rs), y dejar apuntado cubrir la paridad en L2 con los catálogos. -
Enganchar
--checka CI. Ahora el drift del inventario solo se detecta ennpm testlocal. Bastaría añadirnode scripts/i18n-inventory.mjs --checkal job de CI existente (o dejarlo dentro denpm test, que ya corre en CI — en ese caso basta con confirmarlo aquí).
Ninguna es bloqueante por sí misma; la 1 es la única con efecto real post-merge. Con eso resuelto y las otras 2 anotadas, listo para merge y para arrancar L2.
# Conflicts: # package.json
…ev6#66) The header recorded the last commit touching src/, so a squash or rebase merge would change that id and make `--check` fail with no source change. Record the content-addressed trees of src/ and src-tauri/src/ instead; they survive history rewrites and are checkable with `git rev-parse <commit>:src`.
|
@egdev6 gracias por la revisión. Ya están resueltas las tres, más el conflicto con Conflicto. Integré 1. Inventario anclado. Tenías razón, y el problema era algo mayor: si la PR entra con squash o rebase, el commit de la cabecera deja de existir en Se comprueba con 2. Contrato de locales. Añadí la referencia inversa en 3. Mi propuesta es dejarlo fuera de CI hasta L5. Cada PR de i18n lo regenera y lo verifica con Comprobaciones tras los cambios: |
egdev6
left a comment
There was a problem hiding this comment.
@egdev6 @danielgap — re-revisé los commits 3ee9595 y 7fe3717: las tres observaciones menores quedaron resueltas, y la del inventario con una solución mejor que la que propuse.
- Inventario anclado a árboles (punto 1): en vez de regenerar en la merge, el generador ahora ancla el doc a los árboles content-addressed (
git rev-parse <sha>:src/:src-tauri/src). El header dice "from the source treessrc/=603840f…", así que el--checkes inmune a commits que no toquensrc/— el drift post-merge desaparece de raíz, no solo se gestiona. El test de drift dei18n-inventory.test.mjscubre el comportamiento. - Contrato cruzado (punto 2):
config.rsahora referenciatypes.ts › Localecon la nota de que la paridad se testeará junto a los catálogos de L2. Queda apuntado donde toca. --checken CI (punto 3): confirmado —npm testincluye el test de drift y corre en CI.
Con esto el PR queda aprobado sin reservas: el deserializador tolerante resuelve el bug real de pérdida de config.json, los 6 tests cubren la matriz completa con roundtrip, los plurales están verificados y el inventario cumple el compromiso path:line de L0 — con la corrección honesta de cifras incluida.
Gracias por la calidad del trabajo, @danielgap: es exactamente el tipo de contribución que hace fácil mantener el proyecto — tests primero, alcance declarado, y hasta la autocorrección de las cifras de L0 documentada en lugar de oculta. Listo para merge, y con esto L2 (catálogos) puede arrancar con la base bien puesta. 👏
|
@egdev6 ¡gracias! Una sola aclaración sobre el punto 3, para que no quede una garantía que no existe: |
|
@egdev6 @danielgap — tienes toda la razón y mi afirmación anterior era incorrecta: los tests de Propuesta aceptada: abrimos tarea específica y el merge de este PR no se bloquea → #155 recoge el paso Con esto el PR queda listo para merge sin pendientes. |
Related issue / Issue relacionada
Refs #66 (la issue sigue abierta hasta L5).
ODD: fase L1 — locale persistido, con las decisiones A y B de L0 aprobadas en #66 (comment).
Scope / Alcance
1.
locale: "es" | "en"enAppConfig(src-tauri/src/config.rs,src/lib/types.ts)config.json)eses; el resto de la config no cambia"en","EN"," en "en(se recortan espacios y se pasa a minúsculas una sola vez)"fr","en-US","es_ES",""," "esnullo tipo no string (42,true,["en"],{…})es, conservando el resto de la configSobre el matiz 2: con un
locale: Stringnormal, unnullo un tipo inesperado hacía que serde rechazara todoconfig.json, yload()volvía a los valores por defecto (cuentas, dock y avisos perdidos). Un deserializador tolerante lee esos casos como vacío ysanitized()los lleva aes. No hay migración: las configs actuales cargan igual. El frontend ya reenvía la config completa ensave_config, así que el campo se conserva al guardar desde Ajustes.2. Inventario reproducible (
scripts/i18n-inventory.mjs→docs/i18n-inventory.md)Sobre el matiz 1: en vez de adjuntar una lista hecha a mano, el inventario se genera desde un commit de git, así que cada
path:linese puede contrastar con ese commit.#[cfg(test)].node scripts/i18n-inventory.mjs --checklo regenera en memoria y falla si el documento está desactualizado. Funciona igual con checkouts CRLF.Corrección de cifras de L0. Al preparar este PR comprobé el inventario que resumí en #66 y sus referencias de línea no eran fiables (muchas desplazadas varias líneas). Por eso lo rehíce con el script. Cifras nuevas: 803 textos distintos / 984 sitios (React 553/651, Rust 279/333), frente a los ~700/~850 que publiqué. Es un superconjunto deliberado:
format.ts('Semana · todos'…), que son justo el emparejamiento por literal que hay que eliminar."req"dentro de una tupla enproviders/nan.rs.3. Plurales (matiz 3) — verificados en
main@3864613. Los dos casos:src-tauri/src/providers/nine_router.rs:426—format!("{providers} proveedor{}", if providers == 1 { "" } else { "es" })src/lib/reset-credits.ts:9—count === 1 ? '1 reset disponible' : \${count} resets disponibles``Además
src/Settings.tsx:1538(` · ${n + 1} cuentas`) solo aparece con ≥ 2 cuentas, así que no tiene rama singular.En inglés también son 2 formas (one/other), así que basta con ramificar en el catálogo sin
Intl.PluralRules.Fuera de alcance: catálogos, selector en Ajustes, ids estables Rust→JS y cualquier texto visible (L2/L3). Sobre el matiz 4,
es.tssaldrá de los literales actuales listados en el inventario, sin retraducir.src-tauri/src/config.rsLOCALES,LOCALE_DEFAULT, deserializador tolerante, saneo y 6 testssrc/lib/types.tsLocaleyAppConfig.localescripts/i18n-inventory.mjs--checkscripts/i18n-inventory.test.mjsdocs/i18n-inventory.mdpackage.jsonnpm testCoordination / Coordinación
Contribuidor: @danielgap
Evidencia de asignación o acuerdo: #66 asignada a @danielgap ("Adelante! toda tuya!") y L0 aprobado en #66 (comment)
Acceptance and validation / Aceptación y validación
path:lineverificable con--check.cargo test: 230/230. Los 6 tests nuevos deconfig::testsfallaron antes de implementar, conlocaleenNull.npx vitest run --dir src: 591/591.npx tsc --noEmit: OK.node --test scripts/i18n-inventory.test.mjs: 11/11.node scripts/i18n-inventory.mjs --check: exit 0, también con CRLF.npm testcompleto en Windows: 216/220 en los scripts de Node. Los 4 fallos sonEPERMal crear symlinks ensetup-signingyprovision-tools, sin privilegio de symlink en Windows; ya fallaban en un árbol limpio y no tienen relación con este cambio.