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

[Feature Request] Gnome 45 update #38

Closed
gilvbp opened this issue Sep 7, 2023 · 39 comments · Fixed by #39
Closed

[Feature Request] Gnome 45 update #38

gilvbp opened this issue Sep 7, 2023 · 39 comments · Fixed by #39

Comments

@gilvbp
Copy link

gilvbp commented Sep 7, 2023

Hi, could you update the G45 version?

Reference:
https://gjs.guide/extensions/upgrading/gnome-shell-45.html#esm

Thanks

@gilvbp
Copy link
Author

gilvbp commented Sep 8, 2023

Could I open a PR for this issue?

@mgalgs
Copy link
Owner

mgalgs commented Sep 8, 2023

Absolutely!

@gilvbp
Copy link
Author

gilvbp commented Sep 9, 2023

@mgalgs, could you help me? I already have ported most part of the code, but I'm stuck on this problem:

image

https://github.com/gilvbp/gnome-shell-system-monitor-applet/blob/gnome-4.5/system-monitor%40paradoxxx.zero.gmail.com/extension.js

Thanks!

@mgalgs
Copy link
Owner

mgalgs commented Sep 13, 2023

Vou dar uma olhada amanhã! After a quick perusal one thing I'd request is to change your approach for refactoring in b8fe462 so that it's more easily reviewable... For example, rather than pulling the definitions for enable() and disable() into the new extension class, you could just leave them alone (maybe rename them to something like extension_{enable,disable}) and call those global functions from the class methods. Or maybe if you just define the class at the bottom of the file so that the diff between enable/disable and the new class enable/disable methods is cleaner. As of right now it's extremely hard to tell if anything changed in those functions.

@ZimbiX
Copy link

ZimbiX commented Sep 14, 2023

Also, could you explain why you've removed the support for previous versions of Gnome? Distros with slow releasing should still be able to use the latest version of the extension imho. btw, it's a good idea to explain in a commit message why you've done what you've done in a commit. Thanks for the contribution. I'm not yet across why the changes are necessary, but just a heads up that it was very difficult to review, so I could only give a cursory look.

@mgalgs
Copy link
Owner

mgalgs commented Sep 14, 2023

Also, could you explain why you've removed the support for previous versions of Gnome?

Looks like that's how the Gnome devs are expecting it to work:

image

https://gjs.guide/extensions/upgrading/gnome-shell-45.html#shell-version

So I guess we'll have to create a new branch for legacy shell versions (in case fixes/updates still need to happen there) and update the uploader Github action accordingly...

btw, it's a good idea to explain in a commit message why you've done what you've done in a commit. Thanks for the contribution.

Agreed. @gilvbp would be nice if you could flesh out the commit messages a bit more. Links to the docs explaining why each change is necessary, etc.

@ZimbiX
Copy link

ZimbiX commented Sep 15, 2023

So I guess we'll have to create a new branch for legacy shell versions (in case fixes/updates still need to happen there) and update the uploader Github action accordingly...

Wow, that's pretty sad that that's the official instructions. I think it'd be worth investigating if it can be wrapped in a conditional like with the other backwards compatibility stuff the extension has. Or some other method of having only one copy of the source.

@rathann
Copy link

rathann commented Sep 25, 2023

Fedora 39 is going to ship with GNOME 45, so it'd be nice if we could get GNOME 45 support before the final freeze date (Oct 3rd). Thanks for working on this, even if you can't make it in time to include in Fedora 39 release. We can always ship an update later.

@mgalgs
Copy link
Owner

mgalgs commented Sep 25, 2023

Yeah, I think I'd like to re-work this PR to only include the minimal changes to comply with the new requirements for extensions on Gnome 45. I'll see if I can find time this week unless someone else beats me to it!

@ixevix
Copy link

ixevix commented Oct 13, 2023

any progress on this?

@laurentpayot
Copy link

Ubuntu 23.10 was released yesterday, also with Gnome 45...

@freddyw
Copy link

freddyw commented Oct 13, 2023

Running Fedora 39 (Beta) here, happy to test if needed.

@mgalgs
Copy link
Owner

mgalgs commented Oct 14, 2023

Sorry, I got swamped with work but hoping to have some time next week. Help testing would be great.

@ixevix
Copy link

ixevix commented Oct 14, 2023

