Review of "Force Quit" version 47

Details Page Preview

Adds a force quit button. Click the toolbar button, then choose the window you want to force quit. On accidental click, right click anywhere or press [Esc] to abort the kill.

Extension Homepage
https://github.com/meghprkh/force-quit/

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) warning 3

Shexli found 3 issues that may need reviewer attention.

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

  • selection.js:67
            this._areaResolution = new St.Label({
                style_class: 'area-resolution',
                text: '',
            })
  • selection.js:56
            this._areaSelection = new St.Widget({
                name: 'area-selection',
                style_class: 'area-selection',
                visible: 'true',
                reactive: 'true',
                x: -10,
                y: -10,
            })

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

  • selection.js:67
            this._areaResolution = new St.Label({
                style_class: 'area-resolution',
                text: '',
            })
  • selection.js:56
            this._areaSelection = new St.Widget({
                name: 'area-selection',
                style_class: 'area-selection',
                visible: 'true',
                reactive: 'true',
                x: -10,
                y: -10,
            })

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

  • extension.js:39
            this._clickGesture.connect('recognize', () => {
                this._selection = new Selection.SelectionWindow();
            })
  • selection.js:79
                this._signalCapturedEvent = this._areaSelection.connect(
                    'captured-event',
                    this._onCaptureEvent.bind(this)
                )

All Versions

Previous Reviews on this Version

megh posted a review
The previous method did not even exist. Subclassing SignalEmitter would hopefully call emit stop on destruction which will do the cleanup
fmuellner posted a review
The 'stop' signal is emitted on a button-press event, so no, that won't happen on destruction. It also doesn't look like anything connects to the 'stop' signal? The easiest option would really to add a cleanup method (like "destroy()") to SelectionWindow that at the very least calls `this._capture._stop()`, then make sure that both the extension and button classes call it on their respective "this._selection" objects on disable/destroy.
JustPerfection active