Migrate to Apple-native string catalog and codegen LocalizedStringResources with xcstrings-tool-plugin #738

tyiu · 2023-12-15T03:31:33Z

Closes: #718

Previously,
English strings were written in Nos/Assets/Localization/Localized.swift on the Localized enum, and then Generated.strings were generated for each locale that Nos supports, which could be localized and read in code. Argument substitutions were done in a bespoke way using regex. Locales (such as Arabic) that did not follow the pluralization rules of English would not be able to localize properly. There were no comments to provide context to translators, which probably made their work more difficult.

Now,
Nos/Assets/Localization/*.xcstrings (string catalog) files are used instead. String catalogs were introduced in Xcode 15 and they are supported natively, including argument substitution and localized plural rules. Each xcstrings file contains the translations of all locales. Crowdin has support for xcstrings files, and will write translations into the same file. I opted to use an MIT-licensed open source plugin called xcstrings-tool that transforms xcstrings files to generated code, so that we can reference LocalizedStringResources in a type-safe way. There's also now a way to provide comments to strings so that translators have context when translating.

I copied all the current translations from Crowdin to the string catalog files. There was a bit of manual work here so hopefully I got it all right.

Someone who has access to the Crowdin project will need to make sure the integration works fine with these changes. I did some testing on my local Crowdin project and it seemed to work. Keep in mind that the history of the past translation work and translator attribution will probably get wiped out.

There also seems to be a bug where Xcode can occasionally crash when entering strings with arguments in the string catalog editor for non-default locales. The workaround for now is to edit in a text editor. But typically translators won't be interacting with Xcode, just Crowdin.

English	German	Chinese (Traditional)

Xcode String Catalog Editor

Generated code from xcstrings-tool

tyiu · 2023-12-15T18:24:20Z

Nos/Views/AuthorStoryView.swift

@@ -251,9 +251,7 @@ fileprivate struct BottomOverlay: View {
        if replyCount == 0 {
            return nil
        }
-        let replyCount = replyCount
-        let localized = replyCount == 1 ? Localized.Reply.one : Localized.Reply.many


This logic did not work for locales that support different pluralization rules than just one and many.

tyiu · 2023-12-15T18:24:50Z

Nos/Views/RepostButton.swift

@@ -57,16 +57,16 @@ struct RepostButton: View {
            .padding(.vertical, 12)
        }
        .disabled(tapped)
-        .confirmationDialog("Repost", isPresented: $shouldConfirmRepost) { 


Hardcoded string that was not localized.

tyiu · 2023-12-15T18:24:55Z

Nos/Views/RepostButton.swift

@@ -57,16 +57,16 @@ struct RepostButton: View {
            .padding(.vertical, 12)
        }
        .disabled(tapped)
-        .confirmationDialog("Repost", isPresented: $shouldConfirmRepost) { 
-            Button("Repost") { 


Hardcoded string that was not localized.

tyiu · 2023-12-15T18:25:09Z

Nos/Views/RepostButton.swift

                tapped = false
            }
        }
-        .confirmationDialog("Delete repost", isPresented: $shouldConfirmDelete) { 


Hardcoded string that was not localized.

tyiu · 2023-12-15T18:25:14Z

Nos/Views/RepostButton.swift

                tapped = false
            }
        }
-        .confirmationDialog("Delete repost", isPresented: $shouldConfirmDelete) { 
-            Button("Delete repost", role: .destructive) { 


Hardcoded string that was not localized.

tyiu · 2023-12-15T18:26:28Z

Nos/Views/NoteCard.swift

@@ -42,9 +42,7 @@ struct NoteCard: View {
        if replyCount == 0 {
            return nil
        }
-        let replyCount = replyCount
-        let localized = replyCount == 1 ? Localized.Reply.one : Localized.Reply.many


This logic did not work for locales that support different pluralization rules than just one and many.

tyiu · 2023-12-15T18:27:25Z

Nos/Views/ProfileKnownFollowersView.swift

-                        "two": second.safeName,
-                        "count": "\(followers.count - 2)"
-                    ]))
+                    Text(.localizable.followedByTwoAndMore(first.safeName, second.safeName, followers.count - 2))


I wasn't able to figure out if named arguments could be supported. Leaving that as an open question.

tyiu · 2023-12-15T18:28:27Z

crowdin.yml

-    translation: /Nos/Assets/Localization/%osx_code%/%original_file_name%
+  - source: /Nos/Assets/Localization/ImagePicker.xcstrings
+    translation: /Nos/Assets/Localization/ImagePicker.xcstrings
+    multilingual: 1


Indicates that the file contains translations for multiple locales.

tyiu · 2023-12-18T01:13:44Z

Rebased on the main branch and resolved the conflicts. I also brought in new Persian translations that came in after I created this pull request.

mplorentz

Wow, this is a huge contribution and a big improvement over the previous system! Regarding the history here: I actually don't know why we were using a handmade ExportStrings.sh to do localization. It was set up for the Planetary app before I started, and when we started Nos we copied a lot of the strings over and with it came this system.

@martindsq can you give this a proper review since you manage the Crowdin integration and have a lot more expertise than me in this area?

tyiu · 2023-12-19T15:20:06Z

There seems to be an issue where Xcode formats the JSON in the *.xcstrings files with a space after the JSON key, but Crowdin does not. There will likely be some noise flip-flopping back and forth between the spacing discrepancies until either one of them decides to change how they write xcstrings files to be consistent with the other. This may cause merge conflicts, not sure what the best path forward is. I've filed an issue here and they've said that they're working on a fix: https://community.crowdin.com/t/apple-xcstrings-string-catalog-format-discrepancy-between-crowdin-and-xcode/5937

Crowdin has fixed the issue. I'm removing this blurb from the PR description.

tyiu · 2023-12-19T15:21:40Z

This PR should be rebased and adjusted after Tweak warning message to support localization #742 is merged to main with its localization code and string changes.

mplorentz · 2023-12-19T21:44:42Z

This PR should be rebased and adjusted after Tweak warning message to support localization #742 is merged to main with its localization code and string changes.

Yeah sorry about that. Just clicked merge on that PR. Also we have some tests failing on main, I will be addressing them tomorrow.

tyiu · 2023-12-20T00:27:18Z

This PR should be rebased and adjusted after Tweak warning message to support localization #742 is merged to main with its localization code and string changes.

Yeah sorry about that. Just clicked merge on that PR. Also we have some tests failing on main, I will be addressing them tomorrow.

No worries. I just rebased and resolved the conflicts..

tyiu · 2023-12-20T00:28:22Z

Nos/Views/PreviewData.swift

@@ -55,7 +55,7 @@ struct PreviewData {
    }()

    lazy var bob: Author = {
-        let author = Author(context: previewContext)
+        let author = try! Author.findOrCreate(by: KeyFixture.bob.publicKeyHex, context: previewContext)


I think this was a bug that bob wasn't in Core Data. alice and eve were though, so I copied what they did.

tyiu · 2023-12-20T00:28:56Z

Nos/Views/WarningView.swift

@@ -157,13 +157,13 @@ struct ContentWarningMessage: View {
    // Assuming there's a property or method 'safeName' in 'Author' that safely returns the author's name.
    private var firstAuthorSafeName: String {
        // Getting the safe name of the first author. Adjust according to your actual data structure.
-        reports.first?.author?.safeName ?? "Unknown Author"


Pulled out Unknown Author to localize it.

tyiu · 2023-12-20T00:29:09Z

Nos/Views/WarningView.swift

    }

    private var reason: String {
        // Extract content from reports and remove empty content
-        let contents = reports.compactMap { (($0.content?.isEmpty) != nil) ? nil : $0.content }


This was a bug. It would always return nil.

…ources with xcstrings-tool-plugin

tyiu · 2023-12-25T04:44:26Z

Rebased on the main branch, resolved conflicts, and added new translations for Chinese Simplified, Chinese Traditional, and German locales.

tyiu commented Dec 15, 2023

View reviewed changes

tyiu changed the title ~~Migrate to Apple-native strings catalog and codegen LocalizedStringResources with xcstrings-tool-plugin~~ Migrate to Apple-native string catalog and codegen LocalizedStringResources with xcstrings-tool-plugin Dec 18, 2023

tyiu force-pushed the tyiu-718 branch from 926bfc0 to 094b9e6 Compare December 18, 2023 01:12

mplorentz reviewed Dec 18, 2023

View reviewed changes

setch-l mentioned this pull request Dec 18, 2023

String localization is not fully internationalized #718

Closed

tyiu force-pushed the tyiu-718 branch from 094b9e6 to 1edcfb0 Compare December 19, 2023 00:04

tyiu force-pushed the tyiu-718 branch from 1edcfb0 to 98b0358 Compare December 20, 2023 00:26

tyiu commented Dec 20, 2023

View reviewed changes

Migrate to Apple-native string catalog and codegen LocalizedStringRes…

acbc10b

…ources with xcstrings-tool-plugin

tyiu force-pushed the tyiu-718 branch from 98b0358 to acbc10b Compare December 25, 2023 04:42

martindsq merged commit 0209599 into planetary-social:main Dec 28, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to Apple-native string catalog and codegen LocalizedStringResources with xcstrings-tool-plugin #738

Migrate to Apple-native string catalog and codegen LocalizedStringResources with xcstrings-tool-plugin #738

tyiu commented Dec 15, 2023 •

edited

Loading

tyiu Dec 15, 2023

tyiu Dec 15, 2023

tyiu Dec 15, 2023

tyiu Dec 15, 2023

tyiu Dec 15, 2023

tyiu Dec 15, 2023

tyiu Dec 15, 2023

tyiu Dec 15, 2023

tyiu commented Dec 18, 2023

mplorentz left a comment

tyiu commented Dec 19, 2023

tyiu commented Dec 19, 2023 •

edited

Loading

mplorentz commented Dec 19, 2023

tyiu commented Dec 20, 2023

tyiu Dec 20, 2023

tyiu Dec 20, 2023

tyiu Dec 20, 2023

tyiu commented Dec 25, 2023

Migrate to Apple-native string catalog and codegen LocalizedStringResources with xcstrings-tool-plugin #738

Migrate to Apple-native string catalog and codegen LocalizedStringResources with xcstrings-tool-plugin #738

Conversation

tyiu commented Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyiu commented Dec 18, 2023

mplorentz left a comment

Choose a reason for hiding this comment

tyiu commented Dec 19, 2023

tyiu commented Dec 19, 2023 • edited Loading

mplorentz commented Dec 19, 2023

tyiu commented Dec 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyiu commented Dec 25, 2023

tyiu commented Dec 15, 2023 •

edited

Loading

tyiu commented Dec 19, 2023 •

edited

Loading