ddterm / gnome-shell-extension-ddterm

Another drop down terminal extension for GNOME Shell. With tabs. Works on Wayland natively
https://extensions.gnome.org/extension/3780/ddterm/
GNU General Public License v3.0
289 stars 26 forks source link

Fix GLib warning #981

Open C-512L opened 2 days ago

amezin commented 2 days ago

Your change isn't compatible with GNOME versions older than 46. ddterm master branch currently supports GNOME >= 43. Between supporting more GNOME versions, and getting rid of a warning, I will obviously choose compatibility.

But you likely can make it work for all GNOME versions using https://gjs-docs.gnome.org/gjs/overrides.md#gi-require

C-512L commented 2 days ago

I have been able to backport it to all systems using ESM on my own branch, but I might have stumbled on a ESM translator edge case being unable to translate import Gi from 'gi'; correctly. I tried to look at that code of translate-esm.js but unfortunalety I haven't found much documentation about using gi.require() on pre-ESM GJS to make a patch by myself.

amezin commented 2 days ago

There's also sed preprocessing step to solve issues like that: https://github.com/ddterm/gnome-shell-extension-ddterm/blob/bfcdf692fe2e1e907556aa6243ebc5d50de3fa31/ddterm/shell/notifications.js#L143-L151 (although here it's used just to remove unnecessary if from legacy version)

You should be able to use legacy imports between // BEGIN !ESM and // END !ESM lines

C-512L commented 2 days ago

I didn't feel sure about adding a lint exception, but I guessed it would be a better solution than adding dead code to the non-ESM build or duplicating the preprocessed code. Feedback is welcomed.