Review of "ddterm" version 43

Details Page Preview

Another drop down terminal extension for GNOME Shell. With tabs. Works on Wayland natively

Extension Homepage
https://github.com/ddterm/gnome-shell-extension-ddterm

No comments.

Diff Against

Files

Note: Binary files aren't shown on the web site. To see all files, please download the extension zipfile.

All Versions

Previous Reviews on this Version

JustPerfection waiting for author
Where do you use the application.js file? I'm asking that because you cannot import gtk and gdk in extension.js. and the line 307 (application.js) cannot create objects in global scope (if that's getting imported in extension.js).
amezin posted a review
application.js is a separate gjs process. Launched by spawn_app() in extension.js It's been this way since the 1st version.
JustPerfection waiting for author
So you should start that file with this: ```js #!/usr/bin/env gjs ``` and if that's a separate application why do you import `misc`to use `getCurrentExtension` (line 24 application.js)? why not getting `Me.path` value from argv?
amezin posted a review
Well.. Actually, subprocess's entry point is `com.github.amezin.ddterm` script. That one starts with `#!/usr/bin/env gjs` shebang, and imports `application.js`. It also sets up `Me.path`. If you look at `misc.js`, you'll see that it's a tiny "fake" getCurrentExtension() implementation, for the cases where the code is actually not running in extension context. Why do I need the trick with fake `misc.getCurrentExtension()`? Because I want to reuse some code in both application/subprocess and extension. Primarily, it's Preferences dialog, which can be used both by the extension API and loaded by the application directly.
JustPerfection active
Okay. Maybe moving those app related files to app sub folder can make it easier for the reviewers but you can also leave it like that.
JustPerfection posted a review
We have a fork of this extension posted for review: https://extensions.gnome.org/review/44843 Seems they just added 44 and they mentioned this in the description: > I have no plans on add/removing features..just thought others may like it.. Are you going to move to 44?
amezin posted a review
That seems to be just my "master" branch (but a bit outdated). I stuck in an infinite "fix one small thing before release" (initially - because removal of rxjs turned to almost full rewrite, and I didn't want to upload code that will be completely replaced later again). I'm going to upload a release today/tonight.
JustPerfection posted a review
Ok. Thank you for the clarification.