-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
[gen_l10n] Optionally generate list of inputs/outputs #56490
[gen_l10n] Optionally generate list of inputs/outputs #56490
Conversation
dev/tools/localization/gen_l10n.dart
Outdated
} | ||
_inputsAndOutputsListFile.writeAsStringSync( | ||
inputAndOutputsJsonFileTemplate | ||
.replaceAll('@(inputs)', _inputFileList.map((String file) => '"$file"').join(',\n ')) |
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.
Instead of using string replace, create a map and use json.encode:
_inputsAndOutputsListFile.writeAsStringSync(
json.encode(<String, Object>{
'inputs': _inputFileList,
'outputs': _outputFileList,
}),
)
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.
LGTM
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.
LGTM too, modulo a few trivial wording issues
'gen-inputs-and-outputs-list', | ||
valueHelp: 'path-to-output-directory', | ||
help: 'When specified, the tool generates a JSON file containing the ' | ||
'tool\'s inputs and outputs titled gen_l10n_inputs_and_outputs.json.' |
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.
titled => named
'project were used when generating the latest set of localizations. ' | ||
'For example, the Flutter tool\'s build system uses this file to ' | ||
'keep track of when to call gen_l10n during hot reload.\n\n' | ||
'This option takes in the directory where the JSON file will be ' |
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.
The value of this option is the directory...
Description
This PR creates a new option to output a list of inputs and outputs for the gen_l10n tool. It takes in the path (either relative or absolute) and creates an
gen_l10n_inputs_and_outputs.json
file. Using this option for the stocks app example, here is a sample output:The PR also gratuitously fixes up terminology and tests that could be improved surrounding
writeOutputFile
andgenerateCode
. These updates should not be breaking, since the developer API has not been modified.Related Issues
Fixes #55503
Tests
I added the following tests:
generateCode
has become an implementation ofwriteOutputFile
writeOutputFile
to pluralwriteOutputFiles
since the tool now generates more than one localization file (it has been generating more than one file prior to this PR, but the code hasn't been updated so it was confusing).Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.