fix(dicts): fix youdaotrans and caiyun translation engines#2312
fix(dicts): fix youdaotrans and caiyun translation engines#2312
Conversation
bylsxy
commented
Dec 31, 2025
- youdaotrans: use free demo API, no API key required
- caiyun: require user API token (website redesigned)
- youdao: limit to short text (max: 5 words)
- reorder: youdaotrans before caiyun
- pdf.js: fix Windows compatibility (use curl)
- README: add Node.js 17+ build instructions
There was a problem hiding this comment.
Pull request overview
This PR fixes and updates two translation engines (youdaotrans and caiyun) by migrating them to use free APIs and adding new utility functions for chunked translation.
Key changes:
- Youdaotrans now uses a free demo API without requiring API keys
- Caiyun now requires user API tokens and has been updated for their redesigned API
- Added chunked translation support to handle large texts for Google and Baidu translators
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/dictionaries/youdaotrans/engine.ts | Complete rewrite to use Youdao's free demo API with custom language detection and TTS integration |
| src/components/dictionaries/caiyun/engine.ts | Migrated to new Caiyun API with required authentication and Baidu TTS fallback |
| src/components/dictionaries/youdao/config.ts | Reduced max selection word count from unlimited to 5 words |
| src/components/dictionaries/google/engine.ts | Added chunked translation support for handling large texts |
| src/components/dictionaries/baidu/engine.ts | Added credential requirement check and chunked translation support |
| src/components/MachineTrans/engine.ts | Added translateInChunks utility function and chunkText helper for splitting large texts |
| src/app-config/profiles.ts | Reordered dictionaries to place youdaotrans before caiyun |
| scripts/pdf.js | Updated to use curl for Windows compatibility and added support for local pdfjs-dist |
| README.md | Added Node.js 17+ build instructions with OpenSSL environment variable |
| README-zh.md | Added Chinese version of Node.js 17+ build instructions |
| .gitignore | Added scripts/pdfjs.zip to ignored files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
scripts/pdf.js
Outdated
| shell.cd('./' + cacheDir) | ||
| exec(downloadCmd, 'Error: download failed') | ||
|
|
||
| exec(`tar -xf "${zipPath}" -C "${cacheDir}"`, 'Error: download failed') |
There was a problem hiding this comment.
The error message 'Error: download failed' is misleading for the tar extraction step. If the curl download succeeds but tar extraction fails, this error message would incorrectly suggest the download failed. Consider using a more specific error message like 'Error: extraction failed' for this extraction step.
| exec(`tar -xf "${zipPath}" -C "${cacheDir}"`, 'Error: download failed') | |
| exec(`tar -xf "${zipPath}" -C "${cacheDir}"`, 'Error: extraction failed') |
| langcodes | ||
| ) | ||
| } | ||
| const token = userToken |
There was a problem hiding this comment.
The variable token is assigned from userToken but never modified, making it redundant. The code should either use userToken directly on line 104, or remove the assignment on line 95. This reduces code clarity without providing any benefit.
| tts: | ||
| srcText.length <= 200 | ||
| ? `https://fanyi.baidu.com/gettts?lan=${slCode}&text=${encodeURIComponent( | ||
| srcText | ||
| )}&spd=3&source=web` | ||
| : undefined |
There was a problem hiding this comment.
The language code used for Baidu TTS may not match Baidu's expected format. For Caiyun API, Chinese uses 'zh' but Baidu TTS may expect different codes (like 'zh' for Simplified Chinese and 'cht' for Traditional Chinese). When slCode is 'ja' or 'en', this should work, but ensure Baidu TTS supports the same language codes that Caiyun uses, or add proper mapping.
|
|
||
| const result = response.data | ||
|
|
||
| if (!result || result.errorCode !== '0' || !result.translation) { |
There was a problem hiding this comment.
The error code check compares result.errorCode !== '0' (string), but API error codes can sometimes be returned as numbers. If the API returns numeric 0 instead of string "0", this check will fail and reject a valid response. Consider using loose equality (result.errorCode != '0') or explicitly handle both string and numeric error codes.
| if (!result || result.errorCode !== '0' || !result.translation) { | |
| if ( | |
| !result || | |
| (result.errorCode !== 0 && result.errorCode !== '0') || | |
| !result.translation | |
| ) { |
scripts/pdf.js
Outdated
| shell.cd('./' + cacheDir) | ||
| exec(downloadCmd, 'Error: download failed') | ||
|
|
||
| exec(`tar -xf "${zipPath}" -C "${cacheDir}"`, 'Error: download failed') |
There was a problem hiding this comment.
The tar command is being used to extract a ZIP file, but tar cannot extract ZIP archives on all platforms (especially older versions). This will fail on Windows and some Unix systems. You should use a cross-platform solution like the unzipper npm package or extract-zip, or use shell.js's built-in methods if available.
| if ( | ||
| options.keepLF === 'none' || | ||
| (options.keepLF === 'pdf' && !payload.isPDF) || | ||
| (options.keepLF === 'webpage' && payload.isPDF) |
There was a problem hiding this comment.
The code accesses payload.isPDF but the MachineTranslatePayload interface (lines 8-11 in MachineTrans/engine.ts) doesn't include an isPDF property. This will cause a TypeScript compilation error. The interface should be updated to include isPDF?: boolean, or the payload parameter should be properly typed to include this property.
| if (sl === 'auto') { | ||
| if (isContainJapanese(text)) { | ||
| sl = 'ja' | ||
| } else if (isContainKorean(text)) { | ||
| sl = 'ko' | ||
| } else if (isContainChinese(text)) { | ||
| sl = 'zh-CN' | ||
| } else { | ||
| sl = 'en' | ||
| } | ||
| } |
There was a problem hiding this comment.
The language detection logic doesn't handle the 'zh-TW' language code that exists in the langMap. If the text is Traditional Chinese or if 'zh-TW' is explicitly passed as payload.sl, this code path would set sl to 'zh-CN' (line 88) which is Simplified Chinese, potentially causing incorrect translations. Consider adding support for Traditional Chinese detection or handling 'zh-TW' explicitly.
| id: 'caiyun', | ||
| sl: result.from, | ||
| tl: result.to, | ||
| slInitial: profile.dicts.all.caiyun.options.slInitial, | ||
| searchText: result.origin, | ||
| trans: result.trans | ||
| sl, | ||
| tl, | ||
| slInitial: options.slInitial, |
There was a problem hiding this comment.
The Caiyun API supports language detection (via the detect field on line 109), but the code doesn't use the detected language from the API response. After translation, the code returns the initially guessed sl and tl values instead of using the actual detected language from the API response. This could lead to displaying incorrect source/target language labels to the user.
| type YoudaotransSearchResult = DictSearchResult<YoudaotransResult> | ||
|
|
There was a problem hiding this comment.
The type alias YoudaotransSearchResult is defined but never used in the file. Consider removing it to reduce code clutter, or use it if it was intended for the function return type.
| type YoudaotransSearchResult = DictSearchResult<YoudaotransResult> |
| type CaiyunSearchResult = DictSearchResult<CaiyunResult> | ||
|
|
There was a problem hiding this comment.
The type alias CaiyunSearchResult is defined but never used in the file. Consider removing it to reduce code clutter, or use it if it was intended for the function return type.
| type CaiyunSearchResult = DictSearchResult<CaiyunResult> |
- youdaotrans: use free demo API, no API key required - caiyun: require user API token (website redesigned) - youdao: limit to short text (max: 5 words) - reorder: youdaotrans before caiyun - pdf.js: fix Windows compatibility (use curl) - README: add Node.js 17+ build instructions
|
Fixed TS typing issue and Youdao errorCode handling. |
|
谢谢你的 PR,项目正在重构中,我会在合适的时候合并进去。 |