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

fix regression in ExtensionCore not working when updates are available #8964

Merged
merged 1 commit into from Nov 22, 2019

Conversation

eli-schwartz
Copy link
Contributor

@eli-schwartz eli-schwartz commented Nov 17, 2019

In commit 6ac295a, Settings>Applets>Download gained the ability to display the number of available updates, using an ngettext-translated message. However, it has never worked since ngettext was never imported in the file in question, so it simply raised a traceback.

In the same commit, gettext.install() was taught to declare names="ngettext", but as per the python documentation, the names= argument requires a sequence (of one or more aliases to declare in the global namespace), not a string. So it tried to check if it could install the gettext functions: {'x', 't', 'e', 'g'}, which was wrong.

The fix is to either import ngettext directly, so it can be used as-is, or globally install ngettext by passing it as a one-item list to the gettext.install() function. I've done the latter.

@eli-schwartz
Copy link
Contributor Author

@clefebvre
Copy link
Member

Already done in #8959.

@clefebvre clefebvre closed this Nov 20, 2019
@eli-schwartz
Copy link
Contributor Author

I... don't even understand that other PR. It seems to be re-doing 058e3a1, fixing some gtk deprecation, and switching one incorrect use of gettext.install() for another, none of which has anything to do with the commit message (that claims this is for python 3.8).

@eli-schwartz
Copy link
Contributor Author

@clefebvre

When the ngettext call was initially added in 6ac295a, I think the person who added it tried to use gettext.install(..., names="gettext"), but that's totally incorrect. Please reopen this PR. I can either leave it to continue explicitly importing ngettext from gettext (it works for me on python 3.8) or switch the gettext.install() call to use names=["gettext"] as it is required to be a sequence and not a string (well, a string is a sequence too, but you don't exactly get the right answer when you use set('ngettext') instead of set(['ngettext'])).

I initially used from gettext import ngettext, because I wasn't sure under what circumstances the gettext.install() was guaranteed to have run, BTW. But I'm totally okay with either solution.

@collinss collinss reopened this Nov 22, 2019
In commit 6ac295a,
Settings>Applets>Download gained the ability to display the number of
available updates, using an ngettext-translated message. However, it has
never worked since ngettext was never imported in the file in question,
so it simply raised a traceback.

In the same commit, gettext.install() was taught to declare
names="ngettext", but as per the python documentation, the names=
argument requires a sequence (of one or more aliases to declare in the
global namespace), not a string. So it tried to check if it could
install the gettext functions: {'x', 't', 'e', 'g'}, which was wrong.

The fix is to either import ngettext directly, so it can be used as-is,
or globally install ngettext by passing it as a one-item list to the
gettext.install() function. I've done the latter.
@eli-schwartz
Copy link
Contributor Author

I've switched to using gettext.install() with the correct sequence.

@clefebvre clefebvre merged commit 5174ec9 into linuxmint:master Nov 22, 2019
@eli-schwartz eli-schwartz deleted the gettext branch November 24, 2019 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants