-
-
Notifications
You must be signed in to change notification settings - Fork 364
Feature/generator yaml support #772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Feature/generator yaml support #772
Conversation
extended the generator to support yaml localization files.
added yaml package to dependencies
added change description
WalkthroughAdds YAML support to the generate CLI: accepts Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Gen as bin/generate.dart
participant FS as FileSystem
participant YAML as package:yaml (loadYaml)
participant JSON as dart:convert (jsonDecode)
participant CG as Codegen/Writers
Dev->>Gen: run :generate --format yaml|json
Gen->>FS: list files (*.json, *.yml, *.yaml)
alt YAML file
Gen->>FS: read file contents
Gen->>YAML: loadYaml(contents)
YAML-->>Gen: YamlMap / YamlList
Gen->>Gen: normalize to Map<String,dynamic>
else JSON file
Gen->>FS: read file contents
Gen->>JSON: jsonDecode(contents)
JSON-->>Gen: Map/List
end
Gen->>CG: assemble mapLocales (per-locale maps)
CG-->>Gen: generated Dart artifacts
Gen-->>FS: write outputs
Gen-->>Dev: exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pubspec.yaml (1)
8-8
: Version mismatch with CHANGELOG (3.0.9 vs 3.0.8). Bump pubspec.CHANGELOG introduces 3.0.9; pubspec still at 3.0.8. Please align.
Apply this diff:
-version: 3.0.8 +version: 3.0.9
🧹 Nitpick comments (4)
CHANGELOG.md (1)
1-1
: Optional: satisfy markdown heading-increment rule without touching all entries.Change top heading to H2 so subsequent H3 release sections comply with MD001.
-# Changelog +## Changelogbin/generate.dart (3)
47-85
: Tighten help text phrasing (nit).Minor clarity/grammar improvements for CLI help.
- help: 'Folder containing localization files', + help: 'Folder containing localization files', @@ - help: 'File to use for localization', + help: 'Specific localization file to use', @@ - help: 'Output folder stores for the generated file', + help: 'Output folder for the generated file', @@ - help: 'Output file name', + help: 'Output file name', @@ - help: 'Support json or keys formats', + help: 'Supported output formats: json, keys',
154-156
: Optional: handle directory listing errors.Add onError to complete the completer with an error to avoid hanging on I/O issues.
lister.listen( (file) => files.add(file), - onDone: () => completer.complete(files), + onDone: () => completer.complete(files), + onError: (e, st) => completer.completeError(e, st), );
214-257
: Optional: sanitize constant identifiers to valid Dart names.Keys with hyphens/spaces produce invalid identifiers. Consider normalizing to [a-zA-Z0-9_] and prefixing leading digits.
Example adjustment (conceptual):
- static const ${accKey.replaceAll('.', '_')}_$key = ... + static const ${_id('${accKey.replaceAll('.', '_')}_$key')} = ...With helper:
String _id(String raw) { final s = raw.replaceAll(RegExp(r'[^a-zA-Z0-9_]'), '_'); return s.startsWith(RegExp(r'[0-9]')) ? '_$s' : s; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
CHANGELOG.md
(1 hunks)bin/generate.dart
(10 hunks)pubspec.yaml
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[grammar] ~6-~6: Ensure spelling is correct
Context: ... ### [3.0.9] - added .yaml support for transaltion files in the :generate script ### [3....
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
4-4: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
pubspec.yaml (1)
20-20
: Dependency addition looks correct.Adding yaml ^3.1.2 is appropriate for YAML parsing used by the generator. No concerns.
bin/generate.dart (2)
7-7
: Import of yaml is appropriate.Required for YAML parsing; no issues.
310-323
: Top-level YAML lists produce an empty map with a '' key — likely unintended.Either disallow top-level lists or map them to a well-defined key. Current behavior generates an empty identifier and invalid constants.
Suggest enforcing top-level mapping and throwing on YamlList:
- } else if (yaml is YamlList) { - return {'': yaml.map((e) => _yamlToMap(e)).toList()}; + } else if (yaml is YamlList) { + // Top-level lists are not supported in translation files + return {}; }If you prefer supporting lists under named keys, keep this helper as-is and validate at call sites that the root is a map (see _parseTranslations suggestion).
bin/generate.dart
Outdated
final outputPath = Directory( | ||
path.join(current.path, output.path, options.outputFile), | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Treating a file path as a Directory.
outputPath holds a file path (includes options.outputFile) but is typed/constructed as Directory. Use File to avoid semantic confusion and potential path issues.
- final outputPath = Directory(
- path.join(current.path, output.path, options.outputFile),
- );
+ final outputFile = File(
+ path.join(current.path, output.path, options.outputFile),
+ );
Also update the call site at Line 143 and the generateFile signature accordingly (see separate diffs).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
final outputPath = Directory( | |
path.join(current.path, output.path, options.outputFile), | |
); | |
final outputFile = File( | |
path.join(current.path, output.path, options.outputFile), | |
); |
🤖 Prompt for AI Agents
In bin/generate.dart around lines 118 to 121, outputPath is constructed as a
Directory but it actually contains a file path (includes options.outputFile);
change it to a File (use File(path.join(...))) to reflect a file target, update
the call site at line 143 to pass a File instead of Directory, and adjust the
generateFile function signature to accept a File (and update any
Directory-specific APIs inside generateFile to File APIs).
bin/generate.dart
Outdated
void generateFile( | ||
List<FileSystemEntity> files, | ||
Directory outputPath, | ||
GenerateOptions options, | ||
) async { | ||
var generatedFile = File(outputPath.path); | ||
if (!generatedFile.existsSync()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Align generateFile API with a File destination.
Update signature and usage to accept File instead of Directory.
-void generateFile(
- List<FileSystemEntity> files,
- Directory outputPath,
- GenerateOptions options,
-) async {
- var generatedFile = File(outputPath.path);
+void generateFile(
+ List<FileSystemEntity> files,
+ File outputFile,
+ GenerateOptions options,
+) async {
+ var generatedFile = outputFile;
if (!generatedFile.existsSync()) {
generatedFile.createSync(recursive: true);
}
And at Line 143:
- generateFile(files, outputPath, options);
+ generateFile(files, outputFile, options);
And at Line 186:
- stdout.writeln('All done! File generated in ${outputPath.path}');
+ stdout.writeln('All done! File generated in ${outputFile.path}');
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
void generateFile( | |
List<FileSystemEntity> files, | |
Directory outputPath, | |
GenerateOptions options, | |
) async { | |
var generatedFile = File(outputPath.path); | |
if (!generatedFile.existsSync()) { | |
// Updated signature to take a File instead of Directory | |
void generateFile( | |
List<FileSystemEntity> files, | |
File outputFile, | |
GenerateOptions options, | |
) async { | |
var generatedFile = outputFile; | |
if (!generatedFile.existsSync()) { | |
generatedFile.createSync(recursive: true); | |
} | |
// …rest of the implementation… | |
} |
void generateFile( | |
List<FileSystemEntity> files, | |
Directory outputPath, | |
GenerateOptions options, | |
) async { | |
var generatedFile = File(outputPath.path); | |
if (!generatedFile.existsSync()) { | |
// At Line 143: pass the File instead of a Directory | |
generateFile(files, outputFile, options); |
void generateFile( | |
List<FileSystemEntity> files, | |
Directory outputPath, | |
GenerateOptions options, | |
) async { | |
var generatedFile = File(outputPath.path); | |
if (!generatedFile.existsSync()) { | |
// At Line 186: reference outputFile.path | |
stdout.writeln('All done! File generated in ${outputFile.path}'); |
🤖 Prompt for AI Agents
In bin/generate.dart around lines 160-166 (and also update references at lines
143 and 186), the generateFile function currently accepts a Directory but should
accept a File destination; change the function signature parameter type from
Directory outputPath to File outputFile (and rename usages accordingly), replace
any calls to outputPath.path with outputFile.path, update existence and
parent-directory handling to use outputFile.existsSync() and
outputFile.parent.createSync(recursive: true) as needed, and update the two call
sites at lines 143 and 186 to pass a File object instead of a Directory.
bin/generate.dart
Outdated
Future _writeKeys( | ||
StringBuffer classBuilder, | ||
List<FileSystemEntity> files, | ||
bool? skipUnnecessaryKeys, | ||
) async { | ||
var file = ''' | ||
// DO NOT EDIT. This is code generated via package:easy_localization/generate.dart | ||
|
||
// ignore_for_file: constant_identifier_names | ||
|
||
abstract class LocaleKeys { | ||
abstract class LocaleKeys { | ||
'''; | ||
|
||
final fileData = File(files.first.path); | ||
|
||
Map<String, dynamic> translations = | ||
json.decode(await fileData.readAsString()); | ||
// Parse YAML file | ||
final yamlString = await fileData.readAsString(); | ||
final yamlData = loadYaml(yamlString); | ||
Map<String, dynamic> translations = _yamlToMap(yamlData); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Keys generation reads only YAML; restore JSON support and validate top-level structure.
Use a shared parser that detects by extension and ensures a top-level map.
- // Parse YAML file
- final yamlString = await fileData.readAsString();
- final yamlData = loadYaml(yamlString);
- Map<String, dynamic> translations = _yamlToMap(yamlData);
+ // Parse file (.json, .yml, .yaml)
+ final Map<String, dynamic> translations = await _parseTranslations(fileData);
Additional helper to add (see snippet below in this comment thread).
Add this helper outside the function (near _yamlToMap):
// Parses .json, .yml, .yaml into a Map<String, dynamic>.
Future<Map<String, dynamic>> _parseTranslations(File file) async {
final ext = path.extension(file.path).toLowerCase();
final content = await file.readAsString();
if (ext == '.json') {
final decoded = json.decode(content);
if (decoded is! Map) {
throw FormatException('Top-level JSON must be an object');
}
return Map<String, dynamic>.from(decoded as Map);
}
final yamlRoot = loadYaml(content);
final map = _yamlToMap(yamlRoot);
if (map.isEmpty && yamlRoot is! YamlMap) {
throw FormatException('Top-level YAML must be a mapping');
}
return map;
}
🤖 Prompt for AI Agents
In bin/generate.dart around lines 189 to 208, the _writeKeys function currently
reads only YAML files and directly calls loadYaml, so JSON input and top-level
structure validation are missing; replace the inline YAML-only parsing with a
shared async parser that checks the file extension ('.json' vs '.yml'/' .yaml'),
decodes JSON into a Map and throws a FormatException if the decoded top-level is
not a Map, or for YAML uses loadYaml then converts via _yamlToMap and throws if
the YAML root is not a mapping; add this helper function outside _writeKeys
(near _yamlToMap) and call it from _writeKeys instead of the current YAML-only
code so both JSON and YAML are supported and validated.
adapted the changes a bit
added comments back in
Fix Typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bin/generate.dart (2)
145-172
: Fix async and destination typing: return Future, accept File, and await.
Avoid async void and pass a File instead of a Directory with a file path.-void generateFile(List<FileSystemEntity> files, Directory outputPath, - GenerateOptions options) async { - var generatedFile = File(outputPath.path); +Future<void> generateFile( + List<FileSystemEntity> files, + File outputFile, + GenerateOptions options, +) async { + var generatedFile = outputFile; if (!generatedFile.existsSync()) { generatedFile.createSync(recursive: true); } @@ generatedFile.writeAsStringSync(classBuilder.toString()); - stdout.writeln('All done! File generated in ${outputPath.path}'); + stdout.writeln('All done! File generated in ${outputFile.path}');Also update the caller to await:
- generateFile(files, outputPath, options); + await generateFile(files, outputFile, options);
20-26
: Propagate Futures and await in main.
Ensure the CLI exits after generation completes.-void main(List<String> args) { +Future<void> main(List<String> args) async { if (_isHelpCommand(args)) { _printHelperDisplay(); } else { - handleLangFiles(_generateOption(args)); + await handleLangFiles(_generateOption(args)); } }-void handleLangFiles(GenerateOptions options) async { +Future<void> handleLangFiles(GenerateOptions options) async {Also applies to: 103-104
♻️ Duplicate comments (4)
bin/generate.dart (4)
108-111
: Bug: Treating a file path as a Directory (same as earlier feedback).
This path includes the output filename; model it as File, not Directory. Also return/await Futures to avoid fire-and-forget.Apply:
- final outputPath = - Directory(path.join(current.path, output.path, options.outputFile)); + final outputFile = + File(path.join(current.path, output.path, options.outputFile));And update call sites + signatures below (see dedicated diffs in comments for Lines 145-172 and 20-26/103-104).
186-188
: Keys path parses only via YAML; restore JSON support with a shared parser.
Use a helper that decodes by extension and enforces top-level mapping.- Map<String, dynamic> translations = - json.decode(json.encode(loadYaml(await fileData.readAsString()))); + final Map<String, dynamic> translations = + await _parseTranslations(fileData);Add once (outside this function):
// Parses .json/.yaml/.yml into Map<String, dynamic> with top-level validation. Future<Map<String, dynamic>> _parseTranslations(File file) async { final ext = path.extension(file.path).toLowerCase(); final content = await file.readAsString(); if (ext == '.json') { final decoded = json.decode(content); if (decoded is! Map) { throw const FormatException('Top-level JSON must be an object'); } return Map<String, dynamic>.from(decoded as Map); } final yamlRoot = loadYaml(content); final normalized = json.decode(json.encode(yamlRoot)); if (normalized is! Map) { throw const FormatException('Top-level YAML must be a mapping'); } return Map<String, dynamic>.from(normalized as Map); }
298-304
: Locale name extraction should also strip .json and anchor to end.
Prevents leftover “.json” in locale keys when mixed inputs slip in.- final localeName = path - .basename(file.path) - .replaceFirst(RegExp(r'\.(yaml|yml)'), '') - .replaceAll('-', '_'); + final localeName = path + .basename(file.path) + .replaceFirst(RegExp(r'\.(json|ya?ml)$', caseSensitive: false), '') + .replaceAll('-', '_');
306-309
: YAML normalization: ensure JSON-encodable Map before embedding.
JsonEncoder on YamlMap/YamlScalar can fail; normalize first or reuse the shared parser.- var data = loadYaml(await fileData.readAsString()); - final mapString = const JsonEncoder.withIndent(' ').convert(data); + final Map<String, dynamic> data = await _parseTranslations(fileData); + final mapString = const JsonEncoder.withIndent(' ').convert(data); gFile += 'static const Map<String,dynamic> _$localeName = $mapString;\n';
🧹 Nitpick comments (4)
bin/generate.dart (4)
74-76
: CLI help/allowed values updated correctly.
Consider updating README/CHANGELOG examples accordingly.
158-160
: Add YAML case — OK, but consider de-duplicating loaders.
json and yaml writers are near-identical; share a single loader builder with a unified parser to reduce drift.
198-200
: Nit: ‘sortedKeys’ isn’t sorted; sort for stable output.- final sortedKeys = translations.keys.toList(); + final sortedKeys = translations.keys.toList()..sort();
275-315
: Reduce duplication: merge _writeJson and _writeYaml into one loader.
Both functions differ only by parsing and extension stripping. Keep one writer and delegate to _parseTranslations().Example outline:
Future _writeLoader( StringBuffer classBuilder, List<FileSystemEntity> files, ) async { final buffer = StringBuffer(/* header & class */); final locales = <String>[]; for (final fse in files.whereType<File>()) { final localeName = path.basename(fse.path) .replaceFirst(RegExp(r'\.(json|ya?ml)$', caseSensitive: false), '') .replaceAll('-', '_'); final data = await _parseTranslations(fse); final mapString = const JsonEncoder.withIndent(' ').convert(data); locales.add('"$localeName": _$localeName'); buffer.writeln('static const Map<String,dynamic> _$localeName = $mapString;'); } buffer.writeln( 'static const Map<String, Map<String,dynamic>> mapLocales = {${locales.join(', ')}};' ); classBuilder.writeln(buffer.toString()); }Then in switch:
- case 'json': await _writeLoader(...);
- case 'yaml': await _writeLoader(...);
</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 75053687db651f126e4e80b675a75fefaf49fb22 and a5645ac2f9d0c65479cfb9afbf2b1508194c2c80. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `bin/generate.dart` (6 hunks) </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: Codacy Static Code Analysis </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>bin/generate.dart (1)</summary><blockquote> `7-7`: **LGTM: YAML import added where needed.** </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
//filtering format | ||
files = files.where((f) => f.path.contains('.json')).toList(); | ||
files = files.where((f) => f.path.contains(RegExp(r'\.(json|yaml|yml)$'))).toList(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Filter by selected format and ensure only files are processed.
Currently mixes JSON with YAML when format='yaml' and may include directories. Make filtering deterministic and case-insensitive.
- //filtering format
- files = files.where((f) => f.path.contains(RegExp(r'\.(json|yaml|yml)$'))).toList();
+ // Filter files based on requested format; keep only regular files.
+ files = files
+ .whereType<File>()
+ .where((f) {
+ final p = f.path.toLowerCase();
+ switch (options.format) {
+ case 'json':
+ return p.endsWith('.json');
+ case 'yaml':
+ return p.endsWith('.yaml') || p.endsWith('.yml');
+ case 'keys':
+ return p.endsWith('.json') || p.endsWith('.yaml') || p.endsWith('.yml');
+ default:
+ return false;
+ }
+ })
+ .toList();
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
//filtering format | |
files = files.where((f) => f.path.contains('.json')).toList(); | |
files = files.where((f) => f.path.contains(RegExp(r'\.(json|yaml|yml)$'))).toList(); | |
} | |
// Filter files based on requested format; keep only regular files. | |
files = files | |
.whereType<File>() | |
.where((f) { | |
final p = f.path.toLowerCase(); | |
switch (options.format) { | |
case 'json': | |
return p.endsWith('.json'); | |
case 'yaml': | |
return p.endsWith('.yaml') || p.endsWith('.yml'); | |
case 'keys': | |
return p.endsWith('.json') || p.endsWith('.yaml') || p.endsWith('.yml'); | |
default: | |
return false; | |
} | |
}) | |
.toList(); | |
} |
🤖 Prompt for AI Agents
In bin/generate.dart around lines 125-127, the current filter mixes JSON and
YAML when format='yaml' and can include directories; update the filtering to
first exclude non-files using FileSystemEntity.isFileSync(path) and then match
extensions deterministically: build the allowed extensions based on the selected
format (e.g., ['json'] for 'json', ['yaml','yml'] for 'yaml'), compile a RegExp
that matches the extension at the end of the path with caseSensitive: false, and
filter files by both being a real file and matching that regex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bin/generate.dart (1)
116-124
: Bug: sourceFile existence check uses the wrong base path.Use
sourcePath.path
(absolute) instead ofsource.path
(relative), otherwise the file may be reported missing.- final sourceFile = File(path.join(source.path, options.sourceFile)); + final sourceFile = File(path.join(sourcePath.path, options.sourceFile!));
♻️ Duplicate comments (6)
CHANGELOG.md (1)
4-6
: Fix heading level (MD001) and mention both YAML extensions.
- Use H2 after H1 to satisfy markdownlint.
- Reflect support for both .yaml and .yml.
-### [3.0.9] +## [3.0.9] -- added .yaml support for translation files in the :generate script +- added .yaml/.yml support for translation files in the :generate scriptbin/generate.dart (5)
108-110
: Treating a file path as a Directory — switch to File.
outputPath
includes the file name; model it asFile
to avoid confusion and API misuse.- final outputPath = - Directory(path.join(current.path, output.path, options.outputFile)); + final outputFile = + File(path.join(current.path, output.path, options.outputFile!));
129-131
: Pass a File to generateFile.- generateFile(files, outputPath, options); + generateFile(files, outputFile, options);
145-175
: Align generateFile API with a File destination and fix the completion message.-void generateFile(List<FileSystemEntity> files, Directory outputPath, - GenerateOptions options) async { - var generatedFile = File(outputPath.path); +void generateFile(List<FileSystemEntity> files, File outputFile, + GenerateOptions options) async { + var generatedFile = outputFile; if (!generatedFile.existsSync()) { generatedFile.createSync(recursive: true); } @@ classBuilder.writeln('}'); generatedFile.writeAsStringSync(classBuilder.toString()); - stdout.writeln('All done! File generated in ${outputPath.path}'); + stdout.writeln('All done! File generated in ${outputFile.path}'); }
125-127
: Bug: Filtering ignores selected format and may pass YAML to the JSON path.Respect
options.format
and only include files of that type; also exclude non-files.- //filtering format - files = files.where((f) => f.path.contains(RegExp(r'\.(json|yaml|yml)$'))).toList(); + // Filter files deterministically by the requested format + files = files + .whereType<File>() + .where((f) { + final p = f.path.toLowerCase(); + switch (options.format) { + case 'json': + return p.endsWith('.json'); + case 'yaml': + return p.endsWith('.yaml') || p.endsWith('.yml'); + case 'keys': + // Keys can be generated from either JSON or YAML + return p.endsWith('.json') || p.endsWith('.yaml') || p.endsWith('.yml'); + default: + return false; + } + }) + .toList();
301-312
: Normalize YAML to JSON-safe Map and reuse the shared parser.Avoid relying on YamlMap being JSON-encodable across SDKs; parse by extension.
- var data = loadYaml(await fileData.readAsString()); - final mapString = const JsonEncoder.withIndent(' ').convert(data); + final Map<String, dynamic> data = await _parseTranslations(fileData); + final mapString = const JsonEncoder.withIndent(' ').convert(data);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
CHANGELOG.md
(1 hunks)bin/generate.dart
(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
4-4: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
bin/generate.dart (3)
7-7
: Import looks good.
74-76
: CLI option update is correct.
158-160
: YAML path added — OK. Ensure parser handles both formats consistently.This is fine once parsing is shared (see comments below).
final fileData = File(files.first.path); | ||
|
||
Map<String, dynamic> translations = | ||
json.decode(await fileData.readAsString()); | ||
json.decode(json.encode(loadYaml(await fileData.readAsString()))); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keys generation reads YAML only — add shared parser to support JSON and validate top-level.
Without this, -f keys
fails for .json
sources and lacks structure checks.
- Map<String, dynamic> translations =
- json.decode(json.encode(loadYaml(await fileData.readAsString())));
+ final Map<String, dynamic> translations = await _parseTranslations(fileData);
Add this helper (place near _resolve
):
// Parses .json, .yaml, .yml into Map<String, dynamic> and validates top-level map.
Future<Map<String, dynamic>> _parseTranslations(File file) async {
final ext = path.extension(file.path).toLowerCase();
final content = await file.readAsString();
if (ext == '.json') {
final decoded = json.decode(content);
if (decoded is! Map) {
throw const FormatException('Top-level JSON must be an object');
}
return Map<String, dynamic>.from(decoded as Map);
}
// YAML path
final yamlRoot = loadYaml(content);
// Normalize YamlMap/YamlList into JSON-friendly Map/List
final normalized = json.decode(json.encode(yamlRoot));
if (normalized is! Map) {
throw const FormatException('Top-level YAML must be a mapping');
}
return Map<String, dynamic>.from(normalized as Map);
}
🤖 Prompt for AI Agents
In bin/generate.dart around lines 187 to 191, the current code reads YAML only
and directly decodes a YAML structure into a Map which breaks when input is JSON
and does not validate top-level structure; add a helper method named
_parseTranslations placed near _resolve that: reads the file bytes, inspects
path.extension(file.path).toLowerCase(), for '.json' uses json.decode and throws
a FormatException if the decoded value is not a Map, for YAML ('.yaml' or
'.yml') uses loadYaml then normalizes the YamlMap/YamlList to JSON-friendly
structures via json.encode/json.decode, validates the normalized value is a Map,
converts and returns Map<String, dynamic>; then replace the current YAML-only
logic at lines 187–191 to call await _parseTranslations(fileData) and use its
returned Map.
Extended the generate class to support generation of LocaleKeys from a yaml file.
This helps structuring the translations with comments which .json does not allow.
Summary by CodeRabbit