Skip to content

feat: enhance localization handling by adding xcstringsParsingKey method and updating parsing logic#394

Open
serhii-londar wants to merge 2 commits intomasterfrom
#371
Open

feat: enhance localization handling by adding xcstringsParsingKey method and updating parsing logic#394
serhii-londar wants to merge 2 commits intomasterfrom
#371

Conversation

@serhii-londar
Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings April 24, 2026 11:29
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves how the SDK resolves the localization key used when parsing .xcstrings files, particularly to support Crowdin custom languages whose folder locale can differ from the locale stored inside the xcstrings JSON.

Changes:

  • Add ManifestManager.xcstringsParsingKey(for:) to derive the correct xcstrings localization key (custom vs standard languages).
  • Update .xcstrings parsing and download flow to use the derived parsing key instead of the raw iOS localization value.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
Sources/CrowdinSDK/Providers/Crowdin/ManifestManager/ManifestManager+LanguageResolver.swift Introduces xcstringsParsingKey(for:) to map an iOS localization to the key used inside .xcstrings files (custom-language aware).
Sources/CrowdinSDK/Providers/Crowdin/LocalizationDownloader/CrowdinLocalizationDownloader.swift Uses xcstringsParsingKey(for:) when parsing cached xcstrings and when downloading/parsing xcstrings files.
Comments suppressed due to low confidence (1)

Sources/CrowdinSDK/Providers/Crowdin/LocalizationDownloader/CrowdinLocalizationDownloader.swift:88

  • Typo in local variable name: xcStringsFilesToDownlaod is misspelled, which makes the code harder to read/search. Rename to xcStringsFilesToDownload (and update references) for clarity.
                let xcStringsFilesToDownlaod = xcstringsFiles.filter({ self.manifestManager.hasFileChanged(filePath: $0, localization: self.manifestManager.xcstringsLanguage) })
                let filesToDownload = xcStringsFilesToDownlaod + notXcstringsFilesToDownload

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 3.52941% with 82 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.60%. Comparing base (30455af) to head (171259d).

Files with missing lines Patch % Lines
...s/Tests/CrowdinProvider/ManifestManagerTests.swift 0.00% 65 Missing ⚠️
...festManager/ManifestManager+LanguageResolver.swift 0.00% 16 Missing ⚠️
...tionDownloader/CrowdinLocalizationDownloader.swift 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #394      +/-   ##
==========================================
- Coverage   57.30%   56.60%   -0.69%     
==========================================
  Files         134      134              
  Lines        6540     6622      +82     
==========================================
+ Hits         3747     3748       +1     
- Misses       2793     2874      +81     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Fix spelling: 'wont' -> 'won't' in CrowdinLocalizationDownloader
- Normalize unknown language fallback to BCP 47 in xcstringsParsingKey
- Return language.iOSLanguageCode for standard languages (ensures BCP 47 output)
- Update doc comment to clarify expected input/output formats
- Add unit tests for standard, underscore-normalized, and custom language cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants