dialect-app / dialect

A translation app for GNOME.
https://dialectapp.org/
GNU General Public License v3.0
597 stars 69 forks source link

Initial libsoup async implementation #236

Closed rafaelmardojai closed 2 years ago

rafaelmardojai commented 2 years ago

Changes to use libsoup async functions. Now most Translator methods should just process data.

TODO

Notes and open questions

~With this implementation all the JSON processing code is run in the main thread, test if this doesn't affect the app fluidity. If so, maybe we can rewrite Session.multiple() and create other helper functions to run the processing callbacks threaded.~

~We need to figure out what to do with the TTS code. We don't have time to re-implement gTTS. Should we just leave it as is?~

~It's fine to make requests from Translator.__init__()? Or should we figure out a way to make them from DialectWindow.load_translator() and make Translator() only provide the needed urls and processing functions?~

~It's Session.multiple() fine or should we make it run one task after another?~

mufeedali commented 2 years ago

With this implementation all the JSON processing code is run in the main thread, test if this doesn't affect the app fluidity. If so, maybe we can rewrite Session.multiple() and create other helper functions to run the processing callbacks threaded.

Doesn't seem to cause any slow downs so far for me.

We need to figure out what to do with the TTS code. We don't have time to re-implement gTTS. Should we just leave it as is?

Yeah, probably better to just leave it as is. Proxy users get broken TTS (lol) but not much we can do there. gTTS uses requests IIRC, so maybe it'll just work?

It's fine to make requests from Translator.init()? Or should we figure out a way to make them from DialectWindow.load_translator() and make Translator() only provide the needed urls and processing functions?

As long as we're using the Session helper class and every request is from the same thread, I don't think it really matters much.

It's Session.multiple() fine or should we make it run one task after another?

Should be fine.

mufeedali commented 2 years ago

Rewrite the history code that uses trans_queue

Seems like it works just fine with a minor change.

mufeedali commented 2 years ago

So, there are some issues...

There are others but I'll have to make sure, so I'll post them later.

rafaelmardojai commented 2 years ago

The settings UI doesnt update appropriately when instance has been reset (i.e. it doesn't hide the api key field).

Fixed this and made me remove an unnecessary request when validating.

mufeedali commented 2 years ago

That should fix all the weird re-translation issues. Hopefully, for good.

Overall, this is very much working out fine for me. I think it might be the right direction to go.