Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds Japanese language support by introducing two new locale JSON files ( Changes
Sequence Diagram(s)(omitted — changes are localization additions and minor routing/i18n updates that do not introduce new multi-component control flow requiring a sequence diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/js/i18n/i18n.ts (1)
62-64: Consider generating regex fromsupportedLanguagesto reduce maintenance burden.The language list is hardcoded in three separate regex patterns (lines 63, 153, 246) in addition to the
supportedLanguagesarray. Adding a new language requires updating four locations, which is error-prone.♻️ Suggested refactor to DRY up language matching
+// Build regex pattern from supportedLanguages array +const langPattern = supportedLanguages.join('|'); + export const getLanguageFromUrl = (): SupportedLanguage => { // ... const langMatch = path.match( - /^\/(en|ar|fr|es|de|zh|zh-TW|vi|tr|id|it|pt|nl|be|da|ko|sv|ru|ja)(?:\/|$)/ + new RegExp(`^\\/(${langPattern})(?:\\/|$)`) );Apply the same pattern to the regexes in
changeLanguage(line 153) andrewriteLinks(line 246).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/i18n/i18n.ts` around lines 62 - 64, Replace the hardcoded language alternation in the regex used to detect language prefixes with a regex built from the existing supportedLanguages array; specifically, generate an escaped alternation string from supportedLanguages and use it to build the regex used for langMatch (the path.match call), and reuse the same generated regex (or a helper function that returns it) in changeLanguage and rewriteLinks so all three places derive their pattern from supportedLanguages instead of duplicated literals.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/locales/ja/tools.json`:
- Around line 463-464: The translation for the JSON key "rightDocument" is
incorrect: it currently reads "正しい文書" (meaning "correct document") but should
indicate the right-side panel; update the value for "rightDocument" (paired with
"leftDocument") to a phrase like "右の文書" or "右側の文書" so it clearly means "right
side document" in the side-by-side PDF comparison UI.
---
Nitpick comments:
In `@src/js/i18n/i18n.ts`:
- Around line 62-64: Replace the hardcoded language alternation in the regex
used to detect language prefixes with a regex built from the existing
supportedLanguages array; specifically, generate an escaped alternation string
from supportedLanguages and use it to build the regex used for langMatch (the
path.match call), and reuse the same generated regex (or a helper function that
returns it) in changeLanguage and rewriteLinks so all three places derive their
pattern from supportedLanguages instead of duplicated literals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b51a632-344d-4c26-92c8-61b6454df428
📒 Files selected for processing (4)
public/locales/ja/common.jsonpublic/locales/ja/tools.jsonsrc/js/i18n/i18n.tsvite.config.ts
|
I have read the CLA Document and I hereby sign the CLA |
Description
Add Japanese translation
Fixes # (issue)
Type of change
Please delete options that are not relevant.
🧪 How Has This Been Tested?
I confirmed it using Docker on my own server.
Checklist:
Expected Results:
All Japanese translations
Actual Results:
There were no items to translate in faq.html.
Checklist:
Summary by CodeRabbit