-
Notifications
You must be signed in to change notification settings - Fork 108
feat: pre-translate, add exclude-language option #982
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #982 +/- ##
============================================
+ Coverage 65.30% 65.30% +0.01%
- Complexity 1671 1679 +8
============================================
Files 244 244
Lines 6860 6881 +21
Branches 1043 1046 +3
============================================
+ Hits 4479 4493 +14
- Misses 1787 1790 +3
- Partials 594 598 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull request overview
Adds support for --exclude-language to crowdin pre-translate, matching the behavior available in other commands and improving ergonomics when most languages should be targeted except a few.
Changes:
- Add
--exclude-language/-eoption topre-translateand validate it can’t be used with an explicit--languagelist (exceptall). - Thread the new option through
PreTranslateSubcommand -> Actions/CliActions -> PreTranslateActionand apply filtering in language selection. - Add/adjust unit tests to cover exclude-language filtering and updated method signatures.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/crowdin/cli/commands/picocli/PreTranslateSubcommand.java | Adds the new CLI option and conflict validation, passes it into the action factory. |
| src/main/java/com/crowdin/cli/commands/actions/PreTranslateAction.java | Validates excluded language IDs and filters them out when “all languages” mode is used. |
| src/main/java/com/crowdin/cli/commands/Actions.java | Updates the preTranslate API to include excludeLanguageIds. |
| src/main/java/com/crowdin/cli/commands/actions/CliActions.java | Wires the new parameter into PreTranslateAction construction. |
| src/main/resources/messages/messages.properties | Adds help text and a dedicated conflict error key for pre-translate. |
| src/test/java/com/crowdin/cli/commands/actions/PreTranslateActionTest.java | Adds coverage for exclude-language filtering and updates existing tests for the new constructor signature. |
| src/test/java/com/crowdin/cli/commands/actions/CliActionsTest.java | Updates test call to match the new preTranslate signature. |
| src/test/java/com/crowdin/cli/commands/picocli/PicocliTestUtils.java | Updates mocks/stubs for the new preTranslate signature. |
| src/test/java/com/crowdin/cli/commands/picocli/PreTranslateSubcommandTest.java | Updates verification for the new preTranslate signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/test/java/com/crowdin/cli/commands/actions/PreTranslateActionTest.java
Show resolved
Hide resolved
|
Hi @bonqus, thanks a lot for the contribution! It's a nice addition to the Please refer to Copilot's comment above. If you don't think it's relevant, just resolve it. |
Summary
exclude-languageoption ofcrowdin pulltocrowdin pre-translate.This is useful for the same reasons that it was added to the pull command.
If 20 languages, except one, need pre-translating. I need to specify 19 languages; instead of excluding one.
This also ensures more consistency in the crowdin cli options.
Testing