Migrate to Slang#344
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (3)
⛔ Files ignored due to path filters (4)
⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (3)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAll 20 Dart files across app sources and components systematically update their localization dependencies by removing imports of ChangesLocalization import migration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces generated localization files for 39 locales using the slang package. Feedback indicates several critical issues that will prevent the project from compiling: the base English translation class is missing nearly all keys present in other locales, and it incorrectly includes dependency information as translation data. Additionally, multiple non-English locale files contain untranslated English strings and lack the required pluralization forms for languages like Arabic and Czech. Finally, the Esperanto locale is misconfigured with an incorrect language code.
| class Translations with BaseTranslations<AppLocale, Translations> { | ||
| /// Returns the current translations of the given [context]. | ||
| /// | ||
| /// Usage: | ||
| /// final t = Translations.of(context); | ||
| static Translations of(BuildContext context) => InheritedLocaleData.of<AppLocale, Translations>(context).translations; | ||
|
|
||
| /// You can call this constructor and build your own translation instance of this locale. | ||
| /// Constructing via the enum [AppLocale.build] is preferred. | ||
| Translations({Map<String, Node>? overrides, PluralResolver? cardinalResolver, PluralResolver? ordinalResolver, TranslationMetadata<AppLocale, Translations>? meta}) | ||
| : assert(overrides == null, 'Set "translation_overrides: true" in order to enable this feature.'), | ||
| $meta = meta ?? TranslationMetadata( | ||
| locale: AppLocale.en, | ||
| overrides: overrides ?? {}, | ||
| cardinalResolver: cardinalResolver, | ||
| ordinalResolver: ordinalResolver, | ||
| ) { | ||
| $meta.setFlatMapFunction(_flatMapFunction); | ||
| } | ||
|
|
||
| /// Metadata for the translations of <en>. | ||
| @override final TranslationMetadata<AppLocale, Translations> $meta; | ||
|
|
||
| /// Access flat map | ||
| dynamic operator[](String key) => $meta.getTranslation(key); | ||
|
|
||
| late final Translations _root = this; // ignore: unused_field | ||
|
|
||
| Translations $copyWith({TranslationMetadata<AppLocale, Translations>? meta}) => Translations(meta: meta ?? this.$meta); | ||
|
|
||
| // Translations | ||
| late final TranslationsDependenciesEn dependencies = TranslationsDependenciesEn._(_root); | ||
| } |
There was a problem hiding this comment.
The base Translations class is missing almost all the translation keys (e.g., invalidURLForSource, noReleaseFound, primary, etc.) that are present in other locale files like strings_ar.g.dart. Since AppLocale.en is configured as the baseLocale in AppLocaleUtils, this class defines the interface that all other translations must implement. As it stands, the project will fail to compile because the other generated classes (like TranslationsAr) are attempting to override members that do not exist in the base class.
Additionally, the English locale contains a dependencies key that is missing from other locales, which is another source of compilation errors. It appears that slang might have processed an incorrect file (possibly containing dependency info) as the source for the English locale. Please ensure your base translation file (e.g., assets/translations/strings_en.i18n.json) is correctly populated with all keys.
| @override String appRemoved({required num n}) => (_root.$meta.cardinalResolver ?? PluralResolvers.cardinal('ar'))(n, | ||
| one: 'Removed one app', | ||
| two: 'Removed {count} apps', | ||
| other: 'Removed {count} apps', | ||
| ); |
There was a problem hiding this comment.
The pluralization strings for appRemoved are in English instead of Arabic. Furthermore, Arabic requires more plural forms (zero, few, many) to be grammatically correct, but only one, two, and other are provided here. This suggests the source translation file for Arabic is incomplete and relies on a fallback structure that doesn't match the target language's requirements.
| @override String appRemoved({required num n}) => (_root.$meta.cardinalResolver ?? PluralResolvers.cardinal('cs'))(n, | ||
| one: 'Removed one app', | ||
| other: 'Removed {count} apps', | ||
| ); |
There was a problem hiding this comment.
The pluralization strings for appRemoved are in English. Additionally, the Czech language requires the few and many plural forms for correctness, which are missing here. Please update the source translation file for the cs locale to include all required plural forms and their translations.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
14 UNAVAILABLE: Connection dropped |
This reverts commit 1944a15.
1 participant
Work in progress.
Summary by CodeRabbit
Chores
Refactor