-
Notifications
You must be signed in to change notification settings - Fork 1.6k
refactor(i18n): Sort i18n keys in translation files to match English ordering #4572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reordered keys in all tranlation json files to maintain consistent ordering that matches the 'en' locale. This improves readability and maintainability of the translation files.
2bcdfe6
to
d094cc3
Compare
There were a bunch of inconsistencies revealed when I ran the linting script, so I fixed them!
Adds a new CI step to verify that all locale files maintain the same key ordering as their corresponding English locale files. This helps ensure consistency across all committed translations and prevents accidental reordering going forward The new `lint-locale-key-ordering.js` script checks both backend (`src/i18n/locales`) and frontend (`webview-ui/src/i18n/locales`) translation files. It identifies missing keys, extra keys, and keys that are out of order compared to the English reference. This linter will run as part of the `code-qa` workflow.
I added a commit that adds a linter to try to maintain the ordering. It revealed that there were actually a bunch of mis-matched strings between english and the other locales, so I fixed those too! |
} | ||
"input": { | ||
"task_prompt": "Què vols que faci Roo?", | ||
"task_placeholder": "Escriu la teva tasca aquí" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These keys live in settings.json now, they must have be left here at some point!
scripts/lint-locale-key-ordering.js
Outdated
} | ||
|
||
console.log( | ||
`\n${area === "core" ? "BACKEND" : "FRONTEND"} - Checking key ordering for ${locales.length} locale(s): ${locales.join(", ")}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vibe fix- just say "core" or "webview", not "BACKEND" / "FRONTEND"
Refactor the `lint-locale-key-ordering.js` script to enhance readability and provide more concise output. - Updated help message for clarity and brevity. - Simplified key extraction logic to directly push keys. - Streamlined issue reporting for missing, extra, and out-of-order keys. - Improved console logging for better user experience. - Removed redundant comments and simplified function descriptions.
Hey @hassoncs, we really appreciate your contribution! Just a quick thought. This is quite a significant change, and I’m a bit concerned it might introduce a lot of conflicts in most PRs that touch translations. Given that we already have a translation mode that really helps with adding new translations and a script to handle missing ones, it might not be needed to go this route. That said, happy to discuss it further if you have other thoughts! |
@daniel-lxs, that's a very good point! Do you think if the lint script could fix the ordering instead of just error-ing out, then users with open PRs could run the script which would solve the conflicts in the short-term? Then the linter will keep things in sync going forward? |
@hassoncs I'm not against adding a script to order the i18n files at all, but I think we should have a pretty good reason to create all the extra work that it requires. |
@daniel-lxs, ah yeah, I see what you are saying! Keeping them ordered is just another thing for contributors to worry about. It does make it a lot easier to see when locales have mismatches– this PR revealed a bunch of strings that the find-missing script seems to have missed. I'll open a PR with just those targeted string changes if that's ok! Thanks for taking the time to explain! I'll keep this branch around if you ever change your mind haha |
Description
Reorder keys in all i18n json files to maintain consistent ordering that matches the 'en' locale. This improves readability and maintainability of the translation files without changing any behavior. I'm happy to close this PR if you think this a bad idea! Just thought I'd propose it :)
Dev notes
I sorted the keys programmatically using Node, not by hand, so there shouldn't be anything missing or out of place!
Test Procedure
No strings have been removed, so the tests should catch any missing strings!
Type of Change
src
or test files.Pre-Submission Checklist
npm run lint
).console.log
) has been removed.npm test
).main
branch.npm run changeset
if this PR includes user-facing changes or dependency updates.Important
Refactors i18n JSON files by sorting keys to match the 'en' locale, improving readability and maintainability without changing behavior.
ca/chat.json
,de/chat.json
,es/chat.json
... and 18 other files.This description was created by
for 2bcdfe6. You can customize this summary. It will automatically update as commits are pushed.