I can test it out. Arch upgraded GNOME to 45 already and all extensions stopped working.

@sibradzic
Copy link

Happy to do some tests if needed

@Totto16
Copy link

Totto16 commented Oct 15, 2023

also happy to help in the porting process or with some errors that might occur, just ported some other extensions to gnome 45, so I could help with some things if needed

@badsmoke
Copy link

I am now also on gnome 45, but unfortunately have no experience to debug gnome extensions.

@gilvbp what about your branch?
@Totto16 maybe you can help there

@Totto16
Copy link

Totto16 commented Oct 16, 2023

Yes I certainly could, I saw in this thread, that some ports already exist, or am I wrong about that? I look around, and maybe we should make a PR, that is the best port so far... 🤔

@mgalgs
Copy link
Owner

mgalgs commented Oct 16, 2023

Is anyone here able to successfully restart their Gnome Shell session under GS 45 using Alt-F2 r? I've tried on three different distros (Fedora 39 beta, Ubuntu 23.10, and Arch Linux) under GS 45 and they all crash and give me the "Oh no! Something has gone wrong." unhappy computer... I have to leave extensions enabled overall (using the gnome shell extensions app) to reproduce the crash. If I disable extensions support the crash goes away. But if I enable extensions support, even if I've disabled all individual extensions, then it crashes when I try to do a r. This is making it difficult to do development, obviously...

@mgalgs
Copy link
Owner

mgalgs commented Oct 16, 2023

I'm hoping this is just something with my setup, but seeing as how it's affecting 3 different distros (including fresh installs of Fedora 39 beta and Ubuntu 23.10), I've filed a Bug report filed with Gnome Shell: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/7109

@Totto16
Copy link

Totto16 commented Oct 16, 2023

You can debug extensions without restarting your current session: see here

e.g.

dbus-run-session -- gnome-shell --nested --wayland

With that method I tested some extension, even if the normal reload (in x11 mode) crashed

mgalgs added a commit that referenced this issue Oct 17, 2023
Fixes #38

Suggested-by: Gil Vicente Bernardo Pinto <gilvbp@gmail.com>
mgalgs added a commit that referenced this issue Oct 17, 2023
Fixes #38

Suggested-by: Gil Vicente Bernardo Pinto <gilvbp@gmail.com>
@mgalgs
Copy link
Owner

mgalgs commented Oct 17, 2023

@Totto16 do you know how to run Looking Glass inside the nested Gnome Shell session under Wayland? I'm trying to get better logs...

@Totto16
Copy link

Totto16 commented Oct 17, 2023

@mgalgs

I used this approach, to see all logs:

(taken from here)

#!/bin/sh -e

export G_MESSAGES_DEBUG=all
export MUTTER_DEBUG_DUMMY_MODE_SPECS=1366x768

dbus-run-session -- \
    gnome-shell --nested \
                --wayland

( I put this into a script)

With that you can grep / redirect output to a file and search there more easily

Looking Glass has to be started, which takes some time and effort, if you restart many times 😓

mgalgs added a commit that referenced this issue Oct 17, 2023
Fixes #38

High-level summary of changes:

  - Migrated to es6 style imports [1]
  - Removed support for GS<45 [2]
  - Added default class exports in lieu of enable/disable functions [3]
  - Moved to subclassing Extension for main extension, with its associated
    helper properties/methods in lieu of extensionUtils [4]
  - Moved to subclassing ExtensionPreferences for prefs, with its
    associated helper properties/methods in lieu of extensionUtils [5]
  - Removed legacy Class syntax (Lang) [6]
  - Migrated to GLib-based Mainloop [7]
  - Use gettext from the extension module [8]
  - Migrated logging to standard console.* methods [9]
  - Migrated preferences dialog to Adwaita [10]

TODO:

  - [ ] Figure out Battery (Main.panel.statusArea.quickSettings._system)

Suggested-by: Gil Vicente Bernardo Pinto <gilvbp@gmail.com>

 [1] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#esm
 [2] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#shell-version
 [3] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extension-js
 [4] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [5] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#prefs-js
 [6] https://gjs.guide/guides/gjs/legacy-class-syntax.html#binding
 [7] https://discourse.gnome.org/t/how-to-import-mainloop-on-gnome-45-version/17605
 [8] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [9] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#logging
