fix: resouce leak in translate_with_setter by deleting the offending code

[Computerdores]

Aims to fix #750 by removing the offending methods along with the remains of the live retranslation and applying necessary refactors along the way.

If this PR is merged using translations would look like this in the future:

label1 = QLabel(Translations["example.no_params"])
label2 = QLabel(Translations["example.one_param"].format(param="This example has a formatting parameter"))

Note: I am kind of busy with other things at the moment so progress will be slow and halting. Also no worries if this takes a while to merge once it is done, since I just started working on this with communicating on that front first, my bad!

[CyanVoxel]

Thank you for working on this so far! A quick note, I'm not sure if the changes in #788 will allow the original bug to be reproduced in the same was as in #733. If not then I may hotwire the tag search panel to no longer recycle its widgets in order to test the original issue

[CyanVoxel]

Actually I see that part of your solution is to create wrapper classes for a lot of the QWidgets. At that point is it worth trying to save the translations updating without a restart? Besides the issues with this chain of bugs, there's already some related Qt issues involving trying to update widgets that have been garbage collected that prevented me from even being able to leverage the benefits of the setter translators when working on #803 (sorry for not opening a related issue for it), and instead I've just had to prompt the user to restart until or even if we can prevent the need for it.

While not having to restart to change the on-screen language is would be nice, I feel like it may just not be worth the time and work on our side for the benefit of saving people a likely one-time restart to apply these changes to the program. Especially when it's not all too common for other applications to update the language without requiring restarting either the program or OS. I'd like to hear more of your thoughts on it, though.

[Computerdores]

Actually I see that part of your solution is to create wrapper classes for a lot of the QWidgets. At that point is it worth trying to save the translations updating without a restart? Besides the issues with this chain of bugs, there's already some related Qt issues involving trying to update widgets that have been garbage collected that prevented me from even being able to leverage the benefits of the setter translators when working on #803 (sorry for not opening a related issue for it), and instead I've just had to prompt the user to restart until or even if we can prevent the need for it.

The wrapper classes aren't strictly necessary for this fix, however I think that using them will ultimately lead to both a simpler design on the translations infrastructure side and for nicer usage, because the translation keys can then be used just like the hard coded text would usually be used.

While not having to restart to change the on-screen language is would be nice, I feel like it may just not be worth the time and work on our side for the benefit of saving people a likely one-time restart to apply these changes to the program. Especially when it's not all too common for other applications to update the language without requiring restarting either the program or OS. I'd like to hear more of your thoughts on it, though.

Honestly my reasons for wanting the live retranslation are mostly that there shouldn't be any technical problem with doing so and that it would be reeeeeally clean. Not the best of reasons all in all, but as long as it doesn't turn out to be too much of a hassle they are good enough for me to waste my own time on.

[CyanVoxel]

The wrapper classes aren't strictly necessary for this fix, however I think that using them will ultimately lead to both a simpler design on the translations infrastructure side and for nicer usage, because the translation keys can then be used just like the hard coded text would usually be used.

I'd really like to avoid wrapper classes if possible since they add a lot of confusion to the codebase. Case and point there's already a wrapper class for the QPushButton floating around called QPushButtonWrapper that sometimes gets used, and sometimes doesn't. It's also been a nightmare for trying to properly type hint methods and variables. I've been slowly working at refactoring out the need for that wrapper to finally end that confusion. Having wrapper classes like this also introduces the precedent that all translatable QWidgets will presumably need wrapper classes to translate for in the future, adding a maintainability burden for the rest of the UI codebase.

Honestly my reasons for wanting the live retranslation are mostly that there shouldn't be any technical problem with doing so and that it would be reeeeeally clean. Not the best of reasons all in all, but as long as it doesn't turn out to be too much of a hassle they are good enough for me to waste my own time on.

I agree that it would be very nice to leverage Qt in a way that lets us re-translate widgets on the fly, which is why I was excited about your initial implementation. But even besides the issues currently being faced, there can always be UI edge cases that we don't account for or that this system corners us in. For example I'm not sure how well this would play with a situation where we had a layout such as a flowlayout that uses the initial text size to calculate the width of the widgets before performing the layout operation. My assumption is that any text that gets updated in such a layout would not automatically prompt the layout operation again, and we'd instead need to add additional manual code to anticipate the unlikely event that a re-translation would occur.

[Computerdores]

The wrapper classes aren't strictly necessary for this fix, however I think that using them will ultimately lead to both a simpler design on the translations infrastructure side and for nicer usage, because the translation keys can then be used just like the hard coded text would usually be used.

I'd really like to avoid wrapper classes if possible since they add a lot of confusion to the codebase. Case and point there's already a wrapper class for the QPushButton floating around called QPushButtonWrapper that sometimes gets used, and sometimes doesn't. It's also been a nightmare for trying to properly type hint methods and variables. I've been slowly working at refactoring out the need for that wrapper to finally end that confusion. Having wrapper classes like this also introduces the precedent that all translatable QWidgets will presumably need wrapper classes to translate for in the future, adding a maintainability burden for the rest of the UI codebase.

👍

Honestly my reasons for wanting the live retranslation are mostly that there shouldn't be any technical problem with doing so and that it would be reeeeeally clean. Not the best of reasons all in all, but as long as it doesn't turn out to be too much of a hassle they are good enough for me to waste my own time on.

I agree that it would be very nice to leverage Qt in a way that lets us re-translate widgets on the fly, which is why I was excited about your initial implementation. But even besides the issues currently being faced, there can always be UI edge cases that we don't account for or that this system corners us in. For example I'm not sure how well this would play with a situation where we had a layout such as a flowlayout that uses the initial text size to calculate the width of the widgets before performing the layout operation. My assumption is that any text that gets updated in such a layout would not automatically prompt the layout operation again, and we'd instead need to add additional manual code to anticipate the unlikely event that a re-translation would occur.

Yeah that is fair enough. In that case I think I will strip out the remains of the live retranslation and also remove the translate_qobject and translate_with_setter methods as those exist purely for the live retranslation. Having a translated QLabel would then look like this in the future:

label1 = QLabel(Translations["example.no_params"])
label2 = QLabel(Translations.formatted("example.one_param", param="This example has a formatting parameter"))

[Computerdores]

This is now ready for review.