GNOME Bugzilla – Bug 774072
Add extension reload button to gnome-tweak-tool
Last modified: 2018-01-24 15:17:25 UTC
Created attachment 339269 [details] [review] Extension Reload Button patch The attached patch adds an Extension Reload Button to the gnome-tweak-tool. Wayland does not allow restarting the Shell session. The gnome-shell-extension-tool (reference bug 772593) is executed by this button to reload an extension. I have written a gnome-shell extension which creates a panel menu button to reload extensions but providing this function in the Tweak Tool seems more appropriate.
Created attachment 341588 [details] [review] Updated Extension Reload Button patch Added code to allow only enabled extensions to have sensitive reload button and to check return code of gnome-shell-extension-tool for failure.
Review of attachment 341588 [details] [review]: Thanks for the patch, review inline ::: gtweak/tweaks/tweak_group_shell_extensions.py @@ +136,3 @@ + execute_subprocess(["notify-send", _("Error reloading"), uuid], block=False) + else: + execute_subprocess(["notify-send", _("Reloading"), uuid], block=False) can you please re-write this method to use the dbus apis instead of execute_subprocess() ? @@ +146,3 @@ else: self._shell.enable_extension(uuid) + self.reloadButton.set_sensitive(self.switch.get_active()); I'd prefer you used g_object_bind_property() to avoid having to do this
Created attachment 341670 [details] [review] Updated patch after review 1. Changed to use Notification class from Utils instead of execute_subprocess(). 2. Changed to bind reload btn to sw. 3. Removed un-needed assignments of reload btn and sw.
Review of attachment 341670 [details] [review]: Thanks for working on this! Can you submit a git formatted patch so that this is properly attributed to you? ::: gtweak/tweaks/tweak_group_shell_extensions.py @@ +106,3 @@ + btn.props.valign = Gtk.Align.CENTER + btn.connect("clicked", self._on_reload_clicked, uuid) + btn.set_sensitive(sw.get_active()); If you bind the property below with the G_BINDING_SYNC_CREATE flag instead of 0 you won't need this line. @@ +131,3 @@ + def _on_reload_clicked(self, btn, uuid): + stdout, stderr, returncode = execute_subprocess(["gnome-shell-extension-tool", "-r", uuid], block=True) I was more referring to this one in particular. Instead of relying an external tool, we can call the gnome-shell dbus API here directly. See the gshellwrapper.py file where you'd need to add a method to call org.gnome.Shell.Extensions.ReloadExtension @@ +135,3 @@ + Notification(_("Error reloading"), uuid) + else: + Notification(_("Reloading"), uuid) I don't think using a global notification makes sense for this. Maybe a simple GtkLabel that we display inside the extension row only in case of error?
Created attachment 341723 [details] [review] Updated add reload button to gnome-tweak-tool patch. 1. git formatted patch 2. Corrected binding flag 3. Added method to gshellwrapper.py for dbus 4. Removed Notifications 5. Since the extension is in error state if reload fails set switch to reflect error state and displayed a label with failure message.
Created attachment 342380 [details] [review] Add an effective extension reload button to the Tweak Tool Restarting the shell with the "Atl F2 r" is needed most often when the extension is in the error state. The shell ReloadExtension method call will not complete successfully unless the extension state is enabled. This patch sets the extension state to enabled before calling the shell reload code. If the error in the extension code has been corrected the reload will complete. If not the extension will remain in the error state with no ill effects. The error message label added in an earlier submitted patch is removed. An icon with a tool tip is now displayed on reload error to follow the original Tweak Tool design.
Review of attachment 342380 [details] [review]: ::: gtweak/gshellwrapper.py @@ +168,3 @@ + def reload_extension(self, uuid): + return self._proxy.proxy_extensions.ReloadExtension('(s)', uuid) There's nothing to return here ::: gtweak/tweaks/tweak_group_shell_extensions.py @@ +7,3 @@ from gi.repository import Gtk from gi.repository import GLib +from gi.repository import GObject don't think we need this import @@ +145,3 @@ + if not self.sw.get_active(): + self.sw.set_active(True) + except Exception, e: Unfortunately, this isn't how this DBus api works. ReloadExtension() doesn't return anything, you need to call GetExtensionInfo() and check the 'state' and 'error' properties. As is, this code would only ever get errors (python exceptions) from the underlying GDBus proxy, i.e. errors from the dbus communication itself.
Created attachment 343324 [details] [review] Add Reload Extension Button to Tweak Tool Reply to Comment #7 Unfortunately, this isn't how this DBus api works. ReloadExtension() doesn't return anything, you need to call GetExtensionInfo() and check the 'state' and 'error' properties. As is, this code would only ever get errors (python exceptions) from the underlying GDBus proxy, i.e. errors from the dbus communication itself. --- Catching the dbus errors is required. I have examined the extension reload process in extensionUtils.js and extensionSystem.js. On line 144 in extensionSystem.js is "extension.error = '';". This is the error field in the object returned by GetExtensionInfo(). I searched the shell source code and did not find another instance where an assignment to this field is made. I included code to handle it if it shows up. Javascript exceptions are generated when the Shell imports an extension's code containing syntax errors. The exceptions in the shell will cause a GDBus error which must be handled. I added the following after the "try: except:" to get an example. I tested many syntax errors which all produced GDBus.Error:org.gnome.gjs.JSError's. info_obj = self._shell.get_extension_info(uuid) print (info_obj["type"],"type") print (info_obj["state"],"state") print (info_obj["error"],"error") print (info_obj["path"],"path") print (info_obj["hasPrefs"],"hasPrefs") $ gnome-tweak-tool GDBus.Error:org.gnome.gjs.JSError.SyntaxError: missing } after property list (2.0, 'type') (3.0, 'state') ('', 'error') ('/home/bogwan/.local/share/gnome-shell/extensions/donotdisturb-button@nls1729', 'path') (True, 'hasPrefs') From shell log: Jan 08 18:04:19 valkyrie2.att.net org.gnome.Shell.desktop[4979]: (gnome-shell:4979): Gjs-CRITICAL **: JS ERROR: Exception in method call: ReloadExtension: SyntaxError: missing } after property list @ /home/bogwan/.local/share/gnome-shell/extensions/donotdisturb-button@nls1729/extension.js:15
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-tweaks/issues/79.