[10] https://gjs.guide/extensions/development/preferences.html#prefs-js
@mgalgs mgalgs mentioned this issue Oct 17, 2023
4 tasks
@mgalgs
Copy link
Owner

mgalgs commented Oct 17, 2023

Thanks, @Totto16. With debug logs turned on, the nested session via Wayland actually makes for a pretty decent dev environment!

@ALL It ain't pretty, but I have something working... The preferences page in particular needs some massaging by someone who knows Adwaita programming (no close button at the moment -- lol). I've opened a PR (#39) for testing and code review. Please take a look when you get a chance!

@koroki
Copy link

koroki commented Oct 17, 2023

Thanks, @Totto16. With debug logs turned on, the nested session via Wayland actually makes for a pretty decent dev environment!

@ALL It ain't pretty, but I have something working... The preferences page in particular needs some massaging by someone who knows Adwaita programming (no close button at the moment -- lol). I've opened a PR (#39) for testing and code review. Please take a look when you get a chance!

When I copy the files of PR#39, gnome does not detect the extension itself, so, I cannot check anything. Nothing in extensions or in the web local or in lg.

@mgalgs
Copy link
Owner

mgalgs commented Oct 17, 2023

@koroki Did you follow the "Testing Instructions" section of the PR or do something else? Can you please share the full steps you took?

@althaser
Copy link

althaser commented Oct 17, 2023

I think it is about a typo in the instructions, while creating the symbolic link the name must contain -next whereas is used in metadata.json in uuid,

ln -sv gnome-shell-system-monitor-applet/system-monitor-next@paradoxxx.zero.gmail.com

it works properly using X11 in ubuntu 23.10.

Also verified bad window in Preferences due to Adwaita stuff like you said.

thank you

mgalgs added a commit that referenced this issue Oct 17, 2023
Fixes #38

High-level summary of changes:

  - Migrated to es6 style imports [1]
  - Removed support for GS<45 [2]
  - Added default class exports in lieu of enable/disable functions [3]
  - Moved to subclassing Extension for main extension, with its associated
    helper properties/methods in lieu of extensionUtils [4]
  - Moved to subclassing ExtensionPreferences for prefs, with its
    associated helper properties/methods in lieu of extensionUtils [5]
  - Removed legacy Class syntax (Lang) [6]
  - Migrated to GLib-based Mainloop [7]
  - Use gettext from the extension module [8]
  - Migrated logging to standard console.* methods [9]
  - Migrated preferences dialog to Adwaita [10]

TODO:

  - [ ] Figure out Battery (Main.panel.statusArea.quickSettings._system)

Suggested-by: Gil Vicente Bernardo Pinto <gilvbp@gmail.com>

 [1] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#esm
 [2] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#shell-version
 [3] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extension-js
 [4] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [5] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#prefs-js
 [6] https://gjs.guide/guides/gjs/legacy-class-syntax.html#binding
 [7] https://discourse.gnome.org/t/how-to-import-mainloop-on-gnome-45-version/17605
 [8] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [9] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#logging
[10] https://gjs.guide/extensions/development/preferences.html#prefs-js
@mgalgs
Copy link
Owner

mgalgs commented Oct 17, 2023

@althaser that was it! Thank you, and fixed.

mgalgs added a commit that referenced this issue Oct 17, 2023
Fixes #38

High-level summary of changes:

  - Migrated to es6 style imports [1]
  - Removed support for GS<45 [2]
  - Added default class exports in lieu of enable/disable functions [3]
  - Moved to subclassing Extension for main extension, with its associated
    helper properties/methods in lieu of extensionUtils [4]
  - Moved to subclassing ExtensionPreferences for prefs, with its
    associated helper properties/methods in lieu of extensionUtils [5]
  - Removed legacy Class syntax (Lang) [6]
  - Migrated to GLib-based Mainloop [7]
  - Use gettext from the extension module [8]
  - Migrated logging to standard console.* methods [9]
  - Migrated preferences dialog to Adwaita [10]

TODO:

  - [ ] Figure out Battery (Main.panel.statusArea.quickSettings._system)

Suggested-by: Gil Vicente Bernardo Pinto <gilvbp@gmail.com>

 [1] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#esm
 [2] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#shell-version
 [3] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extension-js
 [4] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [5] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#prefs-js
 [6] https://gjs.guide/guides/gjs/legacy-class-syntax.html#binding
 [7] https://discourse.gnome.org/t/how-to-import-mainloop-on-gnome-45-version/17605
 [8] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [9] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#logging
[10] https://gjs.guide/extensions/development/preferences.html#prefs-js
@koroki
Copy link

koroki commented Oct 18, 2023

@koroki Did you follow the "Testing Instructions" section of the PR or do something else? Can you please share the full steps you took?

The system-monitor-next was the problem, yes. I did not know about that the directory name and the information in the metadata should be the same.

Thanks.

For now, the extension works properly. I will come here if I found something wrong.

@gilvbp
Copy link
Author

gilvbp commented Oct 18, 2023

Thanks, @Totto16. With debug logs turned on, the nested session via Wayland actually makes for a pretty decent dev environment!

@ALL It ain't pretty, but I have something working... The preferences page in particular needs some massaging by someone who knows Adwaita programming (no close button at the moment -- lol). I've opened a PR (#39) for testing and code review. Please take a look when you get a chance!

Great work! It's all good.

@badsmoke
Copy link

good work, it basically works

but there are still display errors

the big arrows:
image

and the settings menu

image

@marxin
Copy link

marxin commented Oct 18, 2023

Hey! Thanks for porting of the extension to Gnome 45. Right now, I can't enable the extension in Gnome:

$ pwd
/home/marxin/Programming/gnome-shell-system-monitor-applet
$ git log | head -n1
commit e881d934d3d3ffd237f57e163239991c7a98c7d7
$ cd ~/.local/share/gnome-shell/extensions/
$ ln -sv ~/Programming/gnome-shell-system-monitor-applet/system-monitor-next@paradoxxx.zero.gmail.com
$ marxin@marxinbox:~/.local/share/gnome-shell/extensions> cat system-monitor-next@paradoxxx.zero.gmail.com/metadata.json 
{
    "shell-version": ["45"],
    "uuid": "system-monitor-next@paradoxxx.zero.gmail.com",
    "name": "system-monitor-next",
    "url": "https://github.com/mgalgs/gnome-shell-system-monitor-applet",
    "description": "Display system information in GNOME Shell status bar, such as memory, CPU, disk and battery usages, network rates…\n\nThis fork of paradoxxxzero/gnome-shell-system-monitor-applet is for packaging purposes only. This extension is built and updated continuously with the upstream master branch, occasionally including patches that haven't yet merged upstream.\n\nThis is preferable for users on bleeding edge distributions that prefer not to wait for a stable release from the main repo. Of course, since we're releasing directly from master some instability is inevitable.\n\nIf you get an error after updating, try restarting Gnome Shell with Alt-F2 then 'r'.",
    "settings-schema":  "org.gnome.shell.extensions.system-monitor",
    "gettext-domain": "system-monitor",
    "version": -1
}

but:

marxin@marxinbox:~/.local/share/gnome-shell/extensions> gnome-extensions enable system-monitor-next@paradoxxx.zero.gmail.com
Extension “system-monitor-next@paradoxxx.zero.gmail.com” does not exist

Any idea what can be wrong? Thanks.

@althaser
Copy link

@marxin check with:

$ journalctl -xe /usr/bin/gnome-shell -f

if you can see any messages related to this extension.

@marxin
Copy link

marxin commented Oct 18, 2023

Cool, I've just renamed system-monitor@paradoxxx.zero.gmail.com/ to system-monitor-next@paradoxxx.zero.gmail.com/ in ~/Programming/gnome-shell-system-monitor-applet and it works now 🥳

Screenshot from 2023-10-18 15-33-04

@thekswenson
Copy link

I think it is about a typo in the instructions, while creating the symbolic link the name must contain -next whereas is used in metadata.json in uuid,

ln -sv gnome-shell-system-monitor-applet/system-monitor-next@paradoxxx.zero.gmail.com

I had to do ln -s gnome-shell-system-monitor-applet/system-monitor@paradoxxx.zero.gmail.com gnome-shell-system-monitor-applet/system-monitor-next@paradoxxx.zero.gmail.com and then ln -s gnome-shell-system-monitor-applet/system-monitor-next@paradoxxx.zero.gmail.com

mgalgs added a commit that referenced this issue Oct 18, 2023
Fixes #38

High-level summary of changes:

  - Migrated to es6 style imports [1]
  - Removed support for GS<45 [2]
  - Added default class exports in lieu of enable/disable functions [3]
  - Moved to subclassing Extension for main extension, with its associated
    helper properties/methods in lieu of extensionUtils [4]
  - Moved to subclassing ExtensionPreferences for prefs, with its
    associated helper properties/methods in lieu of extensionUtils [5]
  - Removed legacy Class syntax (Lang) [6]
  - Migrated to GLib-based Mainloop [7]
  - Use gettext from the extension module [8]
  - Migrated logging to standard console.* methods [9]
  - Migrated preferences dialog to Adwaita [10]

TODO:

  - [ ] Figure out Battery (Main.panel.statusArea.quickSettings._system)

Suggested-by: Gil Vicente Bernardo Pinto <gilvbp@gmail.com>

 [1] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#esm
 [2] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#shell-version
 [3] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extension-js
 [4] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [5] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#prefs-js
 [6] https://gjs.guide/guides/gjs/legacy-class-syntax.html#binding
 [7] https://discourse.gnome.org/t/how-to-import-mainloop-on-gnome-45-version/17605
 [8] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [9] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#logging
[10] https://gjs.guide/extensions/development/preferences.html#prefs-js
@mgalgs
Copy link
Owner

mgalgs commented Oct 18, 2023

Whoops, yeah the symlink/directory name mismatch didn't make it into the README. I've just now pushed a change to rename the directory, so the the README instructions actually work (no need name the symlink differently from the directory):

cd ~/.local/share/gnome-shell/extensions
ln -sv /path/to/gnome-shell-system-monitor-applet/system-monitor-next@paradoxxx.zero.gmail.com/

Regarding:

Extension “system-monitor-next@paradoxxx.zero.gmail.com” does not exist

It looks like Gnome Shell needs to be restarted for it to see the new symlink. I've updated the README accordingly.

@mgalgs
Copy link
Owner

mgalgs commented Oct 18, 2023

All right, I think we're down to the following unresolved issues:

  • Battery isn't working, so has been disabled.
  • Preferences page is still a mess, though now it's at least navigable.

I think I'd like to ship it as is and tackle these as separate issues (I don't have any more time to spare on this this week). Please vote with a 👍 or 👎 reaction on this comment.

👍 = ship now
👎 = work out kinks

I'll check back later tonight and will push the release if that's what people prefer.

@mgalgs
Copy link
Owner

mgalgs commented Oct 18, 2023

oh yeah, one more issue from @badsmoke:

the big arrows:

I'm unable to reproduce this... What do your settings look like? (I don't mean what does your settings page look like -- I know it looks like trash 😅 -- but what settings do you have changed from their defaults?)

@mgalgs mgalgs mentioned this issue Oct 19, 2023
@mgalgs
Copy link
Owner

mgalgs commented Oct 19, 2023

Submitted to e.g.o for review. Thanks, everyone, for your help on this!

mgalgs-bot pushed a commit that referenced this issue Oct 19, 2023
Fixes #38

High-level summary of changes:

  - Migrated to es6 style imports [1]
  - Removed support for GS<45 [2]
  - Added default class exports in lieu of enable/disable functions [3]
  - Moved to subclassing Extension for main extension, with its associated
    helper properties/methods in lieu of extensionUtils [4]
  - Moved to subclassing ExtensionPreferences for prefs, with its
    associated helper properties/methods in lieu of extensionUtils [5]
  - Removed legacy Class syntax (Lang) [6]
  - Migrated to GLib-based Mainloop [7]
  - Use gettext from the extension module [8]
  - Migrated logging to standard console.* methods [9]
  - Migrated preferences dialog to Adwaita [10]

TODO:

  - [ ] Figure out Battery (Main.panel.statusArea.quickSettings._system)

Suggested-by: Gil Vicente Bernardo Pinto <gilvbp@gmail.com>

 [1] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#esm
 [2] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#shell-version
 [3] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extension-js
 [4] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [5] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#prefs-js
 [6] https://gjs.guide/guides/gjs/legacy-class-syntax.html#binding
 [7] https://discourse.gnome.org/t/how-to-import-mainloop-on-gnome-45-version/17605
 [8] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#extensionutils
 [9] https://gjs.guide/extensions/upgrading/gnome-shell-45.html#logging
[10] https://gjs.guide/extensions/development/preferences.html#prefs-js
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 a pull request may close this issue.