EDMarketConnector
EDMarketConnector copied to clipboard
Rename the `_()` translation function
An unfortunate side-effect of forcing _()
into builtins
for translation of strings is that it means the _
for things like "I don't care about this return value" clashes. In general it was a REALLY bad choice given the uses of _
normally.
should_return: bool
new_data: dict[str, Any]
should_return, _ = killswitch.check_killswitch('capi.auth', {})
Here I've changed new_data
in the function call to _
. After this the use of _("string to be translated")
breaks:
Traceback (most recent call last):
File "C:\Program Files\Python311\Lib\tkinter\__init__.py", line 1948, in __call__
return self.func(*args)
^^^^^^^^^^^^^^^^
File "C:\Users\Athan\Documents\Devel\EDMarketConnector\EDMarketConnector.py", line 1128, in capi_request_data
self.status['text'] = _('Fetching data...')
^^^^^^^^^^^^^^^^^^^^^
TypeError: 'dict' object is not callable
Proposal:
Change the translation _()
to _t()
- it's only one extra character.
Issues: minor - PLUGINS.md mentions manually setting this up (does that code even work ?), that needs updating. But as plugins would have to manually set things up in this manner nothing will break by changing this in core code.
Follow-ups:
Identify any places where _
as "throw this away" would be appropriate and update them. There's at least some killswitch related code.
Check over all the developer documentation to be sure it's up to date with the change.
Honestly, I'm looking at just dropping forcing the builtin entirely. It confuses type checking, it makes things look weird, and it's not very clear where it's set or what it's doing.
It'd be easier to just add a dummy function to the l10n _Translation class and accept one more direct import.
I don't think it's too much of a design sacrifice to do something along these lines:
Add this to l10n's _Translations class
def tl(self, *args) -> str:
"""
Dummy loader for the translate function. Used as shorthand.
"""
return self.translate(*args)
and in files where we need translations
from l10n import Translations as Tr
...
def somefunc():
foo = Tr.tl("My String Here")
...
instead of the traditional
def somefunc():
foo = _("My String Here")
Mocked up what would need to be done in https://github.com/HullSeals/EDMarketConnector/tree/enhancement/1812/rename_underscore_translate Tasks:
- [x] Rename all usages of the function in the current code
- [x] Add deprecation note to existing builtin override (6.0 removal)
- [x] Add shorthand translation reference func and update imports
- [ ] Update documentation on how to localize with plugins
Some of this may relate to #2188 and may need revisited if/when it gets merged in.
As per comments in Discord, I'd take this opportunity to move to the standard ways of naming things in Python.
-
Translations
is currently the singleton. This should not be capitalised! - This also means that
Tr
shouldn't be capitalised either.
So:
- Rename the
_Translations
class (note the leading underscore) toTranslations
. - Rename the
Translations
singleton totranslations
. - If doing import shortening tricks, then use/recommend something like
from l10n import translations as tr
, and end up withtr.tl(...)
replacing_(...)
.