Skip to content
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

Update languages.js and index.js to fix Punjabi translations, add support for both 'zh' and 'zh-cn' Simplified Chinese and both 'he' and 'iw' Hebrew translations, and allow for translation requests from any language URL #5

Closed
wants to merge 6 commits into from

Conversation

rawr51919
Copy link

@rawr51919 rawr51919 commented Mar 4, 2019

Fixes applied to languages.js to allow for proper Punjabi translations and support for the usage of both zh and zh-cn language code in Simplified Chinese translations and both he and iw language codes in Hebrew translations, as well as to index.js to fix translation requests originating from China and allow for other language URLs to be used as well (this will fix issues #4, matheuss#48, matheuss#59, matheuss#60, matheuss#62, matheuss#68, and matheuss/google-translate-token#10, tying into the PR @ vitalets/google-translate-token#2, which has a better method of URL switching than vitalets/google-translate-token#1 did).

Fix applied to languages.js to allow for proper Punjabi translations.
@codecov-io
Copy link

Codecov Report

Merging #5 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master      #5   +/-   ##
======================================
  Coverage    92.3%   92.3%           
======================================
  Files           2       2           
  Lines          65      65           
======================================
  Hits           60      60           
  Misses          5       5
Impacted Files Coverage Δ
languages.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3cb689...5ce192e. Read the comment docs.

Updates the index.js to work in the automatic URL switching from a PR to the original repo.
Updates index.js due to an error on my part when adapting the index.js for the URL autoswitch code.
Added the possibility of using either 'he' or 'iw' for Hebrew language translation (fixes issue matheuss#59)
@rawr51919 rawr51919 changed the title Update languages.js and index.js to fix Punjabi translations and translation requests originating from China Update languages.js and index.js to fix Punjabi translations, add support for both 'he' and 'iw' Hebrew translations, and translation requests originating from China Mar 4, 2019
@rawr51919 rawr51919 changed the title Update languages.js and index.js to fix Punjabi translations, add support for both 'he' and 'iw' Hebrew translations, and translation requests originating from China Update languages.js and index.js to fix Punjabi translations, add support for both 'he' and 'iw' Hebrew translations, and fix translation requests originating from China Mar 4, 2019
Add support for using both the 'zh' and 'zh-cn' language codes to translate into Simplified Chinese.
@rawr51919 rawr51919 changed the title Update languages.js and index.js to fix Punjabi translations, add support for both 'he' and 'iw' Hebrew translations, and fix translation requests originating from China Update languages.js and index.js to fix Punjabi translations, add support for both 'zh' and 'zh-cn' Simplified Chinese and both 'he' and 'iw' Hebrew translations, and fix translation requests originating from China Mar 5, 2019
@vitalets
Copy link
Owner

vitalets commented Mar 6, 2019

@coltongit Thanks for the PR!
Maybe add option tld be able set https://translate.google.com or https://translate.google.cn, or whatever?

E.g.:

translate('Ik spreek Engels', {to: 'en', tld: 'cn'}).then(res => ...

For me it seems more flexible:

  • any new translate.google.{tld} can be supported without code change
  • clients having translate.google.com available will not perform redundant request to https://translate.google.cn

@rawr51919
Copy link
Author

rawr51919 commented Mar 6, 2019

@coltongit Thanks for the PR!
Maybe add option tld be able set https://translate.google.com or https://translate.google.cn, or whatever?

E.g.:

translate('Ik spreek Engels', {to: 'en', tld: 'cn'}).then(res => ...

For me it seems more flexible:

  • any new translate.google.{tld} can be supported without code change
  • clients having translate.google.com available will not perform redundant request to https://translate.google.cn

There isn't much of a way I could easily implement the tld field in the code, so it might take a while before I can update the PR with the new field (this also nullifies the PR at the token side of things, so it was closed and a new one created with preparations for the better code in place).

@rawr51919 rawr51919 changed the title Update languages.js and index.js to fix Punjabi translations, add support for both 'zh' and 'zh-cn' Simplified Chinese and both 'he' and 'iw' Hebrew translations, and fix translation requests originating from China Update languages.js and index.js to fix Punjabi translations, add support for both 'zh' and 'zh-cn' Simplified Chinese and both 'he' and 'iw' Hebrew translations, and allow for translation requests from any language URL Mar 6, 2019
Updates index.js to prepare for the new `tld` field coming to this API (it'll default to `.com` to prevent any loss of functionality with older code).
@vitalets
Copy link
Owner

vitalets commented Mar 6, 2019

@coltongit could you make a separate PR for language changes (that we can easily merge)
and separate PR for passing tld. Thanks!

@rawr51919
Copy link
Author

@coltongit could you make a separate PR for language changes (that we can easily merge)
and separate PR for passing tld. Thanks!

Sure. This PR's gonna be split in two, so it's best if I close this one, refork, and make 2 new PRs with the changes here in them.

@rawr51919 rawr51919 closed this Mar 6, 2019
@rawr51919
Copy link
Author

@vitalets Done. Check PRs #6 and #7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants