Add optional prefixes when merging in ios_merge_strings_files (#345)

Author: mokagio

CHANGELOG.md

@@ -6,7 +6,8 @@
 
 ### Breaking Changes
 
-_None_
+* Update the API of `ios_merge_strings_files` and `ios_extract_keys_from_strings_files` to support using prefixes for string keys when merging/splitting the files.
+  The actions now expect a `Hash` (instead of an `Array`) for the list of files to provide an associated prefix (or `nil` or `''` when none) for each file to merge/split. [#345]
 
 ### New Features
 

lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_extract_keys_from_strings_files.rb

@@ -3,30 +3,40 @@ module Actions
     class IosExtractKeysFromStringsFilesAction < Action
       def self.run(params)
         source_parent_dir = params[:source_parent_dir]
-        target_original_files = params[:target_original_files]
-        keys_to_extract_per_target_file = keys_list_per_target_file(target_original_files)
+        target_original_files = params[:target_original_files].keys # Array [original-file-paths]
+        keys_to_extract_per_target_file = keys_list_per_target_file(target_original_files) # Hash { original-file-path => [keys] }
+        prefix_to_remove_per_target_file = params[:target_original_files] # Hash { original-file-path => prefix }
+
+        UI.message("Extracting keys from `#{source_parent_dir}/*.lproj/#{params[:source_tablename]}.strings` into:")
+        target_original_files.each { |f| UI.message(' - ' + replace_lproj_in_path(f, with_lproj: '*.lproj')) }
+
+        updated_files_list = []
 
         # For each locale, extract the right translations from `<source_tablename>.strings` into each target `.strings` file
         Dir.glob('*.lproj', base: source_parent_dir).each do |lproj_dir_name|
           source_strings_file = File.join(source_parent_dir, lproj_dir_name, "#{params[:source_tablename]}.strings")
           translations = Fastlane::Helper::Ios::L10nHelper.read_strings_file_as_hash(path: source_strings_file)
 
           target_original_files.each do |target_original_file|
-            target_strings_file = File.join(File.dirname(File.dirname(target_original_file)), lproj_dir_name, File.basename(target_original_file))
+            target_strings_file = replace_lproj_in_path(target_original_file, with_lproj: lproj_dir_name)
             next if target_strings_file == target_original_file # do not generate/overwrite the original locale itself
 
-            keys_to_extract = keys_to_extract_per_target_file[target_original_file]
-            extracted_translations = translations.slice(*keys_to_extract)
-            UI.message("Extracting #{extracted_translations.count} keys (out of #{keys_to_extract.count} expected) into #{target_strings_file}...")
+            keys_prefix = prefix_to_remove_per_target_file[target_original_file] || ''
+            keys_to_extract = keys_to_extract_per_target_file[target_original_file].map { |k| "#{keys_prefix}#{k}" }
+            extracted_translations = translations.slice(*keys_to_extract).transform_keys { |k| k.delete_prefix(keys_prefix) }
+            UI.verbose("Extracting #{extracted_translations.count} keys (out of #{keys_to_extract.count} expected) into #{target_strings_file}...")
 
             FileUtils.mkdir_p(File.dirname(target_strings_file)) # Ensure path up to parent dir exists, create it if not.
             Fastlane::Helper::Ios::L10nHelper.generate_strings_file_from_hash(translations: extracted_translations, output_path: target_strings_file)
+            updated_files_list.append(target_strings_file)
           rescue StandardError => e
             UI.user_error!("Error while writing extracted translations to `#{target_strings_file}`: #{e.message}")
           end
         rescue StandardError => e
           UI.user_error!("Error while reading the translations from source file `#{source_strings_file}`: #{e.message}")
         end
+
+        updated_files_list
       end
 
       # Pre-load the list of keys to extract for each target file.
@@ -43,6 +53,15 @@ def self.keys_list_per_target_file(original_files)
         UI.user_error!("Failed to read the keys to extract from originals file: #{e.message}")
       end
 
+      # Replaces the `*.lproj` component of the path to a `.strings` file with a different `.lproj` folder
+      #
+      # @param [String] path The path the the `.strings` file, assumed to be in a `.lproj` parent folder
+      # @param [String] with_lproj The new name of the `.lproj` parent folder to point to
+      #
+      def self.replace_lproj_in_path(path, with_lproj:)
+        File.join(File.dirname(File.dirname(path)), with_lproj, File.basename(path))
+      end
+
       #####################################################
       # @!group Documentation
       #####################################################
@@ -82,28 +101,27 @@ def self.available_options
                                        default_value: 'Localizable'),
           FastlaneCore::ConfigItem.new(key: :target_original_files,
                                        env_name: 'FL_IOS_EXTRACT_KEYS_FROM_STRINGS_FILES_TARGET_ORIGINAL_FILES',
-                                       description: 'The path(s) to the `<base-locale>.lproj/<target-tablename>.strings` file(s) for which we want to extract the keys to. ' \
-                                        + 'Each of those files should containing the original strings (typically `en` or `Base` locale) and will be used to determine which keys to extract from the `source_tablename`. ' \
-                                        + 'For each of those, the path(s) in which the translations will be extracted will be the files with the same basename in each of the other `*.lproj` sibling folders',
-                                       type: Array,
+                                       description: 'The path(s) to the `<base-locale>.lproj/<target-tablename>.strings` file(s) for which we want to extract the keys to, and the prefix to remove from their keys. ' \
+                                        + 'Each key in the Hash should point to a file containing the original strings (typically `en` or `Base` locale), and will be used to determine which keys to extract from the `source_tablename`. ' \
+                                        + 'For each key, the associated value is an optional prefix to remove from the keys (which can be useful if you used a prefix during `ios_merge_strings_files` to avoid duplicates). Can be nil or empty if no prefix was used during merge for that file.' \
+                                        + 'Note: For each entry, the path(s) in which the translations will be extracted to will be the files with the same basename as the key in each of the other `*.lproj` sibling folders. ',
+                                       type: Hash,
                                        verify_block: proc do |values|
                                          UI.user_error!('`target_original_files` must contain at least one path to an original `.strings` file.') if values.empty?
-                                         values.each do |v|
-                                           UI.user_error!("Path `#{v}` (found in `target_original_files`) does not exist.") unless File.exist?(v)
-                                           UI.user_error! "Expected `#{v}` (found in `target_original_files`) to be a path ending in a `*.lproj/*.strings`." unless File.extname(v) == '.strings' && File.extname(File.dirname(v)) == '.lproj'
+                                         values.each do |path, _|
+                                           UI.user_error!("Path `#{path}` (found in `target_original_files`) does not exist.") unless File.exist?(path)
+                                           UI.user_error! "Expected `#{path}` (found in `target_original_files`) to be a path ending in a `*.lproj/*.strings`." unless File.extname(path) == '.strings' && File.extname(File.dirname(path)) == '.lproj'
                                          end
                                        end),
         ]
       end
 
       def self.return_type
