Skip to content

feat(localization): add Korean support - #2237

Merged
datlechin merged 4 commits into
TableProApp:mainfrom
sophiathedev:feat/korean-localization
Aug 19, 2026
Merged

feat(localization): add Korean support#2237
datlechin merged 4 commits into
TableProApp:mainfrom
sophiathedev:feat/korean-localization

Conversation

@sophiathedev

Copy link
Copy Markdown
Contributor

Summary

  • add Korean as a selectable macOS language and complete Korean string catalogs for macOS, iPhone, and iPad
  • localize privacy prompts, App Shortcut phrases, widgets, runtime-generated labels, and shared package errors
  • add a Korean README, update the language links and settings documentation, and record the feature in the changelog
  • add regression coverage for catalog completeness, format arguments, privacy strings, App Shortcuts, shared package strings, settings persistence, and README links

Validation

  • scripts/localization.py verify
  • macOS catalog: 3,885 of 3,887 values translated; the remaining two are explicitly marked not translatable
  • iOS catalog: 565 of 565 values translated
  • compiled all five Korean string catalogs with xcstringstool
  • passed 20 focused AppLanguage, GeneralSettings, and Korean localization tests
  • parsed all 21 changed Swift files with the Swift frontend
  • passed strict SwiftLint for the new localization source test
  • git diff --check

Environment notes

  • the macOS app and unit-test bundle built successfully, but the full scheme stopped while linking the unrelated TableProUITests runner because the local linker could not write its output file; all focused unit tests passed from the freshly built bundle
  • the generic iOS build could not start because Xcode reports that the iOS 26.5 platform component is not installed; iOS sources were syntax-checked and the generated project resource phases and compiled catalogs were verified

Closes #2219

@sophiathedev
sophiathedev marked this pull request as ready for review August 19, 2026 10:06
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@datlechin

datlechin commented Aug 19, 2026

Copy link
Copy Markdown
Member

Thanks for this, it is a lot of careful work and most of it holds up well. I checked it by measurement rather than by reading: a semantic diff of both catalogs against the merge base, the repo's own catalog guard, scripts/localization.py verify, an xcstringstool compile, a JSONSerialization probe, a macOS Debug build, and your new tests.

No existing translation was altered or lost. I compared every non-Korean localization key by key against d3830ba2d: zero value changes, zero losses, zero state changes in either catalog. The 20k deletions in the diff are the diff algorithm reacting to a ko block inserted at the head of every entry, not a rewrite. ko is in correct sorted position in all 3,827 entries, localization.py verify passes, and ko reaches knownRegions in both generated projects.

I have pushed two commits straight to this branch fixing the three blocking items below, so you do not have to redo them. Pull before you continue. Everything else below is for you to judge.

Blocking (fixed on the branch)

1. An em dash was introduced in Korean. Key %1$@ on %2$@ had %1$@ — 연결: %2$@. This is the VoiceOver announcement built at ConnectionStageLabelFormatter.swift:47, so the string is <step label> on <connection name>. Two problems: the em dash is banned repo-wide by CLAUDE.md, and 연결: adds a word the source does not have. The other languages just reorder, for example %2$@ 上的 %1$@. Changed to %2$@에서 %1$@.

2. Apple's inflection markup was carried into Korean. Key %lld ^[rows](inflect: true) had %lld ^[행](inflect: true). ^[](inflect:) is grammatical agreement Foundation applies for the source language only, so it does nothing for Korean. I compiled the catalog with xcstringstool and the markup lands verbatim in ko.lproj/Localizable.strings. tr, vi and zh all write the plain noun. Changed to %lld행.

3. RowPayloadTests.jsonRejectsUnsupportedShape asserts an error the code cannot throw. JSONSerialization.jsonObject(with:options: []) rejects a top-level fragment, so parseJSON("42") throws error 3840, RowPayload.swift:48 catches it, and the caller gets .malformedPayload, never .invalidJSONShape. The test fails, and .invalidJSONShape was unreachable, so its new Korean string would never have shipped. Measured on this toolchain:

