feat: restrict --lang to zh/en/ja with legacy read fallback#1177
Open
luozhixiong01 wants to merge 5 commits into
Open
feat: restrict --lang to zh/en/ja with legacy read fallback#1177luozhixiong01 wants to merge 5 commits into
luozhixiong01 wants to merge 5 commits into
Conversation
CLI internals only branch on zh/en/ja (TUI bundle, mail signature locale). The other 11 codes were nominally accepted but had no downstream specialization, leaving the contract larger than the capability. Shrink the catalog to match. i18n.Codes() now returns 3 canonical locales; i18n.Parse() rejects ko/fr/de/etc. The 11 LangXxXx constants are removed; only the mail_signature_lang_test fixture referenced one (LangFrFR) and is switched to an i18n.Lang() literal to preserve the test intent.
After the i18n catalog shrink, ko/ko_kr fall into the strict-reject path of cmdutil.ParseLangFlag. Pin that with explicit test cases on all three --lang entry points so future catalog edits can't silently re-expand the supported set.
Old wording 'e.g. zh or zh_cn' implied a larger supported set than the new 3-language catalog. Replace the 'e.g.' framing with the exhaustive list so AI agents and CLI users see the actual contract.
After the 2026-05-28 catalog shrink to zh/en/ja, existing user configs may still hold ko_kr / fr_fr / etc. RuntimeContext.Lang() now coerces any non-empty unrecognized value to LangZhCN instead of returning empty. Storage on disk is untouched — config.json keeps the legacy value verbatim — so users can still see what was previously set via 'config show', but runtime behavior is uniformly zh.
github-actions
Bot
added
domain/mail
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR narrows supported locales to zh, en, and ja; updates CLI help text; coerces unrecognized legacy language values to the Chinese fallback at runtime; and extends tests to reject dropped locales like ChangesLanguage catalog restriction and validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1177 +/- ##
==========================================
+ Coverage 68.71% 68.92% +0.21%
==========================================
Files 627 629 +2
Lines 58451 58766 +315
==========================================
+ Hits 40163 40507 +344
+ Misses 14999 14952 -47
- Partials 3289 3307 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@28d37fe0800c2f8dcc556169978c144fc08fd3b9🧩 Skill updatenpx skills add larksuite/cli#feat/lang-restrict-zh-en-ja -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/i18n/lang_test.go (1)
85-99: ⚡ Quick winLock the full
Codes()contract, not just count + first entry.
len()==3pluscodes[0]=="zh_cn"still passes ifCodes()returns duplicates or silently dropsen_us/ja_jp. Since this catalog drives the user-facing ordering contract, assert the exact slice here.♻️ Suggested test tightening
-import "testing" +import ( + "slices" + "testing" +) ... func TestCodes(t *testing.T) { codes := Codes() - if len(codes) != 3 { - t.Fatalf("len(Codes()) = %d, want 3", len(codes)) - } - if codes[0] != "zh_cn" { - t.Errorf("Codes()[0] = %q, want %q (catalog order)", codes[0], "zh_cn") + want := []string{"zh_cn", "en_us", "ja_jp"} + if !slices.Equal(codes, want) { + t.Fatalf("Codes() = %v, want %v", codes, want) } // Every code must round-trip through Parse to itself (canonical). for _, c := range codes {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/i18n/lang_test.go` around lines 85 - 99, Replace the weak assertions in TestCodes with a strict equality check of the full Codes() slice: instead of checking len(Codes()) and codes[0], assert that Codes() equals the exact expected slice (e.g. []string{"zh_cn","en_us","ja_jp"}) so duplicates or missing entries are detected; use the Codes() result and Parse(c) checks as-is, but remove the separate len/first checks and replace them with a single exact-slice comparison to lock the contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/i18n/lang_test.go`:
- Around line 85-99: Replace the weak assertions in TestCodes with a strict
equality check of the full Codes() slice: instead of checking len(Codes()) and
codes[0], assert that Codes() equals the exact expected slice (e.g.
[]string{"zh_cn","en_us","ja_jp"}) so duplicates or missing entries are
detected; use the Codes() result and Parse(c) checks as-is, but remove the
separate len/first checks and replace them with a single exact-slice comparison
to lock the contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9feefaac-1ee2-4cdc-b432-bdf1c5174d05
📒 Files selected for processing (11)
cmd/config/bind.gocmd/config/bind_test.gocmd/config/config_test.gocmd/config/init.gocmd/profile/add.gocmd/profile/profile_test.gointernal/i18n/lang.gointernal/i18n/lang_test.goshortcuts/common/runner.goshortcuts/common/runner_lang_test.goshortcuts/mail/mail_signature_lang_test.go
Per review feedback: len==3 + codes[0]==zh_cn still passes if Codes() returns duplicates or drops en_us/ja_jp. Assert the full ordered slice to lock the user-facing ordering contract.
Collaborator
Author
|
@coderabbitai Applied the |
|
Tip For best results, initiate chat on the files or code changes.
(🐇 ✨) If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
--langflag (used byconfig init,config bind,profile add) accepted 14 Feishu locales, but CLI internals only ever branch on zh/en/ja (TUI bundle, mail signature locale) — the other 11 were nominally accepted yet had no downstream specialization, leaving the contract wider than the capability. This PR shrinks the accepted set to zh/en/ja (short codes plus canonicalzh_cn/en_us/ja_jp). Invalid values are now strictly rejected with an agent-friendly error, and existing stored configs holding a dropped locale (e.g.ko_kr) are silently coerced to zh on read, so no existing config breaks.Changes
internal/i18ncatalog from 14 to 3 entries; remove the 11 unusedLangXxXxconstants (internal/i18n/lang.go)--langhelp text from(e.g. zh or zh_cn)to(zh, en, or ja)at all three sites (cmd/config/init.go,cmd/config/bind.go,cmd/profile/add.go)LangZhCNinstead of returning empty inRuntimeContext.Lang()(shortcuts/common/runner.go)ko/ko_krrejection contract on all three entry points and flip the read-layer legacy-coerce expectations (cmd/config/*_test.go,cmd/profile/profile_test.go,shortcuts/common/runner_lang_test.go,internal/i18n/lang_test.go)Test Plan
GOMAXPROCS=2)validate.shdeferred to CI — local run hit macOS jetsam OOM killing compile subprocesses (signal: killed);go build/go vetpass locally, affected packages pass individuallyconfig init --lang koexits 2 withinvalid --lang "ko"; valid values: zh_cn, en_us, ja_jpand writes no config;--lang zh_cnexits 0 stored aszh_cn;config showon a config holdingko_krexits 0 with the value preserved verbatimRelated Issues
N/A
Summary by CodeRabbit
Documentation
Bug Fixes
Tests