-        # Describes what type of data is expected to be returned
-        # see RETURN_TYPES in https://github.com/fastlane/fastlane/blob/master/fastlane/lib/fastlane/action.rb
-        nil
+        :array_of_strings
       end
 
       def self.return_value
-        # Freeform textual description of the return value
+        'The list of files which have been generated and written to disk by the action'
       end
 
       def self.authors

lib/fastlane/plugin/wpmreleasetoolkit/actions/ios/ios_merge_strings_files.rb

@@ -2,12 +2,18 @@ module Fastlane
   module Actions
     class IosMergeStringsFilesAction < Action
       def self.run(params)
-        UI.message "Merging strings files: #{params[:paths].inspect}"
+        destination = params[:destination]
+        # Include the destination as the first of the files (without key prefixes) to be included in the merged result
+        all_paths_list = { destination => nil }.merge(params[:paths_to_merge])
 
-        duplicates = Fastlane::Helper::Ios::L10nHelper.merge_strings(paths: params[:paths], output_path: params[:destination])
+        UI.message "Merging strings files #{all_paths_list.inspect}"
+
+        File.write(destination, '') unless File.exist?(destination) # Create empty destination file if it does not exist yet
+        duplicates = Fastlane::Helper::Ios::L10nHelper.merge_strings(paths: all_paths_list, output_path: params[:destination])
         duplicates.each do |dup_key|
           UI.important "Duplicate key found while merging the `.strings` files: `#{dup_key}`"
         end
+        UI.important 'Tip: To avoid those key conflicts, you might want to consider providing different prefixes in the `Hash` you used for the `paths:` parameter.' unless duplicates.empty?
         duplicates
       end
 
@@ -21,12 +27,11 @@ def self.description
 
       def self.details
         <<~DETAILS
-          Merge multiple `.strings` files into one.
+          Merge multiple `.strings` files into another one.
 
-          Especially useful to prepare a single `.strings` file merging strings from both `Localizable.strings` from
-          the app code — typically previously extracted from `ios_generate_strings_file_from_code` —
-          and string files like `InfoPlist.strings` — which values may not be generated from the codebase but
-          manually maintained by developers.
+          Especially useful to prepare a single `.strings` file merging string files like `InfoPlist.strings` — whose
+          content are typically manually maintained by developers — within the main `Localizable.strings` file — which
+          would have typically been previously generated from the codebase via `ios_generate_strings_file_from_code`.
 
           The action only supports merging files which are in the OpenStep (`"key" = "value";`) text format (which is
           the most common format for `.strings` files, and the one generated by `genstrings`), but can handle the case
@@ -38,19 +43,18 @@ def self.details
       def self.available_options
         [
           FastlaneCore::ConfigItem.new(
-            key: :paths,
-            env_name: 'FL_IOS_MERGE_STRINGS_FILES_PATHS',
-            description: 'The paths of all the `.strings` files to merge together',
-            type: Array,
+            key: :paths_to_merge,
+            env_name: 'FL_IOS_MERGE_STRINGS_FILES_PATHS_TO_MERGE',
+            description: 'A hash of the paths of all the `.strings` files to merge into the `destination`, with the prefix to be used for their keys as associated value',
+            type: Hash,
             optional: false
           ),
           FastlaneCore::ConfigItem.new(
             key: :destination,
             env_name: 'FL_IOS_MERGE_STRINGS_FILES_DESTINATION',
-            description: 'The path of the merged `.strings` file to generate. If nil, the merge will happen in-place in the first file in the `paths:` list',
+            description: 'The path of the `.strings` file to merge the other ones into',
             type: String,
-            optional: true,
-            default_value: nil
+            optional: false
           ),
         ]
       end
