Skip to content

Commit 9ba87e4

Browse files
AliSoftwareAliSoftware
authored
Merge pull request #354 from wordpress-mobile/merge-android-strings-toolsignore
Allow auto-addition of `tools:ignore="UnusedResources"` when merging Android strings
2 parents 0e84ac5 + ba4a46c commit 9ba87e4

File tree

4 files changed

+241
-49
lines changed

4 files changed

+241
-49
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ _None_
1313
* Add the option for `an_localize_libs` to provide a `source_id` for each library being merged.
1414
If provided, that identifier will be added as an `a8c-src-lib` XML attribute to the `<string>` nodes being updated with strings from said library.
1515
This can be useful to help identify where each string come from in the resulting, merged `strings.xml`. [#351]
16+
* Add the option for `an_localize_libs` to set the `tools:ignore="UnusedResources"` XML attribute for each string being merged from a library. [#354]
1617

1718
### Bug Fixes
1819

‎lib/fastlane/plugin/wpmreleasetoolkit/actions/android/an_localize_libs_action.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,12 @@ def self.details
3333

3434
def self.available_options
3535
libs_hash_description = <<~KEYS
36-
- `:library`: The library display name
37-
- `:strings_path`: The path to the `strings.xml` file of the library
38-
- `:exclusions`: An optional `Array` of string keys to exclude from merging
36+
- `:library`: The library display name.
37+
- `:strings_path`: The path to the `strings.xml` file of the library.
38+
- `:exclusions`: An optional `Array` of string keys to exclude from merging.
3939
- `:source_id`: An optional `String` which will be added as the `a8c-src-lib` XML attribute
4040
to strings coming from this library, to help identify their source in the merged file.
41+
- `:add_ignore_attr`: If set to true, will add `tools:ignore="UnusedResources"` to merged strings.
4142
KEYS
4243
[
4344
FastlaneCore::ConfigItem.new(key: :app_strings_path,

‎lib/fastlane/plugin/wpmreleasetoolkit/helper/android/android_localize_helper.rb

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,20 @@ def self.skip_string_by_exclusion_list(library, string_name)
3333
end
3434
end
3535

36+
# Adds the appropriate XML attributes to an XML `<string>` node according to library configuration
37+
def self.add_xml_attributes!(string_node, library)
38+
if library[:add_ignore_attr] == true
39+
existing_ignores = (string_node['tools:ignore'] || '').split(',')
40+
existing_ignores.append('UnusedResources') unless existing_ignores.include?('UnusedResources')
41+
string_node['tools:ignore'] = existing_ignores.join(',')
42+
end
43+
string_node[LIB_SOURCE_XML_ATTR] = library[:source_id] unless library[:source_id].nil?
44+
end
45+
3646
# Merge string_line into main_string
3747
def self.merge_string(main_strings, library, string_line)
3848
string_name = string_line.attr('name')
3949
string_content = string_line.content
40-
lib_src_id = library[:source_id]
4150

4251
# Skip strings in the exclusions list
4352
return :skipped if skip_string_by_exclusion_list(library, string_name)
@@ -61,14 +70,14 @@ def self.merge_string(main_strings, library, string_line)
6170
else
6271
# It has the tools:ignore flag, so update the content without touching the other attributes
6372
this_string.content = string_content
64-
this_string[LIB_SOURCE_XML_ATTR] = lib_src_id unless lib_src_id.nil?
73+
add_xml_attributes!(this_string, library)
6574
return result
6675
end
6776
end
6877
end
6978

7079
# String not found, or removed because needing update and not in the exclusion list: add to the main file
71-
string_line[LIB_SOURCE_XML_ATTR] = lib_src_id unless lib_src_id.nil?
80+
add_xml_attributes!(string_line, library)
7281
main_strings.xpath('//string').last().add_next_sibling("\n#{' ' * 4}#{string_line.to_xml().strip}")
7382
return result
7483
end
@@ -103,8 +112,14 @@ def self.verify_string(main_strings, library, string_line)
103112
# @param [Hash] library Hash describing the library to merge. The Hash should contain the following keys:
104113
# - `:library`: The human readable name of the library, used to display in console messages
105114
# - `:strings_path`: The path to the strings.xml file of the library to merge into the main one
106-
# - `:exclusions`: An array of strings keys to exclude during merge. Any of those keys from the library's `strings.xml` will be skipped and won't be merged into the main one.
115+
# - `:exclusions`: An array of strings keys to exclude during merge. Any of those keys from the
116+
# library's `strings.xml` will be skipped and won't be merged into the main one.
117+
# - `:source_id`: An optional `String` which will be added as the `a8c-src-lib` XML attribute
118+
# to strings coming from this library, to help identify their source in the merged file.
119+
# - `:add_ignore_attr`: If set to `true`, will add `tools:ignore="UnusedResources"` to merged strings.
120+
#
107121
# @return [Boolean] True if at least one string from the library has been added to (or has updated) the main strings file.
122+
#
108123
def self.merge_lib(main, library)
109124
UI.message("Merging #{library[:library]} strings into #{main}")
110125
main_strings = File.open(main) { |f| Nokogiri::XML(f, nil, Encoding::UTF_8.to_s) }

‎spec/an_localize_libs_action_spec.rb

Lines changed: 217 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,223 @@
11
require 'spec_helper'
22

33
describe Fastlane::Actions::AnLocalizeLibsAction do
4-
# This test is more of a way of ensuring `run_described_fastlane_action` handles array of hashes properly than a comprehensive test for the `an_localize_libs_action` action.
5-
#
6-
# Please consider expanding this test if you'll find yourself working on its action.
7-
it 'merges the strings from the given array into the given main strings file' do
8-
in_tmp_dir do |tmp_dir|
9-
app_strings_path = File.join(tmp_dir, 'app.xml')
10-
File.write(app_strings_path, android_xml_with_content('<string name="a_string">test from app</string>'))
11-
12-
lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
13-
File.write(lib1_strings_path, android_xml_with_content('<string name="a_lib1_string">test from lib 1</string>'))
14-
15-
lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
16-
File.write(lib2_strings_path, android_xml_with_content('<string name="a_lib2_string">test from lib 2</string>'))
17-
18-
run_described_fastlane_action(
19-
app_strings_path: app_strings_path,
20-
libs_strings_path: [
21-
{ library: 'lib_1', strings_path: lib1_strings_path, exclusions: [] },
22-
{ library: 'lib_2', strings_path: lib2_strings_path, exclusions: [] },
23-
]
24-
)
25-
26-
expected = <<~XML
27-
<string name="a_string">test from app</string>
28-
<string name="a_lib1_string">test from lib 1</string>
29-
<string name="a_lib2_string">test from lib 2</string>
30-
XML
31-
expect(File.read(app_strings_path)).to eq(android_xml_with_content(expected))
32-
end
4+
def android_xml_with_lines(lines)
5+
# I couldn't find a way to interpolate a multiline string preserving its indentation in the heredoc below, so I hacked the following transformation of the input that adds the desired indentation to all lines.
6+
#
7+
# The desired indentation is 4 spaces to stay aligned with the production code applies when merging the XMLs.
8+
indented_content = lines.map { |l| " #{l.chomp}" }.join("\n")
9+
10+
return <<~XML
11+
<?xml version="1.0" encoding="UTF-8"?>
12+
<resources xmlns:tools="http://schemas.android.com/tools">
13+
#{indented_content}
14+
</resources>
15+
XML
3316
end
34-
end
3517

36-
def android_xml_with_content(content)
37-
# I couldn't find a way to interpolate a multiline string preserving its indentation in the heredoc below, so I hacked the following transformation of the input that adds the desired indentation to all lines.
38-
#
39-
# The desired indentation is 4 spaces to stay aligned with the production code applies when merging the XMLs.
40-
indented_content = content.chomp.lines.map { |l| " #{l}" }.join
41-
42-
return <<~XML
43-
<?xml version="1.0" encoding="UTF-8"?>
44-
<resources xmlns:tools="http://schemas.android.com/tools">
45-
#{indented_content}
46-
</resources>
47-
XML
18+
def write_android_xml(path, lines)
19+
File.write(path, android_xml_with_lines(lines))
20+
end
21+
22+
describe 'merges the strings from the given array into the given main strings file' do
23+
it 'handles simple XMLs with no duplicates nor attributes' do
24+
in_tmp_dir do |tmp_dir|
25+
app_strings_path = File.join(tmp_dir, 'app.xml')
26+
File.write(app_strings_path, android_xml_with_lines(['<string name="a_string">test from app</string>']))
27+
28+
lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
29+
File.write(lib1_strings_path, android_xml_with_lines(['<string name="a_lib1_string">test from lib 1</string>']))
30+
31+
lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
32+
File.write(lib2_strings_path, android_xml_with_lines(['<string name="a_lib2_string">test from lib 2</string>']))
33+
34+
run_described_fastlane_action(
35+
app_strings_path: app_strings_path,
36+
libs_strings_path: [
37+
{ library: 'lib_1', strings_path: lib1_strings_path, exclusions: [] },
38+
{ library: 'lib_2', strings_path: lib2_strings_path, exclusions: [] },
39+
]
40+
)
41+
42+
expected = [
43+
'<string name="a_string">test from app</string>',
44+
'<string name="a_lib1_string">test from lib 1</string>',
45+
'<string name="a_lib2_string">test from lib 2</string>',
46+
]
47+
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
48+
end
49+
end
50+
51+
it 'keeps app value if content_override is used' do
52+
in_tmp_dir do |tmp_dir|
53+
app_strings_path = File.join(tmp_dir, 'app.xml')
54+
app_xml_lines = [
55+
'<string name="override-true" content_override="true">from app override-true</string>',
56+
'<string name="override-false" content_override="false">from app override-false</string>',
57+
'<string name="override-missing">from app override-missing</string>',
58+
]
59+
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))
60+
61+
lib_strings_path = File.join(tmp_dir, 'lib.xml')
62+
lib_xml_lines = [
63+
'<string name="override-true">from lib override-true</string>',
64+
'<string name="override-false">from lib override-false</string>',
65+
'<string name="override-missing">from lib override-missing</string>',
66+
]
67+
File.write(lib_strings_path, android_xml_with_lines(lib_xml_lines))
68+
69+
run_described_fastlane_action(
70+
app_strings_path: app_strings_path,
71+
libs_strings_path: [
72+
{ library: 'lib', strings_path: lib_strings_path, exclusions: [] },
73+
]
74+
)
75+
76+
expected = [
77+
'<string name="override-true" content_override="true">from app override-true</string>',
78+
'', '', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
79+
'<string name="override-false">from lib override-false</string>',
80+
'<string name="override-missing">from lib override-missing</string>',
81+
]
82+
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
83+
end
84+
end
85+
86+
it 'adds a8c-lib-src attribute if provided' do
87+
in_tmp_dir do |tmp_dir|
88+
app_strings_path = File.join(tmp_dir, 'app.xml')
89+
app_xml_lines = [
90+
'<string name="override-true" content_override="true">from app override-true</string>',
91+
'<string name="override-missing">from app override-missing</string>',
92+
]
93+
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))
94+
95+
lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
96+
lib1_xml_lines = [
97+
'<string name="override-true">from lib1 override-true</string>',
98+
'<string name="override-missing">from lib1 override-missing</string>',
99+
'<string name="lib1-key">Key only present in lib1</string>',
100+
]
101+
File.write(lib1_strings_path, android_xml_with_lines(lib1_xml_lines))
102+
103+
lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
104+
lib2_xml_lines = [
105+
'<string name="override-true">from lib2 override-true</string>',
106+
'<string name="override-missing">from lib2 override-missing</string>',
107+
'<string name="lib2-key">Key only present in lib2</string>',
108+
]
109+
File.write(lib2_strings_path, android_xml_with_lines(lib2_xml_lines))
110+
111+
run_described_fastlane_action(
112+
app_strings_path: app_strings_path,
113+
libs_strings_path: [
114+
{ library: 'lib1', strings_path: lib1_strings_path, source_id: 'lib1-id' },
115+
{ library: 'lib2', strings_path: lib2_strings_path, source_id: 'lib2-id' },
116+
]
117+
)
118+
119+
expected = [
120+
'<string name="override-true" content_override="true">from app override-true</string>',
121+
'', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
122+
'<string name="lib1-key" a8c-src-lib="lib1-id">Key only present in lib1</string>',
123+
'<string name="override-missing" a8c-src-lib="lib2-id">from lib2 override-missing</string>',
124+
'<string name="lib2-key" a8c-src-lib="lib2-id">Key only present in lib2</string>',
125+
'', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
126+
]
127+
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
128+
end
129+
end
130+
131+
it 'adds tools:ignore attribute when requested' do
132+
in_tmp_dir do |tmp_dir|
133+
app_strings_path = File.join(tmp_dir, 'app.xml')
134+
app_xml_lines = [
135+
'<string name="override-true" content_override="true">from app, override true</string>',
136+
'<string name="ignore-unused" tools:ignore="UnusedResources">from app, tools:ignore="UnusedResources"</string>',
137+
'<string name="ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">from app, tools:ignore mix</string>',
138+
'<string name="ignore-x-y" tools:ignore="X,Y">from app, tools:ignore mix</string>',
139+
]
140+
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))
141+
142+
lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
143+
lib1_xml_lines = [
144+
'<string name="override-true">from lib1, override true</string>',
145+
'<string name="lib1-key">Key only present in lib1, no extra attribute</string>',
146+
'<string name="lib1-ignore-unused" tools:ignore="UnusedResources">Key only present in lib1, with tools:ignore attribute</string>',
147+
'<string name="lib1-ignore-x-y" tools:ignore="X,Y">Key only present in lib1, with tools:ignore attribute x,y</string>',
148+
'<string name="lib1-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">Key only present in lib1, with tools:ignore attribute x,y</string>',
149+
]
150+
File.write(lib1_strings_path, android_xml_with_lines(lib1_xml_lines))
151+
152+
lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
153+
lib2_xml_lines = [
154+
'<string name="override-true">from lib2, override true</string>',
155+
'<string name="lib2-key">Key only present in lib2, no extra attribute</string>',
156+
'<string name="lib2-ignore-unused" tools:ignore="UnusedResources">Key only present in lib2, with tools:ignore attribute</string>',
157+
'<string name="lib2-ignore-x-y" tools:ignore="X,Y">Key only present in lib2, with tools:ignore attribute x,y</string>',
158+
'<string name="lib2-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">Key only present in lib2, with tools:ignore attribute x,y</string>',
159+
]
160+
File.write(lib2_strings_path, android_xml_with_lines(lib2_xml_lines))
161+
162+
run_described_fastlane_action(
163+
app_strings_path: app_strings_path,
164+
libs_strings_path: [
165+
{ library: 'lib1', strings_path: lib1_strings_path, source_id: 'lib1', add_ignore_attr: true },
166+
{ library: 'lib2', strings_path: lib2_strings_path, source_id: 'lib2' },
167+
]
168+
)
169+
170+
expected = [
171+
'<string name="override-true" content_override="true">from app, override true</string>',
172+
'<string name="ignore-unused" tools:ignore="UnusedResources">from app, tools:ignore="UnusedResources"</string>',
173+
'<string name="ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">from app, tools:ignore mix</string>',
174+
'<string name="ignore-x-y" tools:ignore="X,Y">from app, tools:ignore mix</string>',
175+
'<string name="lib1-key" tools:ignore="UnusedResources" a8c-src-lib="lib1">Key only present in lib1, no extra attribute</string>',
176+
'<string name="lib1-ignore-unused" tools:ignore="UnusedResources" a8c-src-lib="lib1">Key only present in lib1, with tools:ignore attribute</string>',
177+
'<string name="lib1-ignore-x-y" tools:ignore="X,Y,UnusedResources" a8c-src-lib="lib1">Key only present in lib1, with tools:ignore attribute x,y</string>',
178+
'<string name="lib1-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y" a8c-src-lib="lib1">Key only present in lib1, with tools:ignore attribute x,y</string>',
179+
'<string name="lib2-key" a8c-src-lib="lib2">Key only present in lib2, no extra attribute</string>',
180+
'<string name="lib2-ignore-unused" tools:ignore="UnusedResources" a8c-src-lib="lib2">Key only present in lib2, with tools:ignore attribute</string>',
181+
'<string name="lib2-ignore-x-y" tools:ignore="X,Y" a8c-src-lib="lib2">Key only present in lib2, with tools:ignore attribute x,y</string>',
182+
'<string name="lib2-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y" a8c-src-lib="lib2">Key only present in lib2, with tools:ignore attribute x,y</string>',
183+
]
184+
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
185+
end
186+
end
187+
188+
it 'handles exclusions list per library' do
189+
in_tmp_dir do |tmp_dir|
190+
app_strings_path = File.join(tmp_dir, 'app.xml')
191+
app_xml_lines = [
192+
'<string name="override-true" content_override="true">from app override-true</string>',
193+
'<string name="override-false" content_override="false">from app override-false</string>',
194+
'<string name="override-missing">from app override-missing</string>',
195+
]
196+
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))
197+
198+
lib_strings_path = File.join(tmp_dir, 'lib.xml')
199+
lib_xml_lines = [
200+
'<string name="override-true">from lib override-true</string>',
201+
'<string name="override-false">from lib override-false</string>',
202+
'<string name="override-missing">from lib override-missing</string>',
203+
]
204+
File.write(lib_strings_path, android_xml_with_lines(lib_xml_lines))
205+
206+
run_described_fastlane_action(
207+
app_strings_path: app_strings_path,
208+
libs_strings_path: [
209+
{ library: 'lib', strings_path: lib_strings_path, exclusions: ['override-missing'] },
210+
]
211+
)
212+
213+
expected = [
214+
'<string name="override-true" content_override="true">from app override-true</string>',
215+
'', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
216+
'<string name="override-missing">from app override-missing</string>',
217+
'<string name="override-false">from lib override-false</string>',
218+
]
219+
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
220+
end
221+
end
222+
end
48223
end

0 commit comments

Comments
 (0)