Introduce some utility singletons, and some minor refactoring #354
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 24 commits
June 10, 2024 03:31
So that we can also get rid of a 'utility class'
Using timeouts in an extension can be somewhat common. For every timeout you create, you need to make sure that it gets removed when the extension is disabled to pass EGO's review. Simply returning `GLib.SOURCE_REMOVE` isn't enough since the extension may get disabled while the timeout is running. EGO is fairly strict about this. So you usually need to create a variable to track the timer and remove it on `disable`. When using timers, it's also common to remove the previously started timer when a new one starts. This commits adds a convenience class that automatically takes care of these 2 issues.
For now this only has getters and setters for better 'typing' and editor integration. The following commits will add more features...
... quoting commit a0ecbd5: The extension is overriding the user settings by writing on them, sadly this implies various issues because even if we reset them on extension unloading, a shell crash or mis-behavior could always lead to affect user settings. Tiling assistant replaces the user settings when running, but not to loose the user settings, we need to save them around so that when the extension gets unloaded they can be restored. So add a SettingsOverrider class that: - Replaces the user settings with the extensions one - Remembers what has been changed and saves it locally and in our settings - When the extension is unloaded the settings are restored or reset Now, in case gnome-shell is stopped or the extension has crashed for some reason, once the extension is reloaded we check if previous settings were saved and in case they are used to restore the user settings on destruction.
and automatically unregister them up on `disable`.
It is fairly common for extensions to tie features to toggles in their settings. Extensions would listen for a settings change and apply/revert a feature. This commit adds a convenience method to register a handler for a settings change and optionally apply the handler `immediate`-ly. The naming is inspired by Vues watchers.
In the past I've been very conservative with changes to existing classes/objects and created wrapper classes (like utility.js::Rect) or attached properties to my own objects even if it didn't make that much sense semantically (e. g. the signals in tilingWindowManager.js). However, that's a bit cumbersome to work with. This singleton should make it easier to 'extend' existing objects. It can inject new accessor and data properties into objects. It also uses GNOME Shell's own InjectionManager to modify, replace or inject a method into a (prototype) object. On the extension's disable call, the singleton will clean up and revert all the changes.
Previously, there was the wrapper class utility.js::Rect. But now that there is the Injections singleton, it's easier to just inject all custom properties and methods into Mtk.Rectangle and use the rect directly. This is pretty much just a copy/paste of the methods from the wrapper class. I've only omitted some unused methods.
eventually, I want to get rid of the signal class since that was a dumb "custom implementation of signal tracking"
Utility is the only class in the file so might as well turn the methods into top level functions since a static Util class feels a bit weird to me. Also, remove an unused debugging functions that was actually buggy because it imported the module wrongly.
for better typing support (at least in VSCode).
In the past I thought it made sense to restore a tile window when the grab is held but the pointer didn't move. But I no longer think so as I can't come up with a reason as to when a user would want untile a window but not move the pointer...
This commit moves the native overrides to a separate function. This makes `enable()` easier to read.
since they weren't used as object instances any way. They were just "handler" classes/entry points for features/modules that were instanced in extension.js.
This will allow for a more pleasant dev experience by adding better auto- complete and typing. Especially, for custom properties and methods that are injected into GNOME's objects. This is probably also the extent of 'typing' I plan to add. I don't intend to enable `checkJs` nor do I plan to transition to Typescript.
I've temporarily enabled `checkJs` and this uncovered some issues. They were mostly wrong JSDoc comments. But there were also 2 actual bugs albeit minor ones. It was the usage of non-existing variables from GLib and Gdk. Also remove brackets in extension.js to satisfy the linter even though the new lint rules will add them again...
The xAnchor parameter was used to restore the size of a window while keeping the titlebar at the relative x position. Our extra calculations were no longer needed (and also were no longer working) since GNOME 44 (https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2942). So remove that code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another set of fairly large changes when looking at the diffs but each commit should be fairly straightforward. The main content here is the introduction of 3 utility singletons to some stuff more automatically. Another feature is the introduction of "types". It's only meant to facilitate auto-complete. I don't intend to switch to TypeScript or checking of JS as the types for GNOME Shell are currently lacking and it would be more work to add them all.
Timeouts
Using timeouts in an extension can be somewhat common. For every timeout you create, you need to make sure that it gets removed when the extension is disabled to pass EGO's review. Simply returning
GLib.SOURCE_REMOVEisn't enough since the extension may get disabled while the timeout is running. EGO is fairly strict about this. So you usually need to create a variable to track the timer and remove it ondisable. When using timers, it's also common to remove the previously started timer when a new one starts. This commits adds a convenience class that automatically takes care of these 2 issues.Settings
Previously there was a settings singleton that was just wrapping a
Gio.Settingsinstance and basically copy-pasted all methods just to delegate it to theGio.Settingsinstance. It didn't provide any usefulness. Now the singleton will expose the settings as getters/setters to improve the auto-complete experience as well as provide "types". It also allows to register shortcuts that will be cleaned up ondisable. It also integrates the "override native settings" feature and uses a more common "watch" method.Injections
In the past I've been very conservative with changes to existing classes/objects and created wrapper classes (like utility.js::Rect) or attached properties to my own objects even if it didn't make that much sense semantically (e. g. the signals in tilingWindowManager.js). However, that's a bit cumbersome to work with. This singleton should make it easier to "extend/inject" existing objects. It can inject new accessor and data properties into objects. It also uses GNOME Shell's own InjectionManager to modify, replace or inject a method into a (prototype) object. On the extension's disable call, the Singleton will clean up and revert all the changes.