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

Add compatibility with Gnome-Shell < 3.34 - for example RHEL 8 with 3… #790

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NVieville
Copy link
Contributor

Hello,

Just hitting this bug report:

https://bugzilla.redhat.com/show_bug.cgi?id=2184351

This pull request helps to resolve this issue:

Add compatibility with Gnome-Shell < 3.34 - for example RHEL 8 with 3.32.2
ModalDialog was converted to GObject.registerClass in 3.34

According to:

https://access.redhat.com/product-life-cycles?product=Red%20Hat%20Enterprise%20Linux,OpenShift%20Container%20Platform%204

RHEL 8 will be maintained until may 31 2029, and maybe until may 2031, so it's
probably worth keeping its users have this extension working.

Cordially,

--
NVieville

Copy link
Collaborator

@ZimbiX ZimbiX left a comment

Choose a reason for hiding this comment

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

I'm not across what GObject.registerClass is for, but I reckon it'd be nicer to deduplicate the code - something like:

const smDialogClass = class SystemMonitor_smDialog extends ModalDialog.ModalDialog {
    // ...
}
const smDialog = (shell_Version >= '3.34') ? GObject.registerClass(smDialogClass) : smDialogClass

@ZimbiX
Copy link
Collaborator

ZimbiX commented Apr 16, 2023

The wrapping was introduced in #777 (25fb5f5), from issue #551

@NVieville
Copy link
Contributor Author

Hello,

Totally agree with you about deduplicating code.
There is an example similar to your proposal here:

https://wiki.gnome.org/Attic/GnomeShell/Extensions/Writing#Version_Compatibility

As I didn't wanted to modify the original code, I just duplicate it and wrap it or not in the registerClass call.
It was more of a quick and easy solution (KISS) to address the issue of launching the extension on RHEL 8.
The example above also uses _init in place of constructor call, but I didn't inspect the consequences of such a switch.

Do you think a new proposal is needed? Or will you implement your proposal?

Thank you for your comment.

Cordially,

--
NVieville

@NVieville
Copy link
Contributor Author

Hello,

I've updated the change proposal according to your advice to not duplicate code.
This proposal has been tested on RHEL 8 (gnome-shell 3.32.2) and Fedora 37 (gnome-shell 43.4) and it works as expected.

I would be very pleased if you could give me your opinion on a proposal of the form suggested here:

https://wiki.gnome.org/Projects/GnomeShell/Extensions/MigratingShellClasses#Migrating_from_Native_to_GObject

A patch against the actual proposal would look like this:

diff --git a/system-monitor@paradoxxx.zero.gmail.com/extension.js b/system-monitor@paradoxxx.zero.gmail.com/extension.js
index 5874451..0fc409b 100644
--- a/system-monitor@paradoxxx.zero.gmail.com/extension.js
+++ b/system-monitor@paradoxxx.zero.gmail.com/extension.js
@@ -283,9 +283,9 @@ const smStyleManager = class SystemMonitor_smStyleManager {
     }
 }
 
-const smDialogClass = class SystemMonitor_smDialog extends ModalDialog.ModalDialog {
-    constructor() {
-        super({styleClass: 'prompt-dialog'});
+var smDialog = class SystemMonitor_smDialog extends ModalDialog.ModalDialog {
+    _init() {
+        super._init({styleClass: 'prompt-dialog'});
         let mainContentBox = new St.BoxLayout({style_class: 'prompt-dialog-main-layout',
             vertical: false});
         this.contentLayout.add(mainContentBox,
@@ -326,7 +326,12 @@ const smDialogClass = class SystemMonitor_smDialog extends ModalDialog.ModalDial
 
 // Add compatibility with Gnome-Shell < 3.34 - for example RHEL 8 with 3.32.2
 // ModalDialog was converted to GObject.registerClass in 3.34
-const smDialog = (shell_Version >= '3.34') ? GObject.registerClass(smDialogClass) : smDialogClass
+if (shell_Version >= '3.34') {
+    smDialog = GObject.registerClass(
+        {GTypeName: 'smDialog'},
+        smDialog
+    );
+}
 
 const Chart = class SystemMonitor_Chart {
     constructor(width, height, parent) {

This form has not been tested, and I don't measure the impact of switching from constructor to _init, not that of switching from const to var for the smDialog object either .

Any comment are welcome.
Hope the proposal will fit the requirements and will be adopted.

Cordially,

--
NVieville

….32.2

ModalDialog was converted to GObject.registerClass in 3.34

Signed-off-by: NVieville <nicolas.vieville@uphf.fr>
@NVieville
Copy link
Contributor Author

Hello,

The pull request have been updated according to my last message and as suggested here:

https://wiki.gnome.org/Projects/GnomeShell/Extensions/MigratingShellClasses#Migrating_from_Native_to_GObject

This new way to add compatibility with Gnome-Shell < 3.34 has been tested successfully on Fedora 37 and RHEL 8.

Hope this will help to provide this extension to these distributions.

Cordially,

--
NVieville

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

2 participants