From 63a003e4a5587ae3f609a86a1716268e3ec2a6b4 Mon Sep 17 00:00:00 2001 From: Alan Silva Date: Sat, 5 Sep 2026 15:43:33 +0100 Subject: [PATCH] Fix review findings: dual-entry image capture, image blob GC at limit, stdin drain, fuzzy query edge cases, uri encoding, GC queue, temp-file pruning, theme border widths --- install.sh | 2 +- plugin/Clipboard.qml | 33 +++++++++++++-------- plugin/Fuzzy.js | 15 +++------- plugin/PreviewPane.qml | 6 ++-- plugin/Store.js | 12 ++++---- plugin/__pycache__/capture.cpython-314.pyc | Bin 0 -> 9426 bytes plugin/capture.py | 9 +++++- plugin/open-entry.sh | 4 ++- plugin/paste-entry.sh | 2 +- tests/test-fuzzy.mjs | 10 ++++++- tests/test-store.mjs | 21 ++++++++++--- 11 files changed, 73 insertions(+), 41 deletions(-) create mode 100644 plugin/__pycache__/capture.cpython-314.pyc diff --git a/install.sh b/install.sh index 4fd8fcd..cb4bc00 100755 --- a/install.sh +++ b/install.sh @@ -37,7 +37,7 @@ echo "==> Enabling $PLUGIN_ID (replaces built-in omarchy.clipboard)" omarchy plugin enable "$PLUGIN_ID" echo "==> Rebinding ALT+SHIFT+V" -if [[ -f $BINDINGS ]] && grep -q "ALT SHIFT, V" "$BINDINGS"; then +if [[ -f $BINDINGS ]] && grep -q "^bindd = ALT SHIFT, V, " "$BINDINGS"; then cp "$BINDINGS" "$BINDINGS.bak.$(date +%s)" sed -i 's|^bindd = ALT SHIFT, V, .*|bindd = ALT SHIFT, V, Clipboard manager (clipboard-history), exec, omarchy-shell shell toggle tank.clipboard|' "$BINDINGS" hyprctl reload >/dev/null diff --git a/plugin/Clipboard.qml b/plugin/Clipboard.qml index 51472cb..84a7ffd 100644 --- a/plugin/Clipboard.qml +++ b/plugin/Clipboard.qml @@ -88,18 +88,27 @@ Item { var pruned = Store.prune(root.history, root.historyLimit) root.history = pruned.entries historyFile.setText(JSON.stringify(root.history, null, 1) + "\n") - if (pruned.droppedImagePaths.length > 0) { - var cmd = ["rm", "-f"].concat(pruned.droppedImagePaths) - gcProc.command = cmd - gcProc.running = true - } + if (pruned.droppedImagePaths.length > 0) queueGc(pruned.droppedImagePaths) + } + + // Serialize GC batches: a single reusable Process would silently drop + // overlapping runs, so pending paths queue until the current rm exits. + property var gcQueue: [] + function queueGc(paths) { + root.gcQueue.push(paths) + if (!gcProc.running) runNextGc() + } + function runNextGc() { + if (root.gcQueue.length === 0) return + gcProc.command = ["rm", "-f"].concat(root.gcQueue.shift()) + gcProc.running = true } function addClipboardJson(line) { var entry = null try { entry = JSON.parse(String(line || "")) } catch (e) { return } if (!entry) return - root.history = Store.addEntry(root.history, entry, root.historyLimit, Math.floor(Date.now() / 1000)) + root.history = Store.addEntry(root.history, entry, Math.floor(Date.now() / 1000)) root.saveHistory() if (root.opened) root.rebuild() } @@ -242,10 +251,7 @@ Item { root.history = [] root.typeCache = {} root.saveHistory() - if (dropped.length > 0) { - gcProc.command = ["rm", "-f"].concat(dropped) - gcProc.running = true - } + if (dropped.length > 0) queueGc(dropped) root.selectedIndex = 0 root.clearConfirmOpen = false root.rebuild() @@ -271,7 +277,10 @@ Item { onFileChanged: reload() } - Process { id: gcProc } + Process { + id: gcProc + onExited: runNextGc() + } // Reap watchers left behind by a previous shell instance, then start our // own. pdeathsig kills them whenever the shell exits. @@ -674,7 +683,7 @@ Item { anchors.bottom: parent.bottom anchors.left: parent.left anchors.leftMargin: root.listWidth + Style.space(6) - width: 1 + width: Style.normalBorderWidth color: root.lineColor } diff --git a/plugin/Fuzzy.js b/plugin/Fuzzy.js index fcbebc5..76dedd4 100644 --- a/plugin/Fuzzy.js +++ b/plugin/Fuzzy.js @@ -65,10 +65,11 @@ function parseQuery(q) { var canonical = TYPE_ALIASES[wanted] if (canonical) { out.type = canonical; continue } // prefix match: "type:im" → image + var matchedType = "" for (var key in TYPE_ALIASES) { - if (key.indexOf(wanted) === 0) { out.type = TYPE_ALIASES[key]; break } + if (key.indexOf(wanted) === 0) { matchedType = TYPE_ALIASES[key]; break } } - if (out.type) continue + if (matchedType) { out.type = matchedType; continue } // unknown type — treat the whole token as a term out.terms.push(tok) continue @@ -91,7 +92,7 @@ function parseQuery(q) { if (AGE_WORDS[lower] !== undefined) { out.maxAge = AGE_WORDS[lower] - if (lower === "yesterday") out.minAge = 0 // refined below relative to now; caller passes now + if (lower === "yesterday") out.minAge = 86400 continue } @@ -219,14 +220,6 @@ function ageFilterOk(parsed, ageSeconds) { return true } -// "yesterday" means older than 24h but within 48h. -function refineYesterday(parsed, now) { - // Only applies when the query contained the bare word "yesterday" and no - // explicit duration bounds; approximated by caller passing minAge via the - // maxAge=172800 already set. We require age > 86400 - slack handled here. - return parsed -} - function recencyBonus(ts, now) { if (!ts) return 0 var ageDays = Math.max(0, (now - ts) / 86400) diff --git a/plugin/PreviewPane.qml b/plugin/PreviewPane.qml index 6f04711..9322beb 100644 --- a/plugin/PreviewPane.qml +++ b/plugin/PreviewPane.qml @@ -163,7 +163,7 @@ Item { anchors.left: parent.left anchors.right: parent.right anchors.topMargin: Style.space(10) - height: 1 + height: Style.normalBorderWidth color: root.lineColor } @@ -224,7 +224,7 @@ Item { height: Style.space(120) radius: Style.cornerRadius color: root.hexRe.test(root.rawSafe()) ? root.rawSafe() : "transparent" - border.width: 1 + border.width: Style.normalBorderWidth border.color: root.lineColor } @@ -322,7 +322,7 @@ Item { Rectangle { anchors.fill: img color: "transparent" - border.width: 1 + border.width: Style.normalBorderWidth border.color: root.lineColor radius: Style.space(4) visible: img.status === Image.Ready diff --git a/plugin/Store.js b/plugin/Store.js index a2a6f1b..8fd553e 100644 --- a/plugin/Store.js +++ b/plugin/Store.js @@ -86,17 +86,17 @@ function parseHistory(raw, now) { } // Add (or bump) an entry; newest first. Keeps pin state and usage count of -// the existing copy when the same content is copied again. -function addEntry(history, entry, limit, now) { +// the existing copy when the same content is copied again. Does NOT truncate +// to the limit: the caller prunes via prune() so evicted image blobs can be +// garbage-collected (truncating here would leak them). +function addEntry(history, entry, now) { var normalized = normalize(entry, now) - var max = limit === undefined || limit === null ? DEFAULT_LIMIT : Number(limit) - if (isNaN(max) || max < 1) max = DEFAULT_LIMIT - if (!normalized) return Array.isArray(history) ? history.slice(0, max) : [] + if (!normalized) return Array.isArray(history) ? history.slice() : [] var key = entryKey(normalized) var next = [normalized] var values = Array.isArray(history) ? history : [] - for (var i = 0; i < values.length && next.length < max; i++) { + for (var i = 0; i < values.length; i++) { var existing = normalize(values[i], now) if (!existing) continue if (entryKey(existing) === key) { diff --git a/plugin/__pycache__/capture.cpython-314.pyc b/plugin/__pycache__/capture.cpython-314.pyc new file mode 100644 index 0000000000000000000000000000000000000000..101baf187b0c8cfea9c2f60c86f747031aafd218 GIT binary patch literal 9426 zcmc&)U2qdumhM)!ep|9E$v+tExUmgHV9SB<<6uI@;2(^^7L5@kV{l}tZ3|>cbhl#c z-AP0?yO<<3b~4N&5~g4>sa;c(t?|~()==}ZkjxCJ+Nv!Zd1BLeD^r=gQVITIobnfl@-}l^e&Ua3GQ-Q-qK&sk!`Q!)11o0Pq(2_k{c<@goL8J(d zkcblmN4Ap^spJO9pyZT9DLF0CN^X>lN^X)&N^X|SN^X&?$0>i2HhnJlm*&b^33{&5 z%DLRXCRf%ZXPR^*0EA$auc^4_UI1^{* z%$$R>a0Q%|b8_l?HiM3dTg2+c-;YC)Avq4sj#0H`~t4zZSY$*N;FNiD;bc7v>4DntTb~zl82H6;N7Y0OD zinC%Ym=Fi!k`1>XN(urWlUV%?d~C~5M1o1q1Vd7E1h&Mp`ww?@ve5{X61a+b0y}Ls zmi?P5X(Yi{?W~gcGg4Kpn(g%Yd{7jt)I#5g#B1eXBB9j&D1U5ZC^*2wVBB$bOd==^ zW{bnwTzk%+PG4iidHW+#Sh%(dCxZ?A5Vm!^w(1GvZHM`o7?C2cz}Wy-MfT}4b*I8S zvP>L~3*m#o80=Qq7lA{oXZIdx_wVUy;+lI8cQl!aiH4TyU(pO`V~4U)RBw(Sld&ebIO*7_Aqjpv24OxO%Nd3w1u# zDW@hbT9v#$5gS;vsl}5CUM~;xeF?40K%`$Q_6;Sp-cqE$U#4KVOr1;&$TW_SDd;Ox z&`YMVJ(Lw~MLJACDfm4|14$8Uh#sXHi9f$zwstKM07VxdIdTV%f|M`>41QWI=hG8} zB8(0cM(Ba-EK4AghH8Q(dMsLli3l69ejZFRLw3*r^e-oTL^ag;aqP0h z89>QrIqQehUY<0iSx6iqlLjA=G_)2F#A(;k`8&Y)1;|cAM!O<`VPmQ74VBND175qM4^ z-)Q9t_sp`wYRF_8&eyBVCsad4E&Rj|ASq(O<`^4I@18E$n6Yh2?fsl7n0LCytn(G@ zm6tBPG-;kH%TzSZRkY4jw9Z!S%P@y#D)vou|3~w9^H?(!XP86N^r3HWGlxV5=KGzs z$wl9^n1F`hQBQbqc(@P#6G%Q|z>d*fV5DpZ zV*)C5I1&rTher){C%q<>iS$V^9+Qn|{Xh;yNdVo$O9CPUuR&OgW9e{E3X1v6WcuTw zq{xSRLG24Z=<_W6#3MiyZr(1azwVl~H7dNk<8-0yxfh(C@zFO&)75XiHbyTv-4k0c zHoVi2Zp(OT<~$pxJsT$t8Rw>H%O*|m+%k*=!E-N)xSRL4G!+v!3rt9hnWh@*W;F>k zqzf4I(Ruu_vXz|h<#A5uA#};69PZHghbnVDwD#!Y)}yOKzpnhW@dT02du@(NqDSYP zUqha`A%x$V_mC}WqO9T{kS2p9c6u!`1=bEt!4NMS69PO4UKk-8!^xopvTZ~ZN|cKJ zXi^;XlClv@n=m5c;jC8$kmdna2n|rN8-C&rAiykl@rCfk$h(oBoXWVXQY~XOXFC>X z>)SQs8{gbGaU^3~GfS5*(B^ad&+eb2i)QGe&*+i|FqBwglJGqh+3KbVBDh4l0>rZ< zM*u#wFn9i_1Oo<+_R-q;Ca3_l!iXpEDaRAe$eA>W2VUtM z@`LhRo`AnP&i1>Ow*vOgRwd}XdxDl~S@MHNvOMK>6rk+{XYN5xDk)&7MctWAc4CWe z1`y_Il=LnGX#)N4N`Zxvcj(r6QbEWF-Pp_`myi5<+~%-5-t5#2;9Xw=iL!v;Jm6;yQq zA_$OYh?KFC=m5Dz;KKpX+D1jlYh>p&;u<9ze9y?_kZe@2i3%&(l&UkWy9W3eea8!#6{UM z6a>FGB8ak8ZEtSp08IqN!DyrpaQI+w%eL*Zb&x+3jtuaC4-UY_W%^`164OSs9P5@T z@TFwR-+n;o!gfO>F4OS@)S367_u1Yh(_&`W;VGQVl#Ic-oRZC5hg$?8E+|IK>kyitQRqa1I8vxn`sVFYKpUs*M|c_A z43Qz(&>xm52^J#zMR0V8{`G7wsZ4ihPK$EjLIQdv+B6g7s?ipJ?xbqiFoZnc5d|9`+6a4rvWea2 zcc*LSn9AEs<$` zbLEFg^5vEny6?l8`8*1=k^oxgsm`d5xS7CXW` z$Kp437zWe*^`VIvlitiK_!5~NR;um)J zJqCI%imw8bKCy0TV~9^0OIjO@zjs-n{D<<&wtC_ZjqbK`;~yPQLHUmjOq<1ei=v_R zmW4vfFl}YjtyLt_a;B};dTWCPdfuvM_HCeUHJ0pi8$NZBQ2x}->|>3eRv55cNn)9$ zu)H2ym*^-nYzOod{osF~0I^16A<+ufKS8cRH!o{@ph-^wQqzzL(BB7XMOT3iAQLuWrmXbKC6kPWlzS3s|tkk1>At%Tl^puno(0vWf*3{~4V5#Yb z3rkl2A~Lo;{#B>iB+$*-0!$s#w)^Y-ClRUt~Km%!MI6 zHDXg2jmJ+xY)y~c_&Q~hSaQC&vSI@;{GnTbDm>vF?$4d+3AO>SMb;_+A21;D9B?69 z1B&K^W+!U0b^?4pv=d(HD;Aa*rECssCn{D5LV;`a0@TT5Nb!r-t4^-42kV|v->nzl za|JsACqsznlPPARtn269SILhKPq}^>nCiRs!^>;0AG$F(^L*!(wePL{MfbNN!qV5> zwd5tbmi)k0OAfqu3DdO~U<()y9yOtJzIi+NbCvKYyN|>o=#@7szPX|=WqYiq!0Z9>m zaaN5{;Obe6=RKtt8{cV6iy2S#So0mb>uWPnSp9VYQCRe6vty#@jaTnDJ)fJIg_5!v zJb+ADSEi(EjG1RjE_>34KBTTT|H7In+xRDD<7D?Nv-ysvbe?ghIvzbNC0x}X9+-A+ z1|?uEmhGqnti_8WLh^I2ruBxK>s?Ka#+y4ypews5DS${Ijv$1{{+v+dE)4_(Y+ZT6 zm=ibkvM7#87QDhm(gg?|lKz~KQcsB0(AikcUH~o4eqDs&O|nVDQ98s`=I#KOAV9Ae zQqXszjZojN1H=SIe9sCH6$1Ez3MXMq&;g@1KKH&pB4G-k3KKxU%-}re%0$~OC<`6x zYU9HY+=p1A$)BfEwEo}kaINHIJz<1Ln>m*U%$fpR=_8HSrsV(;Poq16{Kc~fn6&rE zokLHlHakb70|_pTInVjhIlx)K`O;lgnA57q+5qBunMDVRT76Q8D1o+CnXDc)BNWcED+fUSS^x(Fsl?4 zBoV(s$R+@ad@KxrN{Ax5gSp{rhHOs8PAB6K<+eb*Ql)@A1!k!_!jgxz5M~J|@iLi| zX?Ouqcmtm(EF#8*psbQqjZvtfDQND+7@#l?eQ><^-#}8tytyRJ&zh_6IEw&6-FCKS zm^KwUVMWP=lwryN`6Y{q}Tqo?O)q|+x~X@jo_{5 z&FJ*uz+CImx7){>#@o)f{kR>vq?)IhV~EjUw7nqJa@Xk@qvn}{afCVEIi}_|Q!{UO zjUPCF;Bv{8@=N7&Wpy)UbszQrDtawC=i58u+k0bg#_|4wy^sn2mCvSwNt@~$b1U=v0FC*+Pk|MgdH|f&m zM*1dOg!DP2D=R7ssHgwuScS)#0AjtO5?WblDJoM}0w_2jIy8t5VsItp zR``zSfH-=sOKd+XU&Z$SU;IB|7tA6&k7PHHC7z#Z$t$(H`xsOHk2nk!Le}Nmc4_~_ z>B;1^jf%^+|68FM+EiCh6<3s0MYdDO05jOUyxDZC zr0;|5(Sv|T_3)==Y4S$umgakVm1&#F^#W@H?*TAf4^-!eSPuMH52_eDN6Q}jaaDFM z`?e}z{7AOuzOXukZCC?_2ELffroLoh#mt%; zq3KIo@mqDNy$j}|i6e7n_O_W-`j@7SbLQ&XV4LuQ7fuv^MwdT&SV`E6@xx%waaG{w z>re05M0~u7+4CIraRUidw)FPm=6ibqo8yHSP&@?_5>%`LvPVE1#U zLxX6RrRSBeZlktbi_9G9Zf$_?)LeUKn{3MFG%5|*vA64>zw1b6b1y!6$**uR?^Ck5 zw>KOQ_4W!c;%Z(b$PoeWrNTdf#}dXLWxI+Cd@2SQ!+MPN2t7#NM}q8=Z4eQIkL&OO zNRX|{XZKzR8VPXk3E>Em<4AsjqzFkh5(pG#l^6z8RK6I`^K9p|r}`>~age!nJ7(7Hm~PmYS=XMS_kY^{fHF7?DeFDBMNVyC zx@@{;d&a&aW&NU{H^t05R;AZnEuTzIMQ^mK4{Ij*sh%6#F(T1HLN`YwWqW8cl129m zY49`e7g$L5{St~Sy>G7~3-9kIT|l2M2J(yz{`=_u>SD6u-ZKPk9~+&e%I?sXe`x)h KBIwnM;{0E>3vLhq literal 0 HcmV?d00001 diff --git a/plugin/capture.py b/plugin/capture.py index 535f144..f9a7d78 100755 --- a/plugin/capture.py +++ b/plugin/capture.py @@ -99,6 +99,7 @@ def capture_image(types, app): pass return emit(entry) + return True def decode_text(data): @@ -159,7 +160,13 @@ def capture_text(types, app): def main(): - sys.stdin.close() # watcher payload unused; we probe ourselves + # The watcher pipes the clipboard payload to us; wl-clipboard blocks on + # writes if we close the pipe early, so drain it instead (we still probe + # types ourselves below). + try: + sys.stdin.buffer.read() + except Exception: + pass types = list_types() if not types: return diff --git a/plugin/open-entry.sh b/plugin/open-entry.sh index fce3e91..0d49cf2 100755 --- a/plugin/open-entry.sh +++ b/plugin/open-entry.sh @@ -28,6 +28,8 @@ open_text() { local dir file dir="${XDG_STATE_HOME:-$HOME/.local/state}/omarchy/clipboard-open" mkdir -p "$dir" + # Prune temp copies older than a week; nothing else reclaims them. + find "$dir" -type f -name 'clipboard.*.txt' -mtime +7 -delete 2>/dev/null || true file=$(mktemp --tmpdir="$dir" clipboard.XXXXXX.txt) printf '%s' "$text" >"$file" exec omarchy-launch-editor "$file" @@ -43,7 +45,7 @@ case $(jq -r '.type' <<<"$entry") in exec xdg-open "$path" ;; files) - first=$(jq -r '.paths[0]' <<<"$entry") + first=$(jq -r '.paths[0] // empty' <<<"$entry") [[ -n $first ]] || exit 0 exec xdg-open "$first" ;; diff --git a/plugin/paste-entry.sh b/plugin/paste-entry.sh index e5ed995..a96f227 100755 --- a/plugin/paste-entry.sh +++ b/plugin/paste-entry.sh @@ -23,7 +23,7 @@ case "$type" in ;; files) while IFS= read -r p; do - printf 'file://%s\n' "$p" + printf 'file://%s\n' "$(python3 -c 'import sys, urllib.parse; print(urllib.parse.quote(sys.argv[1]))' "$p")" done < <(jq -r '.paths[]' <<<"$entry") | wl-copy --type text/uri-list ;; *) diff --git a/tests/test-fuzzy.mjs b/tests/test-fuzzy.mjs index 3068371..cb0a5b4 100644 --- a/tests/test-fuzzy.mjs +++ b/tests/test-fuzzy.mjs @@ -156,7 +156,15 @@ test("highlightFirstLine multiline only first line", () => { assert.ok(!html.includes("\n")) }) -test("parseQuery yesterday bounds", () => { +test("parseQuery yesterday excludes today", () => { const q = Fuzzy.parseQuery("yesterday") assert.equal(q.maxAge, 172800) + assert.equal(q.minAge, 86400) +}) + +test("regression: unknown type: token becomes a term even after a known one", () => { + const q = Fuzzy.parseQuery("type:image type:foo") + assert.equal(q.type, "image") + assert.equal(q.terms.length, 1) + assert.equal(q.terms[0], "type:foo") }) diff --git a/tests/test-store.mjs b/tests/test-store.mjs index 9ae4f0a..3d014a5 100644 --- a/tests/test-store.mjs +++ b/tests/test-store.mjs @@ -46,11 +46,24 @@ test("addEntry dedupes and bumps to front, keeps pin/uses", () => { assert.equal(h[0].uses, 3) }) -test("addEntry respects limit", () => { +test("addEntry keeps all entries; prune truncates", () => { let h = [] - for (let i = 0; i < 20; i++) h = Store.addEntry(h, { type: "text", text: "t" + i, ts: i }, 5) - assert.equal(h.length, 5) - assert.equal(h[0].text, "t19") + for (let i = 0; i < 20; i++) h = Store.addEntry(h, { type: "text", text: "t" + i, ts: i }) + assert.equal(h.length, 20) + const r = Store.prune(h, 5) + assert.equal(r.entries.length, 5) + assert.equal(r.entries[0].text, "t19") +}) + +test("regression: addEntry at limit must not leak evicted images (prune reports them)", () => { + // Images beyond the limit are only reclaimable if prune() sees them, + // which is why addEntry must not truncate by itself. + let h = [] + for (let i = 0; i < 10; i++) + h = Store.addEntry(h, { type: i < 2 ? "image" : "text", path: "/tmp/x" + i + ".png", text: "t" + i, ts: i }) + const r = Store.prune(h, 5) + assert.ok(r.droppedImagePaths.includes("/tmp/x0.png")) + assert.ok(r.droppedImagePaths.includes("/tmp/x1.png")) }) test("removeById / findById / togglePin / touch", () => {