PICARD-2870: Get rid of monkeypatched gettext-related builtins

[zas]

Summary

• This is a…
  • Bug fix
  • Feature addition
  • Refactoring
  • Minor / simple change (like a typo)
  • Other
• Describe this change in 1-2 sentences:

Problem

Those gettext-related methods defined as builtins (monkeypatching) leads to a certain number of issues.
For example, static typing checkers like mypy have issues with those (see python/mypy#8727).

We also have exceptions in various tools configurations (pylint for example) for those.

• JIRA ticket (optional): PICARD-2870

Solution

Import those methods from picard.i18n

It will break plugins, but since 3.0 will break plugins anyway... time to do it.

Action

Additional actions required:

• Update Picard documentation (please include a reference to this PR)
• Other (please specify below)

[zas]

_ is missing from generated ui files, not sure about how to handle it

[rdswift]

_ is missing from generated ui files, not sure about how to handle it

Not the greatest solution, but could you perhaps include the necessary include in the comment text added when the ui is built using the setup.py script?

picard/setup.py

Lines 380 to 394 in 0858846

	def compile_ui(uifile, pyfile):
	log.info("compiling %s -> %s", uifile, pyfile)
	tmp = StringIO()
	uic.compileUi(uifile, tmp)
	source = tmp.getvalue()
	rc = re.compile(r'\n\n#.*?(?=\n\n)', re.MULTILINE | re.DOTALL)
	comment = ("\n\n# Automatically generated - don't edit.\n"
	"# Use `python setup.py %s` to update it."
	% _get_option_name(self))
	for r in list(_translate_re):
	source = r.sub(r'_(\1)', source)
	source = rc.sub(comment, source)
	f = open(pyfile, "w")
	f.write(source)
	f.close()

Another "hack" might be to add the necessary include if it is missing when building the application.

Unfortunately, neither of these will help if the ui was generated without setup.py when running locally from source.

[rdswift]

I believe that this is a real improvement. Thanks for all the refactoring changes to implement it. This looks great to me.

[zas]

@phw as discussed, from picard.i18n import gettext as _ is now possible, but I'm not sure about this move, as we use _() everywhere, I don't think it adds anything (but perhaps let use _ for something else, though that will be confusing).

So, may be, keep fc87e6c but drop 7db3d00

Plus I'll have to re-order all imports... for tests to pass

@rdswift what's your opinion on this?

[rdswift]

@rdswift what's your opinion on this?

I don't think it hurts anything by having the extra flexibility, but as you say it could be confusing to use _ for something else. I do think this makes it cleaner though.

[zas]

I don't think it hurts anything by having the extra flexibility, but as you say it could be confusing to use _ for something else. I do think this makes it cleaner though.

It was more about 7db3d00 which spreads from picard.i18n import gettext as _ to replace equivalent from picard.i18n import _.
As said, picard.i18n.gettext is here to stay anyway (though it could conflict with module gettext, as it does in i18n.py, hence import gettext as module_gettext).

[rdswift]

It was more about 7db3d00 which spreads from picard.i18n import gettext as _ to replace equivalent from picard.i18n import _.

Sorry, I misunderstood. I don't really have any issues with it either way.

[phw]

Huge changelist, but looks good.

Let's also update the translations once this got merged.