From 1a48ec82565f6b3c9174b5ef7669dd226105f10d Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 25 Jun 2026 15:52:50 +1000 Subject: [PATCH 1/2] Add specs for inter-token comments in `merge_strings` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These specs fail against the current `merge_strings`, pinning a gap left by PR #741: the duplicate-key scanner and `plutil`-based bookkeeping both accept `.strings` comments *between* a statement's tokens (e.g. `CFBundleName /* c */ = WordPress;`), but the line-based key rewrite only prefixes when `=` and the value are adjacent. A key behind such a comment is written unprefixed while bookkept *with* the prefix — resurfacing the very key collisions the prefix exists to prevent, as the second spec demonstrates by merging two files into a single collapsed key. --- Generated with the help of Claude Code, https://claude.com/claude-code Co-Authored-By: Claude Opus 4.8 (1M context) --- spec/ios_l10n_helper_spec.rb | 40 ++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/spec/ios_l10n_helper_spec.rb b/spec/ios_l10n_helper_spec.rb index 3d2d66ed4..aa53b2ae2 100644 --- a/spec/ios_l10n_helper_spec.rb +++ b/spec/ios_l10n_helper_spec.rb @@ -122,6 +122,46 @@ def file_encoding(path) end end + it 'prefixes keys when `.strings` comments sit between a statement\'s tokens' do + # Regression: the duplicate-key scanner accepts comments *between* a statement's tokens (e.g. + # `CFBundleName /* c */ = WordPress;`), and bookkeeping derives keys from `plutil`, which agrees. + # But the line-based rewrite only prefixes when `=` and the value are adjacent, so an inter-token + # comment leaves the key written unprefixed while still bookkept *with* the prefix — the exact + # collision/inconsistent-output the prefix exists to prevent. + content = <<~STRINGS + CFBundleName = WordPress /* trailing */; + AppName /* between key and = */ = WordPress; + DisplayName /* between key and = */ = "WordPress"; + STRINGS + Dir.mktmpdir('a8c-release-toolkit-l10n-helper-tests-') do |tmp_dir| + input_file = File.join(tmp_dir, 'InfoPlist.strings') + File.write(input_file, content) + output_file = File.join(tmp_dir, 'output.strings') + described_class.merge_strings(paths: { input_file => 'pfx.' }, output_path: output_file) + merged = File.read(output_file) + expect(merged).to include('"pfx.CFBundleName"') + expect(merged).to include('"pfx.AppName"') + expect(merged).to include('"pfx.DisplayName"') + end + end + + it 'does not silently drop a key to a collision when an inter-token comment blocks prefixing' do + # The harm of the gap above: two files each carrying the same key behind an inter-token comment are + # bookkept under distinct prefixes (so no duplicate is reported), yet both are written verbatim — + # producing a genuine duplicate in the merged file that `plutil` later collapses to a single value. + content = "CFBundleName /* c */ = WordPress;\n" + Dir.mktmpdir('a8c-release-toolkit-l10n-helper-tests-') do |tmp_dir| + file_a = File.join(tmp_dir, 'A.strings') + file_b = File.join(tmp_dir, 'B.strings') + File.write(file_a, content) + File.write(file_b, content) + output_file = File.join(tmp_dir, 'output.strings') + described_class.merge_strings(paths: { file_a => 'a.', file_b => 'b.' }, output_path: output_file) + merged_keys = described_class.read_strings_file_as_hash(path: output_file).keys + expect(merged_keys).to contain_exactly('a.CFBundleName', 'b.CFBundleName') + end + end + it 'returns duplicate keys found' do paths = { fixture('Localizable-utf16.strings') => nil, fixture('non-latin-utf16.strings') => nil } Dir.mktmpdir('a8c-release-toolkit-l10n-helper-tests-') do |tmp_dir| From ad221210c51f5c40fc5c0224565460a97fe85318 Mon Sep 17 00:00:00 2001 From: Gio Lodi Date: Thu, 25 Jun 2026 17:42:21 +1000 Subject: [PATCH 2/2] Tighten inter-token merge specs Assert parsed keys so the regression captures the merge contract without depending on raw output formatting. --- Generated with the help of Codex, https://openai.com/codex Co-Authored-By: Codex GPT-5 --- spec/ios_l10n_helper_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/ios_l10n_helper_spec.rb b/spec/ios_l10n_helper_spec.rb index aa53b2ae2..5f01c93e0 100644 --- a/spec/ios_l10n_helper_spec.rb +++ b/spec/ios_l10n_helper_spec.rb @@ -138,10 +138,8 @@ def file_encoding(path) File.write(input_file, content) output_file = File.join(tmp_dir, 'output.strings') described_class.merge_strings(paths: { input_file => 'pfx.' }, output_path: output_file) - merged = File.read(output_file) - expect(merged).to include('"pfx.CFBundleName"') - expect(merged).to include('"pfx.AppName"') - expect(merged).to include('"pfx.DisplayName"') + merged_keys = described_class.read_strings_file_as_hash(path: output_file).keys + expect(merged_keys).to contain_exactly('pfx.CFBundleName', 'pfx.AppName', 'pfx.DisplayName') end end