@@ -60,7 +64,7 @@ def self.return_type
       end
 
       def self.return_value
-        'The list of duplicate keys found while merging the various `.strings` files'
+        'The list of duplicate keys (after prefix has been added to each) found while merging the various `.strings` files'
       end
 
       def self.authors

lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb

@@ -19,6 +19,8 @@ class L10nHelper
         #         - `nil` if the file does not exist or is neither of those format (e.g. not a `.strings` file at all)
         #
         def self.strings_file_type(path:)
+          return :text if File.exist?(path) && File.size(path).zero? # If completely empty file, consider it as a valid `.strings` files in textual format
+
           # Start by checking it seems like a valid property-list file (and not e.g. an image or plain text file)
           _, status = Open3.capture2('/usr/bin/plutil', '-lint', path)
           return nil unless status.success?
@@ -36,8 +38,8 @@ def self.strings_file_type(path:)
 
         # Merge the content of multiple `.strings` files into a new `.strings` text file.
         #
-        # @param [Array<String>] paths The paths of the `.strings` files to merge together
-        # @param [String] into The path to the merged `.strings` file to generate as a result.
+        # @param [Hash<String, String>] paths The paths of the `.strings` files to merge together, associated with the prefix to prepend to each of their respective keys
+        # @param [String] output_path The path to the merged `.strings` file to generate as a result.
         # @return [Array<String>] List of duplicate keys found while validating the merge.
         #
         # @note For now, this method only supports merging `.strings` file in `:text` format
@@ -48,24 +50,35 @@ def self.strings_file_type(path:)
         #
         # @raise [RuntimeError] If one of the paths provided is not in text format (but XML or binary instead), or if any of the files are missing.
         #
-        def self.merge_strings(paths:, output_path: nil)
+        def self.merge_strings(paths:, output_path:)
           duplicates = []
           Tempfile.create('wpmrt-l10n-merge-', encoding: 'utf-8') do |tmp_file|
             all_keys_found = []
 
             tmp_file.write("/* Generated File. Do not edit. */\n\n")
-            paths.each do |input_file|
+            paths.each do |input_file, prefix|
+              next if File.exist?(input_file) && File.size(input_file).zero? # Accept but skip existing-but-totally-empty files (to avoid adding useless `MARK:` comment for them)
+
               fmt = strings_file_type(path: input_file)
               raise "The file `#{input_file}` does not exist or is of unknown format." if fmt.nil?
               raise "The file `#{input_file}` is in #{fmt} format but we currently only support merging `.strings` files in text format." unless fmt == :text
 
-              string_keys = read_strings_file_as_hash(path: input_file).keys
+              string_keys = read_strings_file_as_hash(path: input_file).keys.map { |k| "#{prefix}#{k}" }
               duplicates += (string_keys & all_keys_found) # Find duplicates using Array intersection, and add those to duplicates list
               all_keys_found += string_keys
 
               tmp_file.write("/* MARK: - #{File.basename(input_file)} */\n\n")
               # Read line-by-line to reduce memory footprint during content copy; Be sure to guess file encoding using the Byte-Order-Mark.
-              File.readlines(input_file, mode: 'rb:BOM|UTF-8').each { |line| tmp_file.write(line) }
+              File.readlines(input_file, mode: 'rb:BOM|UTF-8').each do |line|
+                unless prefix.nil? || prefix.empty?
+                  # We need to ensure the line and RegExp are using the same encoding, so we transcode everything to UTF-8.
+                  line.encode!(Encoding::UTF_8)
+                  # The `/u` modifier on the RegExps is to make them UTF-8
+                  line.gsub!(/^(\s*")/u, "\\1#{prefix}") # Lines starting with a quote are considered to be start of a key; add prefix right after the quote
+                  line.gsub!(/^(\s*)([A-Z0-9_]+)(\s*=\s*")/ui, "\\1\"#{prefix}\\2\"\\3") # Lines starting with an identifier followed by a '=' are considered to be an unquoted key (typical in InfoPlist.strings files for example)
+                end
+                tmp_file.write(line)
+              end
               tmp_file.write("\n")
             end
             tmp_file.close # ensure we flush the content to disk
@@ -81,6 +94,8 @@ def self.merge_strings(paths:, output_path: nil)
         # @raise [RuntimeError] If the file is not a valid strings file or there was an error in parsing its content.
         #
         def self.read_strings_file_as_hash(path:)
+          return {} if File.exist?(path) && File.size(path).zero? # Return empty hash if completely empty file
+
           output, status = Open3.capture2e('/usr/bin/plutil', '-convert', 'json', '-o', '-', path)
           raise output unless status.success?