Skip to content

Add section to tell if it's free or pro based on apikey#1049

Merged
tisfeng merged 7 commits intotisfeng:devfrom
purifier1990:users/wenyuzhao/section
Jan 25, 2026
Merged

Add section to tell if it's free or pro based on apikey#1049
tisfeng merged 7 commits intotisfeng:devfrom
purifier1990:users/wenyuzhao/section

Conversation

@purifier1990
Copy link
Copy Markdown
Contributor

@purifier1990 purifier1990 commented Jan 10, 2026

Screenshot 2026-01-10 at 18 47 46

Also added i18n strings

Close #932

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello purifier1990, Thank you for your first PR contribution 🎉 purifier1990

@Jerry23011
Copy link
Copy Markdown
Collaborator

Jerry23011 commented Jan 12, 2026

Thank you for the PR, and I love the idea to make it clearer for users.

I went over the issue mentioned above. I think 沉浸式翻译 has a great naming: 免费/高级 | Free/Advanced

Since we're an open-source software, perhaps it's better not to tier users.

I have several ideas here:

  • 内置/高级 | Built-in/Advanced
  • 免费/高级 | Free/Advanced (沉浸式翻译)
  • 开箱即用/需要密钥 | Built-in/Requires Setup (could be a bit long, but my personal favourite)

Nevertheless, I think Free/Pro already works well, feel free to consider the above.

@purifier1990
Copy link
Copy Markdown
Contributor Author

Thank you for the PR, and I love the idea to make it clearer for users.

I went over the issue mentioned above. I think 沉浸式翻译 has a great naming: 免费/高级 | Free/Advanced

Since we're an open-source software, perhaps it's better not to tier users.

I have several ideas here:

  • 内置/高级 | Built-in/Advanced
  • 免费/高级 | Free/Advanced (沉浸式翻译)
  • 开箱即用/需要密钥 | Built-in/Requires Setup (could be a bit long, but my personal favourite)

Nevertheless, I think Free/Pro already works well, feel free to consider the above.

To me, I will prefer following trending tools' wording, the Builtin/Requres setup seems not have the same patterns, I will vote for Free/Advanced, if Free/Pro is not a good candidate, welcome to vote for others

Jerry23011
Jerry23011 previously approved these changes Jan 17, 2026
Copy link
Copy Markdown
Collaborator

@Jerry23011 Jerry23011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @purifier1990 Feel free to push the Free/Advanced changes. Thank you for your PR!

Copy link
Copy Markdown
Collaborator

@Jerry23011 Jerry23011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After taking a closer look, users can use DeepL and several others without an API key, so I'm thinking how we can let users know about that? Or do we also want to move them to Free?

@Jerry23011 Jerry23011 dismissed their stale review January 17, 2026 21:19

DeepL and several other services can be used for free without and API key

This commit refactors the service section index mapping code in ServiceTab.swift to improve readability and efficiency.

The changes:
- Simplified the enumerated loop by adding a `where` clause directly to the iteration
- Reduced nesting by moving the sectionServices.contains check into the loop condition
- Improved code clarity by eliminating the unnecessary if block inside the loop
- Net result: 5 insertions, 7 deletions (more concise and readable code)

This refactoring makes the code more maintainable and easier to understand while preserving the exact same functionality for mapping service indices between sections and the full service array.
@purifier1990
Copy link
Copy Markdown
Contributor Author

After taking a closer look, users can use DeepL and several others without an API key, so I'm thinking how we can let users know about that? Or do we also want to move them to Free?

@Jerry23011 I think I was mainly depended on what the code already has, like builtin enum and the StreamService, I thought this kind of StreamService should be based on some keys, like what AI did for this code, if this looks not what you expect, maybe the type is being abused using too much?

…ect needPrivateAPIKey usage

This commit refactors the API key requirement logic across multiple services to use the direct `needPrivateAPIKey()` method, eliminating complex conditional checks.

Key changes:
- Removed the `requiresAPIKey(_:)` helper method from ServiceTab (29 lines deleted)
- Updated all service implementations to override `needPrivateAPIKey()` directly:
  - Built-in services (Apple, Bing, DeepL, Google, etc.) return `false`
  - OpenAI StreamService returns dynamic value based on `requireAPIKey`
  - Base QueryService returns `true` as default
- Simplified service filtering in ServiceTab to use `$0.needPrivateAPIKey()` directly

This refactoring:
- Reduces code duplication and complexity
- Makes API key requirements explicit in each service
- Improves maintainability by centralizing the logic
- Eliminates complex runtime type checking and string matching
@tisfeng
Copy link
Copy Markdown
Owner

tisfeng commented Jan 18, 2026

Thanks for your PR, I generally agree with the Free/Pro classification, and I'll make some optimizations and simplifications to it later.

…nto unified apiKeyRequirement

This commit consolidates the API key requirement system by introducing a unified `apiKeyRequirement()` method that replaces both `requireAPIKey` and `needPrivateAPIKey`.

Key improvements:
- Introduces `ServiceAPIKeyRequirement` enum with three cases: `.none`, `.builtIn`, `.userProvided`
- Replaces 15+ `needPrivateAPIKey()` implementations with `apiKeyRequirement()` returning appropriate enum values
- Removes `requireAPIKey` property from StreamService (it was always `true`)
- Updates all service implementations to use the new enum system:
  - Free services (Apple, Bing, DeepL, Google, etc.) return `.none`
  - Built-in services (AITool, BuiltInAI) return `.builtIn`
  - User API services (OpenAI, DeepL Pro, etc.) return `.userProvided`
- Updates ServiceTab filtering logic to use `apiKeyRequirement().needsUserProvidedKey`
- Updates validation logic to check `apiKeyRequirement().requiresKeyForRequest`

This refactoring provides a more expressive and type-safe way to categorize API key requirements across all services.
@tisfeng
Copy link
Copy Markdown
Owner

tisfeng commented Jan 18, 2026

Later I will continue to simplify the code by removing things like API keys for built-in traditional translation services(like tencent api key) and code such as hasFreeQuotaLeft, which I think are no longer needed and just add complexity to the code.

@tisfeng
Copy link
Copy Markdown
Owner

tisfeng commented Jan 21, 2026

please review

@tisfeng tisfeng requested a review from Jerry23011 January 21, 2026 14:17
Copy link
Copy Markdown
Collaborator

@Jerry23011 Jerry23011 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you @purifier1990 and @tisfeng

@tisfeng tisfeng enabled auto-merge (squash) January 25, 2026 02:50
@tisfeng tisfeng merged commit 15ee075 into tisfeng:dev Jan 25, 2026
3 checks passed
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.

🚀 功能建议:服务列表添加分组功能,区分内置和 API KEY 分组

3 participants