Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change translation function alias #424

Open
kevin-bates opened this issue Feb 22, 2021 · 2 comments
Open

Change translation function alias #424

kevin-bates opened this issue Feb 22, 2021 · 2 comments

Comments

@kevin-bates
Copy link
Member

Currently, the translation function trans.gettext is aliased to _ in transutils.py and imported into modules that use it.

This particular alias value is also commonly used as a placeholder for ignored unpacked values (tuples) and other scenarios (e.g., loop variables) which can produce runtime conflicts when co-existing in the same method with translatable text - as was the case with #422.

We should replace the "custom alias" of _ = trans.gettext with something that is both more clear and less intrusive. I propose we update the alias to: _i18n = trans.gettext since i18n is an extremely common abbreviation for text localization (l10n is used as well, but less common). This way, translatable messages can be easily identified and co-exist in methods with the more common use of _.

This would result in the problematic method in #421 to look more like the following...

    def launch_browser(self):
        try:
            browser = webbrowser.get(self.browser or None)
        except webbrowser.Error as e:
            self.log.warning(_i18n('No web browser found: %s.') % e)
            browser = None

        if not browser:
            return

        assembled_url, _ = self._prepare_browser_open()

        b = lambda: browser.open(assembled_url, new=self.webbrowser_open_new)
        threading.Thread(target=b).start()
@jeffyjefflabs
Copy link

Came here to report the same bug in launch_browser() (which I see is already fixed). However, _ is also the recommended alias for gettext in the Python documentation, which honestly seems like a very curious suggestion given its other common usage for throwaway variables. See also this StackOverflow question that warns against this exact scenario.

I agree with you, though; changing the i18n alias to something more explicit would prevent this from happening again.

@kevin-bates
Copy link
Member Author

Hi @jeffyjefflabs - thank you for the references. That is helpful information, although I think I'd still like to see the alias updated as well.

If we don't have a PR in a week or so, I'd be happy to provide one when I could get around to it at that time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants