Review of "Tiling Shell" version 17.3 (76)

Details Page Preview

Extend Gnome Shell with advanced tiling window management. Supports multiple monitors, Windows 11 Snap Assistant, Fancy Zones, automatic tiling, keyboard shortcuts, customised tiling layouts and more!

Extension Homepage
https://github.com/domferr/tilingshell

No comments.

Diff Against

Files

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

Shexli (experimental) error 1 warning 4

Shexli found 5 issues that may need reviewer attention.

EGO-C49-003 error

extensions targeting GNOME 49 must not call maximize or unmaximize with Meta.MaximizeFlags

This extension explicitly targets GNOME Shell 49 but still passes `Meta.MaximizeFlags` to `maximize()` or `unmaximize()`.

Meta.Window

  • utils/gnomesupport.js:29
    window.maximize(Meta.MaximizeFlags.BOTH)
  • utils/gnomesupport.js:33
    window.unmaximize(Meta.MaximizeFlags.BOTH)

EGO-L-002 warning

objects created by extension should be destroyed in disable()

Objects assigned in `enable()` are missing matching `.destroy()` calls in `disable()` or its helper methods.

Destroy all objects

  • components/altTab/MultipleWindowsIcon.js:41
        this._label = new St.Label({
          text: _("Tiled windows")
        })
  • components/altTab/tilePreviewWithWindow.js:12
    this._gaps = new Clutter.Margin()
  • components/editor/editorDialog.js:42
        this._layoutsBoxLayout = new St.BoxLayout({
          styleClass: "layouts-box-layout",
          xAlign: Clutter.ActorAlign.CENTER
        })
  • components/windowsSuggestions/suggestionsTilePreview.js:30
    this.layout_manager = new Clutter.BinLayout()
  • indicator/defaultMenu.js:134
        this._container = new St.BoxLayout({
          xAlign: Clutter.ActorAlign.CENTER,
          yAlign: Clutter.ActorAlign.CENTER,
          xExpand: true,
          yExpand: true,
          styleClass: "default-menu-container",
          ...widgetOrientation(true)
        })

EGO-L-005 warning

owned object references should be released in disable()

Owned references that are cleaned up in `disable()` should also be released with `null` or `undefined`.

Destroy all objects

  • components/altTab/MultipleWindowsIcon.js:41
        this._label = new St.Label({
          text: _("Tiled windows")
        })
  • components/altTab/tilePreviewWithWindow.js:12
    this._gaps = new Clutter.Margin()
  • components/editor/editorDialog.js:42
        this._layoutsBoxLayout = new St.BoxLayout({
          styleClass: "layouts-box-layout",
          xAlign: Clutter.ActorAlign.CENTER
        })
  • components/windowBorder/windowBorderManager.js:15
        this._interfaceSettings = new Gio.Settings({
          schema_id: "org.gnome.desktop.interface"
        })
  • components/windowsSuggestions/suggestionsTilePreview.js:30
    this.layout_manager = new Clutter.BinLayout()
  • indicator/defaultMenu.js:134
        this._container = new St.BoxLayout({
          xAlign: Clutter.ActorAlign.CENTER,
          yAlign: Clutter.ActorAlign.CENTER,
          xExpand: true,
          yExpand: true,
          styleClass: "default-menu-container",
          ...widgetOrientation(true)
        })

EGO-L-003 warning

signals connected by extension should be disconnected in disable()

Signals assigned in `enable()` are missing matching disconnect calls in `disable()` or its helper methods.

