-
Notifications
You must be signed in to change notification settings - Fork 9
Add android_prune_orphaned_translations action
#734
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: trunk
Are you sure you want to change the base?
Changes from 1 commit
ef04613
e1fbe30
efb03cc
e7789a4
b6ac1db
09939a0
d103030
2a7813e
4dad49b
756965b
904e2de
56dd66d
f6c78ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'fastlane/action' | ||
| require 'nokogiri' | ||
|
mokagio marked this conversation as resolved.
|
||
|
|
||
|
mokagio marked this conversation as resolved.
|
||
| module Fastlane | ||
| module Actions | ||
| class AndroidPruneOrphanedTranslationsAction < Action | ||
| def self.run(params) | ||
| res_dir = params[:res_dir] | ||
| source_paths = [File.join(res_dir, 'values', 'strings.xml')] + params[:additional_source_strings_paths] | ||
| valid_keys = collect_keys(source_paths) | ||
|
|
||
| locale_files = Dir.glob(File.join(res_dir, 'values-*', 'strings.xml')) | ||
|
|
||
| total_pruned = 0 | ||
|
|
||
| locale_files.each do |file| | ||
| pruned = prune_file(file: file, valid_keys: valid_keys) | ||
| next if pruned.empty? | ||
|
|
||
| total_pruned += pruned.count | ||
| UI.message("Pruned #{pruned.count} orphaned entries from `#{file}`: #{pruned.join(', ')}") | ||
| end | ||
|
|
||
| UI.success("Pruned #{total_pruned} orphaned translation entries across #{locale_files.count} locale file(s).") | ||
| total_pruned | ||
| end | ||
|
|
||
| # Collects the set of resource names (string, string-array, plurals, …) declared in the given strings files. | ||
| # | ||
| # @param [Array<String>] paths The strings.xml files to read the valid keys from. | ||
| # @return [Set<String>] The set of declared resource names. | ||
| def self.collect_keys(paths) | ||
| paths.each_with_object(Set.new) do |path, keys| | ||
| doc = File.open(path) { |f| Nokogiri::XML(f, nil, Encoding::UTF_8.to_s) } | ||
| doc.xpath('/resources/*[@name]').each { |node| keys << node['name'] } | ||
| end | ||
| end | ||
|
|
||
| # Removes from `file` any resource entry whose `name` is not in `valid_keys`, preserving the rest of the | ||
| # file's formatting (so the change shows up as a minimal diff). | ||
| # | ||
| # @param [String] file The locale strings.xml file to prune. | ||
| # @param [Set<String>] valid_keys The set of names that are allowed to remain. | ||
| # @return [Array<String>] The names of the entries that were pruned. | ||
| def self.prune_file(file:, valid_keys:) | ||
| doc = File.open(file) { |f| Nokogiri::XML(f, nil, Encoding::UTF_8.to_s) } | ||
| orphans = doc.xpath('/resources/*[@name]').reject { |node| valid_keys.include?(node['name']) } | ||
| return [] if orphans.empty? | ||
|
|
||
| names = orphans.map { |node| node['name'] } | ||
| orphans.each do |node| | ||
| # Drop the indentation/newline text node right before the element too, to avoid leaving a blank line. | ||
| previous = node.previous_sibling | ||
| previous.remove if previous&.text? && previous.text.strip.empty? | ||
| node.remove | ||
| end | ||
|
|
||
| File.open(file, 'w') { |f| doc.write_to(f, encoding: Encoding::UTF_8.to_s, indent: 4) } | ||
| names | ||
| end | ||
|
|
||
| ##################################################### | ||
| # @!group Documentation | ||
| ##################################################### | ||
|
|
||
| def self.description | ||
| 'Removes translations whose key is not present in the source strings, to avoid Lint `ExtraTranslation` errors' | ||
|
oguzkocer marked this conversation as resolved.
Outdated
|
||
| end | ||
|
|
||
| def self.details | ||
| <<~DETAILS | ||
| When downloading translations from GlotPress, the export may include keys that are no longer present in | ||
| the app's source strings (e.g. removed or renamed since the GlotPress source was last synced). Android | ||
| Lint flags these orphaned translations as `ExtraTranslation` errors. | ||
|
|
||
| This action removes — from every `values-*/strings.xml` under `res_dir` — any `<string>`, `<string-array>` | ||
| or `<plurals>` entry whose `name` is not declared in the default `values/strings.xml` of `res_dir`, | ||
| optionally unioned with `additional_source_strings_paths` (useful when a product flavor overlays a base | ||
| module's resources at build time, so the base module's keys are valid too). | ||
| DETAILS | ||
| end | ||
|
|
||
| def self.available_options | ||
| [ | ||
| FastlaneCore::ConfigItem.new( | ||
| key: :res_dir, | ||
| description: "Path to the Android project's `res` directory containing the `values-*` locale subdirectories to prune", | ||
| type: String, | ||
| optional: false | ||
| ), | ||
|
Comment on lines
+105
to
+114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like a reasonable suggestion.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| FastlaneCore::ConfigItem.new( | ||
| key: :additional_source_strings_paths, | ||
| description: 'Paths to additional default `strings.xml` files whose keys should also be treated as valid ' \ | ||
| '(e.g. a base module that the pruned `res_dir` overlays at build time)', | ||
| type: Array, | ||
| optional: true, | ||
| default_value: [] | ||
| ), | ||
|
Comment on lines
+115
to
+127
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also seems like a good addition.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ] | ||
| end | ||
|
|
||
| def self.return_value | ||
| 'The total number of orphaned translation entries that were pruned' | ||
| end | ||
|
|
||
| def self.authors | ||
| ['Automattic'] | ||
| end | ||
|
|
||
| def self.is_supported?(platform) | ||
| platform == :android | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'spec_helper' | ||
| require 'tmpdir' | ||
|
mokagio marked this conversation as resolved.
|
||
|
|
||
| describe Fastlane::Actions::AndroidPruneOrphanedTranslationsAction do | ||
| # Writes `content` to `path`, creating intermediate directories. | ||
| def write_file(path, content) | ||
| FileUtils.mkdir_p(File.dirname(path)) | ||
| File.write(path, content) | ||
| end | ||
|
|
||
| # A default `values/strings.xml` declaring `hello`, `bye`, an array and a plural. | ||
| let(:default_strings) do | ||
| <<~XML | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <resources> | ||
| <string name="hello">Hello</string> | ||
| <string name="bye">Bye</string> | ||
| <string-array name="planets"> | ||
| <item>Earth</item> | ||
| </string-array> | ||
| <plurals name="items"> | ||
| <item quantity="one">%d item</item> | ||
| <item quantity="other">%d items</item> | ||
| </plurals> | ||
| </resources> | ||
| XML | ||
| end | ||
|
|
||
| it 'removes only the entries whose key is not in the default strings, keeping the rest intact' do | ||
| Dir.mktmpdir do |dir| | ||
| res_dir = File.join(dir, 'res') | ||
| write_file(File.join(res_dir, 'values', 'strings.xml'), default_strings) | ||
| fr_file = File.join(res_dir, 'values-fr', 'strings.xml') | ||
| write_file(fr_file, <<~XML) | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <resources> | ||
| <string name="hello">Bonjour</string> | ||
| <string name="orphan_string">Orphelin</string> | ||
| <string name="bye">Au revoir</string> | ||
| <plurals name="orphan_plural"> | ||
| <item quantity="one">%d truc</item> | ||
| <item quantity="other">%d trucs</item> | ||
| </plurals> | ||
| </resources> | ||
| XML | ||
|
|
||
| pruned = run_described_fastlane_action(res_dir: res_dir) | ||
|
|
||
| expect(pruned).to eq(2) | ||
| content = File.read(fr_file) | ||
| expect(content).to include('name="hello"', 'name="bye"') | ||
| expect(content).not_to include('orphan_string', 'orphan_plural') | ||
| # No blank line left behind where the orphaned <string> was removed. | ||
| expect(content).not_to match(/\n[[:space:]]*\n[[:space:]]*<string name="bye"/) | ||
| end | ||
| end | ||
|
|
||
| it 'treats keys from `additional_source_strings_paths` as valid (flavor overlay case)' do | ||
| Dir.mktmpdir do |dir| | ||
| res_dir = File.join(dir, 'res') | ||
| write_file(File.join(res_dir, 'values', 'strings.xml'), <<~XML) | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <resources> | ||
| <string name="flavor_only">Flavor</string> | ||
| </resources> | ||
| XML | ||
| base_strings = File.join(dir, 'base', 'values', 'strings.xml') | ||
| write_file(base_strings, default_strings) | ||
| fr_file = File.join(res_dir, 'values-fr', 'strings.xml') | ||
| write_file(fr_file, <<~XML) | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <resources> | ||
| <string name="flavor_only">Saveur</string> | ||
| <string name="hello">Bonjour</string> | ||
| <string name="orphan_string">Orphelin</string> | ||
| </resources> | ||
| XML | ||
|
|
||
| pruned = run_described_fastlane_action(res_dir: res_dir, additional_source_strings_paths: [base_strings]) | ||
|
|
||
| expect(pruned).to eq(1) | ||
| content = File.read(fr_file) | ||
| expect(content).to include('name="flavor_only"', 'name="hello"') | ||
| expect(content).not_to include('orphan_string') | ||
| end | ||
| end | ||
|
|
||
| it 'does nothing and reports zero when there are no orphaned entries' do | ||
| Dir.mktmpdir do |dir| | ||
| res_dir = File.join(dir, 'res') | ||
| write_file(File.join(res_dir, 'values', 'strings.xml'), default_strings) | ||
| fr_file = File.join(res_dir, 'values-fr', 'strings.xml') | ||
| fr_content = <<~XML | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <resources> | ||
| <string name="hello">Bonjour</string> | ||
| <string name="bye">Au revoir</string> | ||
| </resources> | ||
| XML | ||
| write_file(fr_file, fr_content) | ||
|
|
||
| pruned = run_described_fastlane_action(res_dir: res_dir) | ||
|
|
||
| expect(pruned).to eq(0) | ||
| expect(File.read(fr_file)).to eq(fr_content) | ||
| end | ||
| end | ||
| end | ||
Uh oh!
There was an error while loading. Please reload this page.