tisfeng / Easydict

一个简洁优雅的词典翻译 macOS App。开箱即用,支持离线 OCR 识别,支持有道词典,🍎 苹果系统词典,🍎 苹果系统翻译,OpenAI,Gemini,DeepL,Google,Bing,腾讯,百度,阿里,小牛,彩云和火山翻译。A concise and elegant Dictionary and Translator macOS App for looking up words and translating text.
GNU General Public License v3.0
6.68k stars 338 forks source link

add a custom OpenAI service #433

Closed phlpsong closed 5 months ago

phlpsong commented 5 months ago

close https://github.com/tisfeng/Easydict/issues/387

gitguardian[bot] commented 5 months ago

⚠️ GitGuardian has uncovered 4 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | | | -------------- | ------------------ | ------------------------------ | ---------------- | --------------- | -------------------- | | [-](https://dashboard.gitguardian.com/incidents/secrets) | | Generic High Entropy Secret | dbb0db05a5029e9397b63af454aa197dd6419f6a | Easydict/Feature/Service/OneAPI/OneAPIService.swift | [View secret](https://github.com/tisfeng/Easydict/commit/dbb0db05a5029e9397b63af454aa197dd6419f6a#diff-44fbdafb6b36f4ea3a54ade6f4d9a14861481609d6cd4ca417dc99c50ef61b38L24) | | [-](https://dashboard.gitguardian.com/incidents/secrets) | | Generic High Entropy Secret | d9a0f26b70d58617491f278a8fcc295ad10fe007 | Easydict/Feature/Service/OneAPI/OneAPIService.swift | [View secret](https://github.com/tisfeng/Easydict/commit/d9a0f26b70d58617491f278a8fcc295ad10fe007#diff-44fbdafb6b36f4ea3a54ade6f4d9a14861481609d6cd4ca417dc99c50ef61b38L21) | | [-](https://dashboard.gitguardian.com/incidents/secrets) | | Generic High Entropy Secret | dbb0db05a5029e9397b63af454aa197dd6419f6a | Easydict/Feature/Service/OneAPI/OneAPIService.swift | [View secret](https://github.com/tisfeng/Easydict/commit/dbb0db05a5029e9397b63af454aa197dd6419f6a#diff-44fbdafb6b36f4ea3a54ade6f4d9a14861481609d6cd4ca417dc99c50ef61b38R21) | | [-](https://dashboard.gitguardian.com/incidents/secrets) | | Generic High Entropy Secret | e763cc5f3672eaa113dee35e629a69fcf2e25525 | Easydict/Feature/Service/CustomOpenAI/CustomOpenAIService.swift | [View secret](https://github.com/tisfeng/Easydict/commit/e763cc5f3672eaa113dee35e629a69fcf2e25525#diff-55093576f9d99689f6a6222e6ec16fd60d9bd51b92dc6d91fca8d530d272da8bR24) |
🛠 Guidelines to remediate hardcoded secrets
1. Understand the implications of revoking this secret by investigating where it is used in your code. 2. Replace and store your secrets safely. [Learn here](https://blog.gitguardian.com/secrets-api-management?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment) the best practices. 3. Revoke and [rotate these secrets](https://docs.gitguardian.com/secrets-detection/secrets-detection-engine/detectors/generics/generic_high_entropy_secret#revoke-the-secret?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment). 4. If possible, [rewrite git history](https://blog.gitguardian.com/rewriting-git-history-cheatsheet?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment). Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data. To avoid such incidents in the future consider - following these [best practices](https://blog.gitguardian.com/secrets-api-management/?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment) for managing and storing secrets including API keys and other credentials - install [secret detection on pre-commit](https://docs.gitguardian.com/ggshield-docs/integrations/git-hooks/pre-commit?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment) to catch secret before it leaves your machine and ease remediation.

🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

phlpsong commented 5 months ago

截屏2024-03-04 20 39 03 After updating the localization, it seems that the switch is being relocated.

It's strange which en string is longer than ch but its behavior is fine. I will add a minWidth constraint to fix this.

phlpsong commented 5 months ago

This icon looks bad in dark mode.

image

I have made a simple one to replace it, let me know what you think.

截屏2024-03-05 19 26 54
tisfeng commented 5 months ago

Looks bad in English mode.

If it's not appropriate to increase the width here, we could shorten the title spelling, Custom OpenAl Translate --> Custom OpenAl, which I think users should be able to understand.

image
phlpsong commented 5 months ago

Looks bad in English mode.

If it's not appropriate to increase the width here, we could shorten the title spelling, Custom OpenAl Translate --> Custom OpenAl, which I think users should be able to understand.

image

I think it's already fixed in prev commits. Could you build using the latest commit and take a look?

tisfeng commented 5 months ago

ok, it looks good now.

image
tisfeng commented 5 months ago

It's working well enough so far 😌

For custom OpenAI services, how about we show the model currently being used to the right of the header?

image
phlpsong commented 5 months ago

It's working well enough so far 😌

For custom OpenAI services, how about we show the model currently being used to the right of the header?

image

Good idea! Does OpenAI service need to apply this?

tisfeng commented 5 months ago

I'm not sure how the display effect of OpenAI will be, because the names of the two preset models are a bit long, but we can display it first and see the effect.

image
phlpsong commented 5 months ago

I'm not sure how the display effect of OpenAI will be, because the names of the two preset models are a bit long, but we can display it first and see the effect.

image
Screenshot 2024-03-06 at 19 28 42

Custom OpenAI Translate is too long in the mini & fix window which has the overlap issue, but it works well in Chinese.

After increasing the minimumWindowSize, looks good now.

Screenshot 2024-03-06 at 21 52 40
tisfeng commented 5 months ago

We can change Model to Models, users can manually enter multiple models separated by ,, we internally save them in an array, the first one will be used as the selected model by default, and users can conveniently switch between models in the query window.

This is just my initial idea, there may be some things that are not well considered and need to be improved.

image

This is a chat service https://chat.izual.me/chat I built with lobe-chat.

image image
phlpsong commented 5 months ago

I think using the comma to separate models is a bit complicated for users. I will check this later if have any other good ways to handle it.

tisfeng commented 5 months ago

In my opinion, letting the user manually input the custom model is already a relatively simple and general enough solution.

I know some services support API querying model lists, but the custom OpenAI service that we designed is very freestyle and shouldn't assume that it will support API querying model lists.

On the other hand, even if it did support API querying model lists, it's not necessarily suitable to directly use it in Easydict to display the NSMenu, because the number of models that some services support is very large, for example OpenAI or Together AI, they return more than a dozen models., and not every model is suitable for translation.

If you have a better idea, let me know.

tisfeng commented 5 months ago

I just improved the model tag UI.

image image
phlpsong commented 5 months ago

In my opinion, letting the user manually input the custom model is already a relatively simple and general enough solution.

I know some services support API querying model lists, but the custom OpenAI service that we designed is very freestyle and shouldn't assume that it will support API querying model lists.

On the other hand, even if it did support API querying model lists, it's not necessarily suitable to directly use it in Easydict to display the NSMenu, because the number of models that some services support is very large, for example OpenAI or Together AI, they return more than a dozen models., and not every model is suitable for translation.

If you have a better idea, let me know.

Yes, I agree with you, there's no better way to do this.

I have implemented this logic in the latest commit, you could kindly take a look and let me know if you have any comments.

phlpsong commented 5 months ago

For models quick switch function in the query window, I could make this enhancement in a separate PR later. @tisfeng

tisfeng commented 5 months ago

For models quick switch function in the query window, I could make this enhancement in a separate PR later. @tisfeng

Ok, this PR has enough content, let's put the rest of the features in another PR.

tisfeng commented 5 months ago

I just removed the default API key in custom OpenAI service e763cc5f, I plan to add a separate built-in AI translation service later.

phlpsong commented 5 months ago

I just removed the default API key in custom OpenAI service e763cc5, I plan to add a separate built-in AI translation service later.

ok, I will take a look at the name update issue this day later.

tisfeng commented 5 months ago

Every time the settings service page is entered, serviceConfigChanged() is triggered multiple times, posting update service notifications, which can cause the loss of query content.

https://github.com/tisfeng/Easydict/assets/25194972/2219afb0-c07b-4114-afca-5db4dacb096c

phlpsong commented 5 months ago

Every time the settings service page is entered, serviceConfigChanged() is triggered multiple times, posting update service notifications, which can cause the loss of query content.

iShot_2024-03-10_13.49.19.mp4

Yes, I found this issue too, checking it now.

phlpsong commented 5 months ago

The notification trigger issue still exists, need more time than expected to investigate it.

tisfeng commented 5 months ago

ok, you can check this problem carefully.

tisfeng commented 5 months ago

Ok, it looks like there is no problem so far.

This PR code has a lot of code and has been modified a lot, so it is not easy to review.

Next time, we should cut it into smaller PRs to make it easier for everyone to review.

phlpsong commented 5 months ago

Ok, it looks like there is no problem so far.

This PR code has a lot of code and has been modified a lot, so it is not easy to review.

Next time, we should cut it into smaller PRs to make it easier for everyone to review.

Agree, it's a huge work both for author and reviewer.

tisfeng commented 5 months ago

Please review this PR.

tisfeng commented 5 months ago

Great job!

tisfeng commented 5 months ago

Next, I hope to be able to click on the model button to quickly switch models https://github.com/tisfeng/Easydict/issues/456

phlpsong commented 5 months ago

sure, I will do it later.