Skip to content

Commit 07c141d

Browse files
brianckeeganclaudemax3925vats
authored
Confirm before destructive Re-convert action (#20) (#26)
Add a Services.prompt.confirmEx dialog before the Re-convert (replace) menu action runs. Default opt-out via "Don't ask again" checkbox stored in the new `confirmReconvert` pref. Rationale: Re-convert silently deletes the existing markdown attachment and starts fresh. An accidental click is cheap for users running the standard pipeline locally (seconds of compute) but expensive for users on long VLM batches or paired with a remote LLM API (hours of compute, possibly substantial spend). The plugin's other destructive action (Reset preferences) already prompts; this brings Re-convert in line. Implementation notes: - Confirm fires inside runBatch after the filter that drops PDFs with no matching .md, so the count in the dialog ("on N selected PDFs") reflects the actual destructive count. - Confirm fires before setting `batchInFlight`, so cancelling is a clean no-op with no flag to clear. - Button-flag constants fall back to literal values if the Components.interfaces.nsIPromptService constants are absent (Z9 exposes them but defending against future runtime changes is cheap). - "Don't ask again" persistence uses setPref; Reset to defaults already iterates ALL_PREF_KEYS so the new pref restores there. Closes #20 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Mayank Vats <18255879+max3925vats@users.noreply.github.com>
1 parent 820bfe8 commit 07c141d

4 files changed

Lines changed: 93 additions & 1 deletion

File tree

addon/prefs.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ pref("exportFolderPath", "");
4747
// OS-level notification when a batch finishes, only if Zotero isn't focused.
4848
pref("notifyOnComplete", false);
4949

50+
// Show a confirmation prompt before the destructive "Re-convert (replace)"
51+
// menu action. Users can opt out via a "Don't ask again" checkbox in the
52+
// dialog itself; Reset to defaults restores this.
53+
pref("confirmReconvert", true);
5054
// First-run onboarding nudge. Flipped to true after the first startup
5155
// toast fires OR after the first successful Test Connection. Reset to
5256
// defaults re-arms the nudge.

src/modules/menu.ts

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import {
3333
removeMatchingMdChild,
3434
findMatchingMdChild,
3535
} from "../utils/zotero";
36-
import { getPref } from "../utils/prefs";
36+
import { getPref, setPref } from "../utils/prefs";
3737
import { notifyOnBatchComplete } from "../utils/notification";
3838
import { ConcurrencyLimiter } from "../utils/concurrencyLimiter";
3939
import { truncateMiddle } from "../utils/format";
@@ -116,6 +116,84 @@ function shouldShowReconvert(): boolean {
116116
// Batch orchestration
117117
// ---------------------------------------------------------------------------
118118

119+
/**
120+
* Show a "Re-convert with Docling?" confirm dialog with a "Don't ask again"
121+
* checkbox. Returns true if the user confirmed. The destructive Re-convert
122+
* action deletes the existing .md attachment(s); confirming guards against
123+
* an accidental click that would cost minutes-to-hours of compute (or API
124+
* spend, when paired with a remote LLM API).
125+
*
126+
* Uses Services.prompt.confirmEx — cross-platform, supports a checkbox
127+
* natively. If the user ticks "Don't ask again" and confirms, we flip the
128+
* `confirmReconvert` pref off; the Reset-to-defaults button restores it.
129+
*/
130+
function confirmReconvertWithUser(count: number): boolean {
131+
const Services = (globalThis as any).Services;
132+
const prompt = Services?.prompt;
133+
if (!prompt?.confirmEx) {
134+
// No prompt service available (very unusual). Default to allowing the
135+
// action — losing the confirm is preferable to silently blocking the
136+
// user from re-converting at all.
137+
return true;
138+
}
139+
140+
const Ci = (globalThis as any).Components?.interfaces;
141+
const STD =
142+
prompt.BUTTON_TITLE_IS_STRING ??
143+
Ci?.nsIPromptService?.BUTTON_TITLE_IS_STRING ??
144+
127;
145+
const CANCEL =
146+
prompt.BUTTON_TITLE_CANCEL ??
147+
Ci?.nsIPromptService?.BUTTON_TITLE_CANCEL ??
148+
2;
149+
const POS0 = prompt.BUTTON_POS_0 ?? Ci?.nsIPromptService?.BUTTON_POS_0 ?? 0;
150+
const POS1 = prompt.BUTTON_POS_1 ?? Ci?.nsIPromptService?.BUTTON_POS_1 ?? 8;
151+
const flags = STD * POS0 + CANCEL * POS1;
152+
153+
const win =
154+
(Zotero as any).getMainWindow?.() ??
155+
(Zotero as any).getActiveZoteroPane?.()?.document?.defaultView ??
156+
null;
157+
158+
const title = "Re-convert with Docling?";
159+
const body = `This will delete the existing markdown attachment${count === 1 ? "" : "s"} on ${count} selected PDF${count === 1 ? "" : "s"} and run conversion again. Any manual edits to the existing markdown will be lost.`;
160+
const checkLabel = "Don't ask again";
161+
const check = { value: false };
162+
163+
let pressed: number;
164+
try {
165+
pressed = prompt.confirmEx(
166+
win,
167+
title,
168+
body,
169+
flags,
170+
"Re-convert", // button 0 (left)
171+
null, // button 1 — title comes from CANCEL flag
172+
null, // button 2 — unused
173+
checkLabel,
174+
check,
175+
);
176+
} catch (e) {
177+
Zotero.debug(
178+
`${LOG} confirmReconvert prompt threw, allowing action: ${(e as Error).message}`,
179+
);
180+
return true;
181+
}
182+
183+
// Button index 0 is "Re-convert"; 1 is Cancel.
184+
if (pressed !== 0) return false;
185+
if (check.value) {
186+
try {
187+
setPref("confirmReconvert", false);
188+
} catch (e) {
189+
Zotero.debug(
190+
`${LOG} confirmReconvert: failed to persist opt-out: ${(e as Error).message}`,
191+
);
192+
}
193+
}
194+
return true;
195+
}
196+
119197
/**
120198
* Run convertAttachment over a resolved PDF list with parallel concurrency,
121199
* per-item progress, and aggregated status tags + toast.
@@ -148,6 +226,14 @@ async function runBatch(
148226
return;
149227
}
150228

229+
// Re-convert is destructive — it deletes existing .md attachments before
230+
// running a fresh conversion. Confirm with the user unless they've opted
231+
// out via the "Don't ask again" checkbox. Confirm BEFORE setting any
232+
// in-flight state so cancelling is a clean no-op.
233+
if (opts.force && ((getPref("confirmReconvert") ?? true) as boolean)) {
234+
if (!confirmReconvertWithUser(pdfs.length)) return;
235+
}
236+
151237
// Only one batch at a time. A second click while a batch is running
152238
// would otherwise spawn a parallel orchestrator that fights over the
153239
// shared managed-progress window.

src/modules/preferenceScript.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ const ALL_PREF_KEYS: ReadonlyArray<string> = [
3838
"attachToItem",
3939
"exportFolderPath",
4040
"notifyOnComplete",
41+
"confirmReconvert",
4142
"firstRunCompleted",
4243
"lastHealthResult",
4344
];

typings/prefs.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ declare namespace _ZoteroTypes {
3030
"attachToItem": boolean;
3131
"exportFolderPath": string;
3232
"notifyOnComplete": boolean;
33+
"confirmReconvert": boolean;
3334
"firstRunCompleted": boolean;
3435
"lastHealthResult": string;
3536
};

0 commit comments

Comments
 (0)