Add support for Gnome 45

[Abdullah-2003]

Please add support for gnome v45

[raujonas]

It seems like a lot in the background has changed (https://gjs.guide/extensions/upgrading/gnome-shell-45.html) and it's not only bumping the version this time, so sadly I cannot promise any timeline at the moment.

[sjkl90]

I want to tell you that for me, it is the best extension that exists, I don't know anyone who can help, I wish I could... I want you to understand that the Gnome community needs this extension, I hope to be able to give my support, even if it is financially (not I have a lot) :D.

The problem :
`The settings of extension executor@raujonas.github.io had an error:

SyntaxError: import declarations may only appear at top level of a module

Stack trace:

@file:///home/crs/.local/share/gnome-shell/extensions/executor@raujonas.github.io/prefs.js:7:24
_init/GLib.MainLoop.prototype.runAsync/</<@resource:///org/gnome/gjs/modules/core/overrides/GLib.js:266:34

`

[Flimm]

This blog post on GNOME Shell's development blog looks relevant: Extensions in GNOME 45 (2 Sept 2023). Here's a quote:

Extensions that target older GNOME versions will not work in GNOME 45. Likewise, extensions that are adapted to work with GNOME 45 will not work in older versions.

You can still support more than one GNOME version, but you will have to upload different versions to extensions.gnome.org for pre- and post-45 support.

Will pull requests that only work with GNOME 45 be accepted? It seems like it will be difficult to use the same codebase to support both GNOME 45 and earlier versions of GNOME.

[raujonas]

Thanks for all your effort and input! Yes, I think only making it compatible with Gnome 45 and removing the old code for new versions will be the way to go. @Sierra410 already created a draft here #72 but I had no time to test it yet, hopefully today.
So if anyone wants to test the changes on his branch already, please feel free to do so! Hopefully we can get a compatible version up soon.

Edit: this PR looks very promising, I just have two questions but we should have a new version very soon!

[sjkl90]

I'm trying it and it works perfectly for me. Thank you so much

[rouelle]

Same for me, everything works for now! As I use Gnome 45 (Arch Linux) with french Canada locale I can add that translation in french of Executor also works.

[tabascoz]

thanks - ti works ok for me on Gentoo Linux, gnome shell 45

[raujonas]

Thanks for testing, that's good to hear. I'll package and upload the new version tomorrow.

[rouelle]

[...] translation in french of Executor also works.

Just a small glitch for French: the last tab in Executor panel should be « Général » with accents;
also the option is not translated: « Click on output in top active bar » should be « Affichage au clic de la barre de statut »

Merci! Thanks!

[raujonas]

It's in review now in the store 👋

[raujonas]

Ok, it was rejected today, so I have to fix the findings and submit a new version, but it should not be that much effort. Sorry that it takes longer.

Edit: I'm not entirely sure what the first point means in the review, do you have an idea @Sierra410?

[Sierra410]

Looks like this.settings = this.getSettings(); is the culprit?

I assume that the ExecutorPreferences instance is persistent, so by setting a member variable in it, the settings objects never gets garbage collected?

The object should be made local to the method (const settings = ...) to fix it.

The object appears to be used in a few other methods. Some, like clickOnOutputActiveClicked, don't contain much logic at all and are wrapped in an arrow function anyway, so I propose to just remove them altogether and move the logic into the aforementioned arrow functions.

So, this:

clickOnOutputActive.connect('notify::active', () => {
	this.clickOnOutputActiveClicked(clickOnOutputActive.get_active());
});

Should be this:

clickOnOutputActive.connect('notify::active', () => {
	settings.set_boolean('click-on-output-active', clickOnOutputActive.get_active());
});

saveCommands() method is relatively long, so it should be left as it, I think. In that case, the object should be passes as an argument.

By the way, most instances of .bind(this) can be removed by using arrow functions. e.g. saveCommands = () => {...}

[raujonas]

Thanks for your feedback @Sierra410, that is highly appreciated! I decided to only fix the issue mentioned in the review now so that you all get the new version asap. I'll then have a look at the bindings separately, I also noticed that there's an error happening sometimes when saving the settings.

If anyone wants to test the changes again, it's in #73

[raujonas]

It's now in review again 🍀

[raujonas]

It's available! Thanks for all your support again!

[Flimm]

Wonderful! Thank you so much. It's working well, here.