Disconnect all signals

  • components/editor/editorDialog.js:284
          btn.connect("clicked", () => {
            params.onSelectLayout(btnInd, lay);
            this._makeLegendDialog({
              onClose: params.onClose,
              path: params.path
            });
          })
  • components/editor/editorDialog.js:317
        newLayoutBtn.connect("clicked", () => {
          params.onNewLayout();
          this._makeLegendDialog({
            onClose: params.onClose,
            path: params.path
          });
        })
  • components/editor/layoutEditor.js:158
        editableTile.connect("clicked", (_, clicked_button) => {
          if (clicked_button === St.ButtonMask.ONE)
            this.splitTile(editableTile);
          else if (clicked_button === 3) this.deleteTile(editableTile);
        })
  • components/editor/layoutEditor.js:163
        editableTile.connect("motion-event", (_, event) => {
          const [stageX, stageY] = getEventCoords(event);
          this._hoverWidget.handleMouseMove(
            editableTile,
            stageX - this.x,
            stageY - this.y
          );
          return Clutter.EVENT_PROPAGATE;
        })
  • components/editor/layoutEditor.js:172
        editableTile.connect("notify::hover", () => {
          const [stageX, stageY] = Shell.Global.get().get_pointer();
          this._hoverWidget.handleMouseMove(
            editableTile,
            stageX - this.x,
            stageY - this.y
          );
        })
  • components/raiseTogether/raiseTogetherManager.js:54
        window.connect("unmanaged", () => {
          delete this._raiseId[window.get_id()];
        })
  • components/snapassist/snapAssist.js:69
        this._signals.connect(
          St.ThemeContext.get_for_stage(global.get_stage()),
          "changed",
          () => {
            this._applyStyle();
          }
        )
  • components/windowBorder/windowBorder.js:100
        this._signals.connect(global.display, "restacked", () => {
          this.queue_repaint();
          global.windowGroup.set_child_above_sibling(this, null);
        })
  • components/windowBorder/windowBorderManager.js:49
        this._interfaceSettings.connect(
          "changed::accent-color",
          () => this._border?.updateStyle()
        )
  • components/windowManager/tilingShellWindowManager.js:48
        this._signals.connect(
          global.display,
          "window-created",
          (_, window) => {
            window.__ts_cached = new CachedWindowProperties(window, this);
          }
        )
  • indicator/defaultMenu.js:62
          btn.connect(
            "clicked",
            () => !btn.checked && this.emit("selected-layout", lay.id)
          )
  • indicator/defaultMenu.js:349
          row.connect(
            "selected-layout",
            (r, layoutId) => {
              this._indicator.selectLayoutOnClick(
                monitor.index,
                layoutId
              );
            }
          )
  • keybindings.js:29
        this._signals.connect(
          Settings,
          Settings.KEY_ENABLE_MOVE_KEYBINDINGS,
          () => {
            this._setupKeyBindings(extensionSettings);
          }
        )
  • utils/globalState.js:46
        this._signals.connect(
          Settings,
          Settings.KEY_SETTING_LAYOUTS_JSON,
          () => {
            this._layouts = Settings.get_layouts_json();
            this.emit(_GlobalState.SIGNAL_LAYOUTS_CHANGED);
          }
        )

EGO-L-004 warning

main loop sources should be removed in disable()

Main loop sources assigned in `enable()` are missing matching removals in `disable()` or its helper methods.

Remove main loop sources

  • components/windowBorder/windowBorder.js:161
        this._timeout = setTimeout(() => {
          this._computeBorderRadius(winActor).then(() => this.updateStyle());
          if (this._timeout) clearTimeout(this._timeout);
          this._timeout = void 0;
        }, SMART_BORDER_RADIUS_FIRST_FRAME_DELAY)

All Versions

Version Status
17.3 (76) Active
17.3 (75) Rejected
17.3 (74) Rejected
17.3 (73) Rejected
17.3 (72) Active
17.3 (71) Active
17.3 (70) Rejected
17.3 (69) Inactive
17.3 (68) Inactive
17.2 (67) Active
17.2 (66) Active
17.2 (65) Rejected
17.2 (64) Rejected
17.2 (63) Rejected
17.1 (62) Active
17.1 (61) Active
17.1 (60) Rejected
17.0 (59) Active
17.0 (58) Active
17.0 (57) Inactive
17.0 (56) Rejected
16.4 (55) Active
16.4 (54) Active
16.4 (53) Rejected
16.3 (52) Inactive
16.3 (51) Active
16.2 (50) Active
16.2 (49) Active
16.1 (48) Active
16.1 (47) Active
16.0 (46) Inactive
16.0 (45) Inactive
16.0 (44) Inactive
15.1 (43) Active
15.1 (42) Active
15.1 (41) Rejected
15.0 (40) Active
15.0 (39) Active
15.0 (38) Rejected
14.1 (37) Active
14.1 (36) Active
14.1 (35) Rejected
14 (34) Active
14 (33) Active
14 (32) Inactive
14 (31) Rejected
13.1 (30) Active
13.1 (29) Active
13.0 (28) Active
13.0 (27) Active
12.2 (26) Active
12.2 (25) Active
12.1 (24) Active
12.1 (23) Active
12 (22) Active
12 (21) Active
12 (20) Inactive
12 (19) Inactive
11.1 (18) Active
11.1 (17) Active
11 (16) Active
11 (15) Active
11 (14) Rejected
10.0 (13) Active
10.0 (12) Active
9.1 (11) Active
9.1 (10) Active
9.0 (9) Active
9.0 (8) Active
8.0 (7) Active
8.0 (6) Active
5 Active
4 Active
3 Rejected
2 Inactive
1 Rejected

Previous Reviews on this Version

domferr posted a review
Hi. The diff you see in the UI is not correct: it is comparing the files for GNOME 45+ with the ones for GNOME <= 44, which of course have a lot of differences mainly due to the old import system. The best diff is shown when comparing the current update with the latest approved GNOME 45+ version (it is version 71). The right comparison reveals that the diffs is only about 4 files removed and 5 files edited in few lines. I also want to note that the shexli reports are false positive, as previously discussed, and they were reported here https://gitlab.gnome.org/Infrastructure/extensions-web/-/issues/367. This update is mainly focusing on GNOME 50 support and addressing previous shexli reported warns. Thanks!
dlandau active
Thanks for checking the shexli messages and for reporting them to the proper place! We'll need more tooling for false positives, but for now please keep commenting here about it on future updates too.