Skip to content

Conversation

@BenBornschein
Copy link
Contributor

Deleted locale files for various services and refactored the code to use provider-specific classes. Added glossary support for DeepL.

Deleted locale files for various services and refactored the code to use provider-specific classes. Added glossary support for DeepL.
@BenBornschein
Copy link
Contributor Author

I'm unable to reproduce the error you're experiencing. Please note that an API key is always required for DeepL, even with the free version. Could you provide the specific steps needed to trigger this bulk error in DeepL? The bulk option shown in the screenshot functions correctly on my end.
CleanShot 2024-10-30 at 11 26 41@2x

@toolstack
Copy link
Owner

Is the deepl glossary support the difference between this PR and the previous one?

@BenBornschein
Copy link
Contributor Author

This PR is identical to the previous one. I created a new PR due to issues with my local Git submodule configuration, which caused some problems with Git.
I was able to reproduce the second error you reported, but the reason was a missing DeepL key.

@toolstack
Copy link
Owner

Ok, I found the issue with DeepL, just because there is an API key doesn't mean that you have a paid plan, in which case it means you still use the free api url.

The old code just always used the free-api.deepl.com url.

Probably easiest solution is to just go back to the old behaviour. Probably best solution would be to add a configuration option for DeepL url toggle.

Refactor DeepLProvider to split free and pro versions. Introduce DeepLProProvider class with dedicated authentication and glossary handling. Update ProviderManager to register the new DeepL Pro provider.
@BenBornschein
Copy link
Contributor Author

The latest commit fixes this problem. I have also tested it with DeepL Free and DeepL Pro.

@toolstack
Copy link
Owner

Looks good so far, will do some additional testing and if nothing comes up will merge later this week.

@toolstack toolstack merged commit ff048e1 into toolstack:main Nov 10, 2024
@toolstack
Copy link
Owner

I'll do a release in the next day or two.

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