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

Bing translate #165

Closed
wants to merge 5 commits into from
Closed

Bing translate #165

wants to merge 5 commits into from

Conversation

choykarl
Copy link
Collaborator

1、获取重定向的host
2、Microsoft改为Bing

@choykarl
Copy link
Collaborator Author

完善 #161

@tisfeng bing dict那个我之后看下,bing translate就先留着吧,我之后看看dict的东西。

@tisfeng
Copy link
Owner

tisfeng commented Aug 23, 2023

@choykarl 你好,这个 PR 还有什么 问题吗

@choykarl
Copy link
Collaborator Author

@choykarl 你好,这个 PR 还有什么 问题吗

@tisfeng 这个pr没啥问题,你上次说dict那个暂时还没弄,我目前接了点活有点忙,现在是要等dict那个功能然后发新版本吗?

@tisfeng
Copy link
Owner

tisfeng commented Aug 23, 2023

不不,那个 bing dict 不急,可以放后面慢慢再弄,这次只要把目前的翻译这个接口整好就可以了。

@tisfeng
Copy link
Owner

tisfeng commented Aug 23, 2023

@choykarl 目前这个修改你有时间吗,要是你很忙的话,可以放一下,等后面我来整理也行。

@choykarl
Copy link
Collaborator Author

@tisfeng 额,那目前这个接口是还有啥问题么,上次提出的动态获取host和将microsoft改为bing这个两个问题我已经改了下。是还有问题么?

@tisfeng
Copy link
Owner

tisfeng commented Aug 23, 2023

@choykarl 奇怪,你那边看不到评论吗

image

@choykarl
Copy link
Collaborator Author

image
@tisfeng 完全没有啊。。是在这个pr下面评论的么。。
那个确实是少了个return。。
没有保存的磁盘是因为我考虑到有的人会在下次使用的时候打开了代理或者关了代理,如果走本地读取标记的话就会请求失败,比如上次开了代理,我存的标记的是非chinese,但是下次它把代理关了,我读取标记用非chinese的host的话,应该就会一直请求失败。

@tisfeng
Copy link
Owner

tisfeng commented Aug 23, 2023

@choykarl 感觉 GitHub 的 review 评论通知有问题。。

我的想法是,正常请求失败时,再去调用 fetchRequestHost 确定真正的 host,如果请求数据正确,说明 host 是正确的。

@choykarl
Copy link
Collaborator Author

@tisfeng 明白,那我处理一下。

@choykarl
Copy link
Collaborator Author

@tisfeng 我想了下,用bool存标记不太合适,我现在是改用www.bing.com去请求让微软然后获取重定向的host,重定向的结果不一定是只有cn.bing.com应该,其他的地区感觉也可能会有特定的host,但是我目前有的ip也测不出来。
所以我觉得存string,把重定向的host存起来合适点。

@tisfeng
Copy link
Owner

tisfeng commented Aug 24, 2023

@choykarl 你想要用 string 保存 host 也行,这样的话就是理论上更严谨一点,单独针对 Bing 的 host。保存的话可以直接用 NSUserDefaults 写在当前类文件。

但实际上,像中国网络环境这么奇葩的,世界上也难找出第二个,www.bing.com 本是全球通用,只是在中国要接受审查,没办法才特意弄个 cn.bing.com 来重定向。据我所知,似乎没有其他以外国家这么干,我用机场测过大部分国家的 IP,www.bing.com 都是可以用的。

@tisfeng
Copy link
Owner

tisfeng commented Aug 24, 2023

我前面提到,可以在 EZConfiguration 中用个 isChineseIP,就是想简单一点,只考虑是否为中国 IP,相当于顺便借用 Bing 的接口判断一下用户的 IP,方便后续以作他用。当然,这是后话了。

建议还是用 EZConfiguration NSUserDefaults 磁盘保存,可以用个 BOOL isChineseIP

@choykarl
Copy link
Collaborator Author

@tisfeng 代码提交了,麻烦review一下

@tisfeng tisfeng closed this Aug 24, 2023
@tisfeng tisfeng deleted the bing_translate branch August 24, 2023 15:44
@tisfeng tisfeng restored the bing_translate branch August 24, 2023 15:46
@tisfeng
Copy link
Owner

tisfeng commented Aug 24, 2023

@choykarl 代码没问题,我等会就合并。

操作失误了,我重新合并一下。

@tisfeng
Copy link
Owner

tisfeng commented Aug 24, 2023

@choykarl 这个 PR 我在本地合到了 dev 分支,已推送到远程。

才发现你这 PR 分支和一般的有点不一样,难怪我都没有在 Github 上看到自动合并的提示按钮,第一次见到 🥲

image

一般 PR 都是像这样 #156

image

@tisfeng tisfeng deleted the bing_translate branch August 24, 2023 17:37
@tisfeng tisfeng restored the bing_translate branch August 24, 2023 17:39
@tisfeng tisfeng deleted the bing_translate branch August 24, 2023 17:40
@choykarl
Copy link
Collaborator Author

choykarl commented Aug 24, 2023

@tisfeng 我这个这次是直接从本项目开的分支然后提的pr,之前是从fork的项目提的,不知道有没有关系

@tisfeng
Copy link
Owner

tisfeng commented Aug 25, 2023

@choykarl 后面还是建议通过 fork 的项目分支来提 PR,这样便于 PR 管理以及合并。

主项目的分支需要保持简洁,目前只有 dev 和 main,一般的 PR 提交到 dev,我会定期将 dev 合到 main。

@choykarl
Copy link
Collaborator Author

@choykarl 后面还是建议通过 fork 的项目分支来提 PR,这样便于 PR 管理以及合并。

主项目的分支需要保持简洁,目前只有 dev 和 main,一般的 PR 提交到 dev,我会定期将 dev 合到 main。

@tisfeng ok

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.

2 participants