jupyter_server icon indicating copy to clipboard operation
jupyter_server copied to clipboard

Change translation function alias

Open kevin-bates opened this issue 4 years ago • 2 comments

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()

kevin-bates avatar Feb 22 '21 16:02 kevin-bates

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.

jeffyjefflabs avatar Feb 22 '21 17:02 jeffyjefflabs

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.

kevin-bates avatar Feb 22 '21 18:02 kevin-bates