42          -> throws 3840 (fragment not allowed)
"hello"     -> throws 3840
{"a":"1"}   -> object
{not valid  -> throws

Since IntentDataError is Equatable, #expect(throws: .invalidJSONShape) compares by value and cannot match .malformedPayload, so the case fails by construction. The test encodes the right intent, so I fixed the code rather than the test: parseJSON now passes .fragmentsAllowed, which makes the existing throw .invalidJSONShape guard reachable and gives a clear message instead of a raw Foundation string. Re-probed after the change, 42 and "hello" reach .invalidJSONShape, objects and arrays route as before, and {not valid still gives .malformedPayload, so all four RowPayload cases hold.

I could not execute the iOS suite to show it going green. xcodebuild test on a named simulator dies in a @TaskLocal macro expansion inside a dependency (unknown attribute 'usableFromInlinenonisolated' on $jsonMaximumFieldNameSize), which is a toolchain problem here and unrelated to this PR. That is likely the same wall you hit. The reasoning above is from a measured probe and the source, not from a test run.

Why 1 and 2 got past CI, which is our bug not yours

.gitignore:179 is an unanchored Localization/. It was written for the working copies scripts/localization.py produces at the repo root, but git matches an unanchored pattern at any depth, so it also swallows TableProTests/Localization/. StringCatalogIntegrityTests.swift has therefore never been committed and has never run in CI.

I copied it into a checkout of this branch and ran it: 2 of 6 cases failed, translationsAddNoEmDash() and inflectionMarkupStaysInTheSource(), which are exactly items 1 and 2. After the fixes it is 6 of 6. Across all 20,809 translated units in every language there are now zero violations.

I will anchor that pattern and commit the suite separately, so this class of defect is caught on the next translation PR instead of by hand.

Material, your call

4. The imperative register is mixed. 143 strings end in 합쇼체 -십시오 and 87 in 해요체 -세요, and they mix inside single switch statements. XAICallbackServer.ServerError gives 다시 시도하십시오 for one case and 다시 시도하세요 for two others. License.friendlyDescription renders the same English "try again" both ways depending on the case. Across Apple's shipped Korean the five verbs involved run 1094 to 1 in favour of 합쇼체. Either register reads fine on its own, but two of them in consecutive alerts does not. Worth settling on -하십시오 and converting the rest.

5. Redo is 다시 실행; macOS ships 실행 복귀. Undo is correctly 실행 취소, so the pair is broken. It matters more here than in most apps: 다시 실행 reads as "run it again" next to the app's own 실행 for Execute. NSUndoManager.redoMenuItemTitle returns 실행 복귀, and the same Edit menu item uses it.

6. catalogsAreComplete holds Korean to a stricter standard than any shipped language. It fails if any key lacks Korean, while vi, zh-Hans, zh-Hant and tr have hundreds of gaps and nothing gates them. As written, every future PR that adds an English string has to add Korean too or the suite goes red. Consider asserting against a snapshot of the keys this PR covers, or keeping only the structural checks.

Minor

  • Select All is 모두 선택; macOS ships 전체 선택 (28 occurrences across the system, 모두 선택 zero).
  • Details is 세부 정보 and Discard is 버리기; macOS ships 세부사항 and 폐기.
  • Six confirmation titles use 할까요? while 60 identically shaped ones use 하시겠습니까?, including the plugin trust and stop export alerts.
  • %d로 in Process exited with code %d and one other should be %d(으)로, since the correct particle depends on the final digit. You wrote the both-form particle correctly in 25 other places.
  • QueryLiveActivityWidget.swift:123 swaps Apple's automatic inflection for a hard-coded English plural branch, so at exactly one row a vi or zh device shows English "1 row".
  • The CHANGELOG line reads as though iPhone and iPad gained a language picker. They follow the system language; only macOS has the picker.
  • 67 of the new iOS keys are Korean-only even though the macOS catalog already carries vi, zh-Hans and zh-Hant for the same key. Copying those across is cheap.

One consequence worth naming

Extracting the previously hardcoded strings is a real improvement, and it adds 58 macOS keys and 202 iOS keys. The side effect is that the other languages now cover less of the catalog: vi, zh-Hans and zh-Hant go from 100% to 98% on macOS and from 100% to 64% on iOS. That is not a reason to hold the PR, but it is worth knowing before the next release.

Also verified clean

App Shortcut phrases keep ${applicationName} and ${connection}. No String(localized:) was introduced with string interpolation. The README language links are symmetric across all four files. The shared package strings resolve against the app catalog and are present in both. The macOS build passes and your 14 new tests pass.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit 51e10b4 into TableProApp:main Aug 19, 2026
5 of 6 checks passed
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.

feat(i18n): Korean localization support

2 participants