Review of "Clipboard History" version 1

Details Page Preview

Keeps a searchable history of what you copy, and pastes an entry back where you were typing when you click it. Screenshots and other images are listed too, with a thumbnail. * Reads the clipboard to record new copies, and to paste the entry you click * Writes to the clipboard only when you click an entry * Clicking an entry sends a Ctrl+V keystroke to the window you were typing in * Stores history and images under ~/.local/share/clipboard-manager/ and never accesses the network Pairs with the optional clipman GTK app, which reads the same history file.

Extension Homepage
https://github.com/Gabrial-8467/clipboard-manager

No comments.

FAQ

Files

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

Shexli (experimental) warning 6 manual_review 1

Shexli found 7 issues that may need reviewer attention.

EGO-X-004 warning

extensions should avoid synchronous file IO in shell code

Shell code should avoid synchronous file IO APIs like `GLib.file_get_contents()` and `Gio.File.load_contents()`.

File Operations

  • extension.js:38
    GLib.file_get_contents(path)

EGO-A-005 manual_review

extensions should not access the clipboard directly

Direct clipboard access via `St.Clipboard.get_default()` requires reviewer scrutiny.

Review Guidelines

  • extension.js:59
    St.Clipboard.get_default()

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

  • extension.js:280
    this._clearBtn = clearBtn
  • extension.js:306
    this._historySection = new PopupMenu.PopupMenuSection()
  • extension.js:264
            this._search = new St.Entry({
                hint_text: 'Search history…',
                can_focus: true,
                style_class: 'clipboard-history-search',
            })

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

  • extension.js:280
    this._clearBtn = clearBtn
  • extension.js:306
    this._historySection = new PopupMenu.PopupMenuSection()
  • extension.js:264
            this._search = new St.Entry({
                hint_text: 'Search history…',
                can_focus: true,
                style_class: 'clipboard-history-search',
            })

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:287
            menu.connect('open-state-changed', (m, open) => {
                this._menuOpen = open;
                if (open) {
                    this._reload();
                    this._search.text = '';
                    GLib.idle_add(GLib.PRIORITY_DEFAULT, () => {
                        this._search.grab_key_focus();
  • extension.js:269
            this._search.clutter_text.connect('text-changed', () =>
                this._rebuildDebounced())

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

  • extension.js:135
            GLib.timeout_add(GLib.PRIORITY_DEFAULT, PASTE_DELAY_MS, () => {
                try {
                    this._injectPaste([KEY_CTRL], KEY_V);
                } catch (e) {
                    log(`clipboard-history: paste injection failed: ${e}`);
                }
                return GLib.SOURCE_REMOVE;
         
  • extension.js:292
                    GLib.idle_add(GLib.PRIORITY_DEFAULT, () => {
                        this._search.grab_key_focus();
                        return GLib.SOURCE_REMOVE;
                    })

EGO-C45-001 warning

45+ preferences should use fillPreferencesWindow instead of getPreferencesWidget

45+ preferences code should use `fillPreferencesWindow()` instead of `getPreferencesWidget()`.

Preferences

  • prefs.js:8
        getPreferencesWidget() {
            const settings = this.getSettings();
    
            const page = new Adw.PreferencesPage();
            const group = new Adw.PreferencesGroup({
                title: 'History',
                description: 'Copies are stored in ~/.local/share/clipboard-manager/history.jsonl',
      

All Versions

Version Status
2.1 (2) Unreviewed
1 Rejected

Previous Reviews on this Version

gabrial posted a review
gabrial posted a review
this is okk
gabrial posted a review
gabrial auto- rejected
Auto-rejected because of new version 2.1 (2) was uploaded