Support for GNOME 3.26 - 3.38, and GNOME 40 #127
Conversation
This makes the extension compatible with GNOME 3.36 without giving up support for older GNOME versions. Tested with 3.26, 3.34, 3.36; should run on older versions as well. Rationale for the implementation: there are two basic types of classes in GNOME shell: plain ES6 classes, and classes that extend Gobject and are created with GObject.registerClass(). Sometimes a plain class is converted to a GObject class in a later GJS version. In this case, the syntax for derived classes has to be converted from plain to GObject, too. In extensions that need to support a broader range of GNOME versions, these two class definitions can't be easily combined with conditional code. The main reason is the different initialization of the object itself (constructor vs. _init() and the base object (super() vs. super._init()). However, it's possible to separate the actual implementation from the class definition, and use bind() to make access "this" in these functions in different ways, depending on the way the class is defined. For simple classes with just one method (constructor), such as ArgosMenuItem, this can be done using a generic function.
Use a wrapper to implement GNOME-version-dependend access to object.actor. This fixes the incompatibility with GNOME <= 3.30 introduced by bda621e ("Remove deprecated use of object.actor in ArgosButton").
Use GNOME-version dependent method to convert bytearray to String. This fixes the incompatibility with earlier GNOME versions introduced by da857c0 ("Use new method for converting byte array to string).
JS WARNING: [utilities.js 135]: reference to undefined property "alternate"
Even older versions may be supported, too, but I didn't try yet.
The "version" field in metadata.json is for internal bookkeeping at extensions.gnome.org only, and shouldn't be set by locally installed extensions. Doing it nonetheless can have highly surprising effects, including wiping the entire extension from disk without alerting the user (see e.g. https://gitlab.gnome.org/Infrastructure/extensions-web/issues/102#note_739364).
GNOME shell 3.36 has lost the AltSwitcher class that used to be
defined in ui.status.system with commit 147a743d8d79 ("system: Replace action
icons with regular menu items"). Import the code for this class in argos.
Fedora 33 ships with GNOME 3.38 (but with version validation disabled) and this extension works fine there. Fedora 34 beta has GNOME 40 (with version validation enabled) and the extension works once the version is added to the list of supported versions. Signed-off-by: Michel Alexandre Salim <michel@michel-slm.name>
Mark this as compatible with GNOME 3.38 and 40
Numerical ordering pretends that 40.0 was still 3.40.0. See https://discourse.gnome.org/t/new-gnome-versioning-scheme/4235
utilities.js: fix shellVersion for GNOME 40+ versioning scheme
Backward-compatible GNOME 3.36 support
|
I've been running this branch on 40 for a while and I can confirm it works fine. |
|
I've been using this branch with gnome-shell 3.36.7-0ubuntu0.20.04.1 for a few weeks and I haven't encountered any issues. |
|
@dvogel and @real-or-random, thanks for the feedback. Please note that I added the shellVersion patch on top just today, so if you could update to exactly this version I'd be grateful. |
I had not noticed that. I just updated to this latest commit and restarted gnome-shell. No issues so far. |
|
I'm using this too with GNOME Shell 40.1 with no issues. |
| } else if ((i + 1) < dropdownLines.length && | ||
| dropdownLines[i + 1].menuLevel === dropdownLines[i].menuLevel && | ||
| dropdownLines[i + 1].hasOwnProperty("alternate") && | ||
| dropdownLines[i + 1].alternate === "true") { |
There was a problem hiding this comment.
Please use spaces, not tabs, for indentation.
| PopupMenu.PopupBaseMenuItem, | ||
| _ArgosMenuItem_superArg, | ||
| _ArgosMenuItem_init, | ||
| "ArgosMenuItem"); |
There was a problem hiding this comment.
The ); should be on a new line and outdented.
| @@ -3,6 +3,14 @@ | |||
| "name": "Argos", | |||
| "description": "Create GNOME Shell extensions in seconds", | |||
| "url": "https://github.com/p-e-w/argos", | |||
| "version": 3, | |||
There was a problem hiding this comment.
From the commit message for this change:
The "version" field in metadata.json is for internal bookkeeping at
extensions.gnome.org only, and shouldn't be set by locally installed
extensions. Doing it nonetheless can have highly surprising effects,
including wiping the entire extension from disk without alerting the
user (see e.g.
https://gitlab.gnome.org/Infrastructure/extensions-web/issues/102#note_739364).
I don't know the accuracy of this, just quoting the commit message for discussion here.
There was a problem hiding this comment.
It's not clear what the best course of action is here. Should version be set in the metadata.json that is uploaded to extensions.gnome.org, or does extensions.gnome.org do so itself? It's reassuring to see that the extensions system remains an underdocumented mess.
| @@ -167,7 +172,8 @@ function parseLine(lineString) { | |||
| } | |||
|
|
|||
| line.hasAction = line.hasOwnProperty("bash") || line.hasOwnProperty("href") || | |||
| line.hasOwnProperty("eval") || line.refresh === "true"; | |||
| line.hasOwnProperty("eval") || | |||
| (line.hasOwnProperty("refresh") && line.refresh === "true"); | |||
| } | ||
|
|
||
| if (shellVersion <= 33400) | ||
| var AltSwitcher = imports.ui.status.system.AltSwitcher; |
There was a problem hiding this comment.
Indentation doesn't match the else clause.
| _init(standard, alternate) { | ||
| super._init(); | ||
| this._standard = standard; | ||
| this._standard.connect('notify::visible', this._sync.bind(this)); |
There was a problem hiding this comment.
Please use double quotes like elsewhere in the code.
|
Thank you so so so much |
|
@p-e-w, @insign, thanks a lot. Anyone reading this, I could use help with these style fixes. As mentioned before, I'm really not an experienced Javascript programmer at all. I really just did what was necessary to make argos work again. I'll look into the requested changes/improvements, but they are quite many, and it'll take time. So if anyone has time to help out (ideally in the form of PRs against my master branch), I'd greatly appreciate it and we might be able to achieve a mainline merge more quickly. |
|
@insign, I believe you but my experience with this tool is exactly zero. |
|
The standard is "what the rest of the code looks like". I'm not going to write up formal requirements, sorry. I don't understand what the problem is here. I'm asking that tabs be changed to spaces (which all of the existing code uses), single quotes be changed to double quotes (which all of the existing code uses), indentation matches the surrounding code, etc.. That has nothing to do with JavaScript per se. I'm not asking for any functional change, just that the style of the new code fits with the code that is already there so that the quality of the codebase doesn't degrade. |
|
Understood. I haven't said that it's a problem, just that I need to find time for making the changes. |
|
Great work! |
|
Is this PR still needing help regarding formatting/styling? |
Yes, nothing has been done about that. I simply had no time.
That would be highly appreciated. |
|
@mwilck Would you mind if I pull your commits into a new branch/PR and make the requested changes (or you can pull the changes into this branch/PR)? |
Most of these changes were requested in p-e-w#127 (review) Other changes (line wrapping, const naming, additional consts) came from looking at the rest of the code.
|
@mwilck As requested in #127 (comment), I have opened a PR for your master branch: mwilck#4 @p-e-w Would you mind reviewing the changes in mwilck#4 ? I will iterate the style fixes there 🙏 |
Most of these changes were requested in p-e-w#127 (review) Other changes (line wrapping, const naming, additional consts) came from looking at the rest of the code.
JS style fixes and code readability improvements.
|
@p-e-w would appreciate it if you can take another look at this PR, I have addressed most (if not all) of your style concerns 🙏 |
|
MERGED!!! I would like to thank @mwilck, @jefferyto, @michel-slm, and @rammie for their combined effort to finally bring Argos into GNOME's present. While I don't use Argos myself anymore, it feels great to see that others care about the project enough to invest time to keep it working. |
After waiting for a long time, I'm submitting my GNOME shell compatibility work for argos here. See #108 and #111.