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

Closes #8956 - cinnamon-settings.py and ExtensionCore.py: now compatibles with Python 3.8 #8959

Closed
wants to merge 2 commits into from

Conversation

claudiux
Copy link
Member

@claudiux claudiux commented Nov 16, 2019

(Python 3.8 is used on Arch with Cinnamon 4.2.4.)
Also replaces self.window.set_wmclass(wm_class, wm_class) by self.window.set_role(wm_class) to avoid the warning message "Gtk.Window.set_wmclass is deprecated" (https://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-set-wmclass).

Closes #8956.

(Python 3.8 is used on Arch.)
Also replaces `self.window.set_wmclass(wm_class, wm_class)` by `self.window.set_role(wm_class)` to avoid the warning message "Gtk.Window.set_wmclass is deprecated".
@claudiux claudiux changed the title cinnamon-settings.py: now compatible with Python 3.8 cinnamon-settings.py and ExtensionCore.py: now compatibles with Python 3.8 Nov 16, 2019
@claudiux claudiux changed the title cinnamon-settings.py and ExtensionCore.py: now compatibles with Python 3.8 Closes #8956 - cinnamon-settings.py and ExtensionCore.py: now compatibles with Python 3.8 Nov 16, 2019
@clefebvre
Copy link
Member

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 1
           

See the complete overview on Codacy

Copy link
Contributor

@eli-schwartz eli-schwartz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If nothing else, these commit messages are extremely confusing. So is the repetition of changes already on master.

@@ -27,7 +27,7 @@
import SettingsWidgets

# i18n
gettext.install("cinnamon", "/usr/share/locale", names="ngettext")
gettext.install("cinnamon", "/usr/share/locale", names="gettext.ngettext")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both "ngettext" and "gettext.ngettext" are incorrect, they are the set sequence {'t', 'x', 'e', 'n', 'g', '.'} and have no syntactic meaning here. Additionally, your later on ngettext is changed to be accessed via gettext.ngettext, so it doesn't need the install() function to install a global namespace object.

@@ -422,7 +422,7 @@ def __init__(self):
# can consider it as a standalone app and give it its own
# group.
wm_class = "cinnamon-settings %s" % arg1
self.window.set_wmclass(wm_class, wm_class)
self.window.set_role(wm_class)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing a deprecation sounds useful, but why does this come with a commit message claiming it is relevant to python 3.8?

@@ -910,7 +910,7 @@ def build_list(self, *args):
updates_available = self.spices.get_n_updates()
self.update_all_button.set_sensitive(updates_available)
if updates_available > 0:
msg_text = _("Update all") + ' (' + ngettext("%d update available", "%d updates available", updates_available) % updates_available + ')'
msg_text = _("Update all") + ' (' + gettext.ngettext("%d update available", "%d updates available", updates_available) % updates_available + ')'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd intuitively expect any future users of the API to want to use ngettext() directly, but maybe that is just me.

@clefebvre
Copy link
Member

I'll close this for now but we should chase it and fix it all the same. The reason I close it is because neither fixes work here but more importantly they shouldn't be part of the same PR.

Window role:

I don't mind something more modern than self.window.set_wmclass(wm_class, wm_class) but self.window.set_role(wm_class) doesn't seem to work as well. Or if it does, then we also need to modify GWL (the grouped-window-list applet) to take the change into account.

As it is, when you launch cinnamon-settings applets from master you get an applets icon in GWL which is the same as in your alt-tab selector. With this code change, this is broken and GWL shows the generic icon for System Settings.

Ngettext

What's going on with Python 3.8 and ngettext? Where is the regression coming from? Testing here with python 3.6 in French, on master I get "9 mises a jour disponibles".. if I switch from gettext to gettext.ngettext or if I add from gettext import ngettext, it fails to fetch the translation and I end up with the English "9 available updates".

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

@clefebvre I rationalized this in #8964

@claudiux claudiux deleted the compat-python3.8 branch November 22, 2019 16:12
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.

cinnamon-settings applets crashes
3 participants