From 3a68c64c2199a8385b0fc26eb95ca2c49a6a541d Mon Sep 17 00:00:00 2001 From: Stelios Petrakis Date: Tue, 12 Mar 2024 14:45:44 +0100 Subject: [PATCH 1/2] Full String Catalog support With the introduction of the String Catalogs file format in Xcode 15, developers have now the option to create localizations that vary by device and/or by plural. On top of that, multiple tokens can be added in a localized phrase (also known as substitutions) that can also support the aforementioned variations. In order to support all those new formats as well as the old substitution rules found in the Strings Dictionary format (`.stringsdict`), the advanced rules from the CLI tool are formatted as an intermediate XML structure and pushed to CDS. This commit adds this functionality so that complex variation rules can be supported and displayed in the Transifex web interface. More unit tests have been added to test those cases as well as edge-case scenarios and existing unit tests have been improved. --- Package.resolved | 4 +- Package.swift | 2 +- Sources/TXCli/main.swift | 9 +- Sources/TXCliLib/XLIFFParser.swift | 279 ++++++++++++++++-------- Tests/TXCliTests/XLIFFParserTests.swift | 248 ++++++++++++++++++++- 5 files changed, 447 insertions(+), 95 deletions(-) diff --git a/Package.resolved b/Package.resolved index 5a9b922..ba66807 100644 --- a/Package.resolved +++ b/Package.resolved @@ -33,8 +33,8 @@ "repositoryURL": "https://github.com/transifex/transifex-swift", "state": { "branch": null, - "revision": "4490b3ed7abae304e9bb3fed02882320b1df224e", - "version": "2.0.1" + "revision": "b85d7c82966e820ac6e24cbf3f595b0dd02014aa", + "version": "2.0.2" } } ] diff --git a/Package.swift b/Package.swift index 634d887..157956c 100644 --- a/Package.swift +++ b/Package.swift @@ -15,7 +15,7 @@ let package = Package( dependencies: [ .package(name: "transifex", url: "https://github.com/transifex/transifex-swift", - from: "2.0.0"), + from: "2.0.2"), .package(url: "https://github.com/apple/swift-argument-parser", from: "0.3.0"), .package(url: "https://github.com/kiliankoe/CLISpinner", diff --git a/Sources/TXCli/main.swift b/Sources/TXCli/main.swift index 7656229..bf510cb 100644 --- a/Sources/TXCli/main.swift +++ b/Sources/TXCli/main.swift @@ -247,10 +247,13 @@ Emulate a content push, without doing actual changes. // ICU format and use that as a source string switch result.generateICURuleIfPossible() { case .success((let icuRule, let icuRuleType)): - // Only support plural rule type for now - if icuRuleType == .Plural { - sourceString = icuRule + if icuRuleType == TranslationUnit.ICURuleType.Other { + logHandler.error("Error: ICU rule type could not be detected.") + // Do not add a translation unit in case of a non-detected + // ICU rule type. + continue } + sourceString = icuRule case .failure(let error): switch error { case .noRules: diff --git a/Sources/TXCliLib/XLIFFParser.swift b/Sources/TXCliLib/XLIFFParser.swift index 421cd76..98828f4 100644 --- a/Sources/TXCliLib/XLIFFParser.swift +++ b/Sources/TXCliLib/XLIFFParser.swift @@ -207,9 +207,6 @@ extension TranslationUnit: Equatable { } extension TranslationUnit { - private static let LOCALIZED_FORMAT_KEY_PREFIX = "%#@" - private static let LOCALIZED_FORMAT_KEY_SUFFIX:Character = "@" - /// Tags used by Apple's .xcstrings format private static let XCSTRINGS_PLURAL_RULE_PREFIX = "plural" private static let XCSTRINGS_DEVICE_RULE_PREFIX = "device" @@ -217,13 +214,13 @@ extension TranslationUnit { /// The types of the generated ICU rule by the `generateICURuleIfPossible` method. public enum ICURuleType { - // Pluralization + // Pluralization (Simple plural rule, supported by CDS in ICU format) case Plural - // Vary by device + // Vary by device (Converted to XML for CDS) case Device - // Substitution (multiple variables) + // Substitution (multiple variables) (Converted to XML for CDS) case Substitutions - // Something unexpected / not yet supported + // Unexpected/empty rule encountered case Other } @@ -255,80 +252,164 @@ extension TranslationUnit { } /// If the current `TranslationUnit` contains a number of `PluralizationRule` objects in its - /// property, then the method attempts to generate an ICU rule out of them that can be pushed to CDS. + /// property, then the method attempts to generate an ICU rule out of them that can be pushed to CDS + /// either as a single ICU rule or as an intermediate XML structure. /// /// - Returns: The ICU pluralization rule if its generation is possible, nil otherwise. public func generateICURuleIfPossible() -> Result<(String, ICURuleType), ICUError> { guard pluralizationRules.count > 0 else { return .failure(.noRules) } - - var icuRules : [String] = [] - let activeStringsSourceType = pluralizationRules.map { $0.stringsSourceType }.first - - // For the legacy .stringsdict format, require the localized format key - // to have the %#@[KEY]@ format. Otherwise do not process it. - // As per documentation: - // > If the formatted string contains multiple variables, enter a separate subdictionary for each variable. - // Ref: https://developer.apple.com/documentation/xcode/localizing-strings-that-contain-plurals - // So for example, the following is correct: - // - // - // %#@lu_devices@ - // %#@lu_devices@ - // - // - // - // Message is sent to %lu device. - // Message is sent to %lu device. - // - // - // - // Message is sent to %lu devices. - // Message is sent to %lu devices. - // - // - // - // while this is wrong: - // - // - // Message is sent to %#@lu_devices@. - // Message is sent to %#@lu_devices@. - // - // - // - // %lu device - // %lu device - // - // - // - // %lu devices - // %lu devices - // - // - if activeStringsSourceType == .StringsDict, - let target = pluralizationRules.filter({ $0.containsLocalizedFormatKey}).first?.target, - !(target.starts(with: Self.LOCALIZED_FORMAT_KEY_PREFIX) && target.last == Self.LOCALIZED_FORMAT_KEY_SUFFIX) { - return .failure(.malformedPluralizedFormat(target)) + guard let activeStringsSourceType = pluralizationRules.map({ $0.stringsSourceType }).first else { + return .failure(.noRules) } - var isICUFriendly = false + var icuRuleType: ICURuleType = .Other if activeStringsSourceType == .StringsDict { - isICUFriendly = true + // For the legacy .stringsdict format, if the localized format key + // contains more than two tokens, then it means that it contains + // substitutions. In this case we want to convert it to XML just + // like we do with .xcstrings substitutions. + // + // As per documentation: + // > If the formatted string contains multiple variables, enter a + // > separate subdictionary for each variable. + // + // Ref: https://developer.apple.com/documentation/xcode/localizing-strings-that-contain-plurals + if let target = pluralizationRules.filter({ $0.containsLocalizedFormatKey}).first?.target { + let tokenCount = PluralUtils.extractTokens(from: target).count + if tokenCount > 1 { + icuRuleType = .Substitutions + } + else if tokenCount == 1 { + icuRuleType = .Plural + } + } + } + else { + // Simple plural .xcstrings rules can be converted to a single ICU + // rule + if let pluralRule = pluralizationRules.first?.pluralRule, + pluralRule.starts(with: "\(Self.XCSTRINGS_PLURAL_RULE_PREFIX).") { + icuRuleType = .Plural + } + else { + if let rule = pluralizationRules.first?.pluralRule?.components(separatedBy: ".").first { + switch rule { + case Self.XCSTRINGS_DEVICE_RULE_PREFIX: + icuRuleType = .Device + case Self.XCSTRINGS_SUBSTITUTIONS_RULE_PREFIX: + icuRuleType = .Substitutions + default: + icuRuleType = .Other + } + } + } } - else if let pluralRule = pluralizationRules.first?.pluralRule, - pluralRule.starts(with: "\(Self.XCSTRINGS_PLURAL_RULE_PREFIX).") { - isICUFriendly = true + + guard icuRuleType != .Other else { + return .failure(.notSupported(.Other)) } - if isICUFriendly { - for pluralizationRule in pluralizationRules { - if pluralizationRule.containsLocalizedFormatKey { - continue - } + var cdsRule: String? = nil + + if icuRuleType == .Plural { + // Single ICU rules are supported by CDS, so convert it and send it + // like that. + cdsRule = generateSingleICURule(pluralizationRules) + } + else { + // Otherwise convert all the plural rules to XML so that the CDS web + // UI can render them and then process them in the SDK when fetched. + cdsRule = generateXMLRule(pluralizationRules, + type: activeStringsSourceType) + } + + guard let cdsRule = cdsRule else { + return .failure(.emptyRule) + } + + return .success((cdsRule, icuRuleType)) + } + + /// For simple plural variations that just contain one plural rule which covers the whole phrase, we + /// generate the ICU rule in the format that is accepted by CDS. + /// + /// - Parameter pluralizationRules: The pluralization rules that make up the plural variation + /// - Returns: The generated ICU rule as a String, nil in case of an error (if no pluralization rules + /// could be processed). + private func generateSingleICURule(_ pluralizationRules: [PluralizationRule]) -> String? { + var icuRules : [String] = [] + + for pluralizationRule in pluralizationRules { + if pluralizationRule.containsLocalizedFormatKey { + continue + } + + guard let pluralRule = pluralizationRule.pluralRule else { + continue + } + + guard let target = pluralizationRule.target else { + continue + } + + let normalizedRule = pluralRule.replacingOccurrences(of: "\(Self.XCSTRINGS_PLURAL_RULE_PREFIX).", + with: "") + icuRules.append("\(normalizedRule) {\(target)}") + } + guard icuRules.count > 0 else { + return nil + } + + return "{cnt, plural, \(icuRules.joined(separator: " "))}" + } + + /// For any complex variations (device, multiple plurals, tokens and their combinations), we generate + /// an intermediate XML structure that will be rendered in the Transifex web interface accordingly and + /// then processed by the SDK when pulled. + /// + /// - Parameters: + /// - pluralizationRules: The pluralization rules that make up the complex variation. + /// - type: The type governing the pluralization rules + /// - Returns: The generated XML structure as a String, nil in case of an error (if no XML children + /// could be generated). + private func generateXMLRule(_ pluralizationRules: [PluralizationRule], + type: PluralizationRule.StringsSourceType) -> String? { + switch type { + case .XCStrings: + let root = XMLElement(name: TXNative.CDS_XML_ROOT_TAG_NAME) + + // Used for substitutions where the first trans-unit contains the + // phrase. + // e.g. + // ``` + // + // Found %1$#@arg1@ having %2$#@arg2@ + // Found %1$#@arg1@ having %2$#@arg2@ + // + // + // + // %1$ld user + // %1$ld user + // + // + // ... + // ``` + if target != id { + if let attribute = XMLNode.attribute(withName: TXNative.CDS_XML_ID_ATTRIBUTE, + stringValue: Self.XCSTRINGS_SUBSTITUTIONS_RULE_PREFIX) as? XMLNode { + let xmlElement = XMLElement(name: TXNative.CDS_XML_TAG_NAME, + stringValue: target) + xmlElement.addAttribute(attribute) + root.addChild(xmlElement) + } + } + + for pluralizationRule in pluralizationRules { guard let pluralRule = pluralizationRule.pluralRule else { continue } @@ -337,32 +418,60 @@ extension TranslationUnit { continue } - let normalizedRule = pluralRule.replacingOccurrences(of: "\(Self.XCSTRINGS_PLURAL_RULE_PREFIX).", - with: "") - icuRules.append("\(normalizedRule) {\(target)}") + if let attribute = XMLNode.attribute(withName: TXNative.CDS_XML_ID_ATTRIBUTE, + stringValue: pluralRule) as? XMLNode { + let xmlElement = XMLElement(name: TXNative.CDS_XML_TAG_NAME, + stringValue: target) + xmlElement.addAttribute(attribute) + root.addChild(xmlElement) + } } + + return root.childCount > 0 ? root.xmlString : nil + case .StringsDict: + // For the legacy substitutions, we attempt to convert the XML tags + // to the format of the `.xcstrings` above, so that the SDK can + // parse both of them, regardless of their initial type. + // + // This means that the pluralization rule that contains the localized + // format key (.containsLocalizedFormatKey == true), will become the + // main substitutions phrase, and have an id of "substitutions". + // Each of the other rules, will have an id that follows the format: + // "substitutions.PLURAL_KEY.plural.PLURAL_RULE". + let root = XMLElement(name: TXNative.CDS_XML_ROOT_TAG_NAME) - guard icuRules.count > 0 else { - return .failure(.emptyRule) - } + for pluralizationRule in pluralizationRules { + guard let target = pluralizationRule.target else { + continue + } - return .success(("{cnt, plural, \(icuRules.joined(separator: " "))}", .Plural)) - } - else { - var icuRuleType: ICURuleType = .Other + if pluralizationRule.containsLocalizedFormatKey { + if let attribute = XMLNode.attribute(withName: TXNative.CDS_XML_ID_ATTRIBUTE, + stringValue: Self.XCSTRINGS_SUBSTITUTIONS_RULE_PREFIX) as? XMLNode { + let xmlElement = XMLElement(name: TXNative.CDS_XML_TAG_NAME, + stringValue: target) + xmlElement.addAttribute(attribute) + root.addChild(xmlElement) + } + continue + } - if let rule = pluralizationRules.first?.pluralRule?.components(separatedBy: ".").first { - switch rule { - case Self.XCSTRINGS_DEVICE_RULE_PREFIX: - icuRuleType = .Device - case Self.XCSTRINGS_SUBSTITUTIONS_RULE_PREFIX: - icuRuleType = .Substitutions - default: - icuRuleType = .Other + guard let pluralRule = pluralizationRule.pluralRule, + let pluralKey = pluralizationRule.pluralKey else { + continue + } + + let id = "\(Self.XCSTRINGS_SUBSTITUTIONS_RULE_PREFIX).\(pluralKey).\(Self.XCSTRINGS_PLURAL_RULE_PREFIX).\(pluralRule)" + if let attribute = XMLNode.attribute(withName: TXNative.CDS_XML_ID_ATTRIBUTE, + stringValue: id) as? XMLNode { + let xmlElement = XMLElement(name: TXNative.CDS_XML_TAG_NAME, + stringValue: target) + xmlElement.addAttribute(attribute) + root.addChild(xmlElement) } } - return .failure(.notSupported(icuRuleType)) + return root.childCount > 0 ? root.xmlString : nil } } } diff --git a/Tests/TXCliTests/XLIFFParserTests.swift b/Tests/TXCliTests/XLIFFParserTests.swift index aea8063..61834fc 100644 --- a/Tests/TXCliTests/XLIFFParserTests.swift +++ b/Tests/TXCliTests/XLIFFParserTests.swift @@ -206,6 +206,41 @@ final class XLIFFParserTests: XCTestCase { %d minutes + + %#@num_people_in_room@ in %#@room@ + %#@num_people_in_room@ in %#@room@ + + + + Only %d person + Only %d person + + + + Some people + Some people + + + + No people + No people + + + + %d room + %d room + + + + %d rooms + %d rooms + + + + no room + no room + + @@ -224,16 +259,217 @@ final class XLIFFParserTests: XCTestCase { let results = xliffParser!.results + XCTAssertTrue(results.count == 2) + + do { + let result = results[0] + XCTAssertEqual(result.pluralizationRules.count, 7) + let icuRule = try result.generateICURuleIfPossible().get() + let expectedIcuRule = "%#@num_people_in_room@ in %#@room@Only %d personSome peopleNo people%d room%d roomsno room" + XCTAssertEqual(icuRule.0, expectedIcuRule) + XCTAssertEqual(icuRule.1, .Substitutions) + } + + do { + let result = results[1] + XCTAssertEqual(result.pluralizationRules.count, 3) + + let icuRule = try result.generateICURuleIfPossible().get() + let expectedIcuRule = "{cnt, plural, one {%d minute} other {%d minutes}}" + + XCTAssertEqual(icuRule.0, expectedIcuRule) + XCTAssertEqual(icuRule.1, .Plural) + } + } + + func testXLIFFParserWithXCStringsSubstitutions() throws { + let fileURL = tempXLIFFFileURL() + let sampleXLIFF = """ + + + +
+ +
+ + + Found %1$#@arg1@ having %2$#@arg2@ + Found %1$#@arg1@ having %2$#@arg2@ + + + + %1$ld user + %1$ld user + + + + %1$ld users + %1$ld users + + + + %2$ld device + %2$ld device + + + + %2$ld devices + %2$ld devices + + + + + +""" + do { + try sampleXLIFF.write(to: fileURL, atomically: true, encoding: .utf8) + } + catch { } + + let xliffParser = XLIFFParser(fileURL: fileURL) + XCTAssertNotNil(xliffParser, "Failed to initialize parser") + + let parsed = xliffParser!.parse() + + XCTAssertTrue(parsed) + + let results = xliffParser!.results + XCTAssertTrue(results.count == 1) - XCTAssertTrue(results.first!.pluralizationRules.count == 3) - + XCTAssertEqual(results.first!.pluralizationRules.count, 4) + let icuRule = try results.first!.generateICURuleIfPossible().get() - let expectedIcuRule = "{cnt, plural, one {%d minute} other {%d minutes}}" + let expectedIcuRule = "Found %1$#@arg1@ having %2$#@arg2@%1$ld user%1$ld users%2$ld device%2$ld devices" XCTAssertEqual(icuRule.0, expectedIcuRule) - XCTAssertEqual(icuRule.1, .Plural) + XCTAssertEqual(icuRule.1, .Substitutions) + } + + func testXLIFFParserWithXCStringsDevices() throws { + let fileURL = tempXLIFFFileURL() + let sampleXLIFF = """ + + + +
+ +
+ + + This is Apple Vision + This is Apple Vision + + + + This is an Apple Watch + This is an Apple Watch + + + + This is an iPhone + This is an iPhone + + + + This is a Mac + This is a Mac + + + + This is a device + This is a device + + + + + +""" + do { + try sampleXLIFF.write(to: fileURL, atomically: true, encoding: .utf8) + } + catch { } + + let xliffParser = XLIFFParser(fileURL: fileURL) + XCTAssertNotNil(xliffParser, "Failed to initialize parser") + + let parsed = xliffParser!.parse() + + XCTAssertTrue(parsed) + + let results = xliffParser!.results + + XCTAssertTrue(results.count == 1) + + XCTAssertEqual(results.first!.pluralizationRules.count, 5) + + let icuRule = try results.first!.generateICURuleIfPossible().get() + + let expectedIcuRule = "This is Apple VisionThis is an Apple WatchThis is an iPhoneThis is a MacThis is a device" + + XCTAssertEqual(icuRule.0, expectedIcuRule) + XCTAssertEqual(icuRule.1, .Device) + } + + func testXLIFFParserWithXCStringsSpecial() throws { + let fileURL = tempXLIFFFileURL() + let sampleXLIFF = """ + + + +
+ +
+ + + Device has %1$#@arg1_iphone@ in %2$ld folders + Device has %1$#@arg1_iphone@ in %2$ld folders + + + + Device has %ld users in %ld folders + Device has %ld users in %ld folders + + + + %ld user + %ld user + + + + %ld users + %ld users + + + + + +""" + do { + try sampleXLIFF.write(to: fileURL, atomically: true, encoding: .utf8) + } + catch { } + + let xliffParser = XLIFFParser(fileURL: fileURL) + XCTAssertNotNil(xliffParser, "Failed to initialize parser") + + let parsed = xliffParser!.parse() + + XCTAssertTrue(parsed) + + let results = xliffParser!.results + + XCTAssertTrue(results.count == 1) + + XCTAssertEqual(results.first!.pluralizationRules.count, 4) + + let icuRule = try results.first!.generateICURuleIfPossible().get() + + let expectedIcuRule = "Device has %1$#@arg1_iphone@ in %2$ld foldersDevice has %ld users in %ld folders%ld user%ld users" + + XCTAssertEqual(icuRule.0, expectedIcuRule) + XCTAssertEqual(icuRule.1, .Device) } func testXLIFFResultConsolidation() throws { @@ -355,7 +591,11 @@ final class XLIFFParserTests: XCTestCase { static var allTests = [ ("testXLIFFParser", testXLIFFParser), + ("testXLIFFParserWithXCStrings", testXLIFFParserWithXCStrings), ("testXLIFFParserWithStringsDict", testXLIFFParserWithStringsDict), + ("testXLIFFParserWithXCStringsSubstitutions", testXLIFFParserWithXCStringsSubstitutions), + ("testXLIFFParserWithXCStringsDevices", testXLIFFParserWithXCStringsDevices), + ("testXLIFFParserWithXCStringsSpecial", testXLIFFParserWithXCStringsSpecial), ("testXLIFFResultConsolidation", testXLIFFResultConsolidation), ("testXLIFFParserWithQuotes", testXLIFFParserWithQuotes), ] From 532ec03810fb2100008a28ccd583274536710846 Mon Sep 17 00:00:00 2001 From: Stelios Petrakis Date: Wed, 29 May 2024 10:37:09 +0200 Subject: [PATCH 2/2] Bump version to 2.1.6 * Bumps CLI version to 2.1.6. * Updates the CHANGELOG. --- CHANGELOG.md | 8 ++++++++ Sources/TXCli/main.swift | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c343620..882e84e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -137,3 +137,11 @@ set to `false`. - Parses and processes the new `.xcstrings` files. Only supports simple "plural." rules for now. + +## Transifex Command Line Tool 2.1.6 + +*May 29, 2024* + +- Adds full support for String Catalogs support. +- Adds support for substitution phrases on old Strings Dictionary file format. +- Updates unit tests. diff --git a/Sources/TXCli/main.swift b/Sources/TXCli/main.swift index bf510cb..a16cf1b 100644 --- a/Sources/TXCli/main.swift +++ b/Sources/TXCli/main.swift @@ -42,7 +42,7 @@ that can be bundled with the iOS application. The tool can be also used to force CDS cache invalidation so that the next pull command will fetch fresh translations from CDS. """, - version: "2.1.5", + version: "2.1.6", subcommands: [Push.self, Pull.self, Invalidate.self]) }