Repository navigation
Follow-up to #5379: move Pandoc conversion out of Export, gate it behind a preference, and report which binary was found - #5513
Conversation
Jocs
left a comment
There was a problem hiding this comment.
Tried this on macOS with pandoc 3.11 installed. The behaviour matches the description — the menu entry follows the switch, and the switch survives a restart — but two things in the new preference row read wrong.
| }, | ||
| "pandoc": { | ||
| "title": "Pandoc Conversion", | ||
| "description": "Show the Pandoc conversion menu. Requires Pandoc to be installed.", |
There was a problem hiding this comment.
bool appends the colon itself, so this has to be a label, not a sentence.
prefComponents/common/bool/index.vue renders <span>{{ description }}:</span>, so every switch description gets a trailing colon. On screen this row reads:
Show the Pandoc conversion menu. Requires Pandoc to be installed.:
Two sentences, a full stop, then the component's colon. Every other description in this pane is a short label with no terminal punctuation — "Wrap text in table of contents", "Open folders in new window", "Restore previous editor state" — which is why they come out clean. This is the only one written as prose.
| "description": "Show the Pandoc conversion menu. Requires Pandoc to be installed.", | |
| "description": "Show the Pandoc conversion menu", |
The requirement sentence is worth keeping, just not here: bool already accepts detailedDescription, which renders it in the tooltip behind the info icon. preferences.search.items.showPandocConvert can keep the full sentence as-is — full sentences are the convention in that bucket.
The same edit is needed in the other 11 locales; zh-CN currently renders 「显示 Pandoc 转换菜单。需要先安装 Pandoc。:」.
| const command = getCommand() | ||
| if (command !== pandocCommand) return { command, onPath: false } | ||
| return commandExists.sync(pandocCommand) | ||
| ? { command: pandocCommand, onPath: true } |
There was a problem hiding this comment.
Found on PATH never shows a path on the platforms where it is most likely to matter.
An absolute path only comes out of here when MARKTEXT_PANDOC is set or one of the Windows well-known folders hits. An ordinary macOS/Linux install falls through to commandExists.sync('pandoc'), which answers yes/no and nothing else — so Found: {path} is effectively Windows-only, and everyone else always sees Found on PATH.
That inverts what point 3 set out to do. Tested here on macOS with pandoc 3.11 at /opt/homebrew/bin/pandoc: the pane says Found on PATH, which describes how the lookup was done rather than answering which binary was found — the question a user actually has when two pandocs are installed.
Suggest resolving the bare name to a real file so every platform lands in the Found: {path} branch: walk process.env.PATH (honouring PATHEXT on win32) and return the first executable match. onPath then drops out of the IPC payload, foundOnPath drops out of the 12 locale files, and pandocSwitchState() is left with found/not-found. pandocLocations() already resolves candidates to files, so this stays in the same shape as the code around it.
Worth not reaching for the which package to do it: it is in the store as a transitive dep but is not declared in packages/desktop/package.json, and importing an undeclared package is exactly what broke #5159.
One edge to keep: if the lookup cannot produce a path but commandExists says yes, fall back to a generic "found" note rather than reporting Not found.
Jocs
left a comment
There was a problem hiding this comment.
Went through the rest of the change on macOS with pandoc 3.11 installed. The feature behaves as described — the entry follows the switch, the switch survives a restart — and moving the converter out of Export ▸ is the right call. The two wording comments I left earlier still stand; the rest of my pass is below.
One thing that has no line to hang off: packages/website/content/docs/end-user/PREFERENCES.md says its tables mirror schema.json, and the General table has no showPandocConvert row. The key is off by default and the menu entry invisible until it is set, so that page is the only place a user editing preferences.json by hand can learn it exists.
| Boolean(entry.type) && key in defaults && typeOf(defaults[key]) !== entry.type | ||
| ) | ||
| .map(([key, entry]) => `${key}: expected ${entry.type}, got ${typeOf(defaults[key])}`) | ||
| expect(mismatched).toEqual([]) |
There was a problem hiding this comment.
The new guard compares keys and types, but not values, and only in one direction.
A new preference whose schema.json default and static/preference.json value disagree passes both assertions — and the two files resolve differently depending on the path: Preference.init() writes static/preference.json wholesale on first launch, while a key that was pruned and back-filled comes from the schema, and PREFERENCES.md publishes the schema value as authoritative. That is the same silent-divergence class as the restart bug this spec was written for.
Comparing entry.default against defaults[key] closes it. Heads up that four existing pairs disagree today (followSystemTheme, codeBlockLineNumbers, wrapCodeBlocks, fileSortBy), so the assertion needs a decision about those — aligning them here, or landing the assertion in a follow-up — rather than being added blind.
The reverse direction is worth a line too: a key in static/preference.json with no schema.json entry is written and read with no type validation and no declared default — the mirror image of the bug this PR fixes.
| } | ||
|
|
||
| /** Which pandoc would run; `exists()` can only answer yes or no. */ | ||
| export interface PandocCommandInfo { |
There was a problem hiding this comment.
One IPC payload, three independent declarations.
PandocCommandInfo here, the inline { command: string | null; onPath: boolean } at src/shared/types/ipc.ts:73, and PandocProbe at prefComponents/general/pandoc.ts:2 all describe the same object, and nothing links them. Rename a field on the main side and vue-tsc stays green while probe.command is undefined at runtime, which makes pandocSwitchState report Not found on a machine that has pandoc installed.
The repo CLAUDE.md is explicit about where this belongs: "Cross-process types live in packages/desktop/src/shared/types/", with the channel contract in ipc.ts. Declared once there and imported on both sides, the failure mode is gone.
| }) | ||
|
|
||
| // Report the binary an export would spawn: `mt::cmd::exists` only asks `PATH` (#2751). | ||
| ipcMain.handle('mt::pandoc::command', () => resolvePandocCommand()) |
There was a problem hiding this comment.
The only handler in this file without an error path.
mt::cmd::exists right above wraps its body and answers false on throw. This one calls resolvePandocCommand(), which reads process.env.MARKTEXT_PANDOC, lstats up to six candidate paths through isFile2, then shells out via commandExists.sync. Any throw rejects the invoke, and the renderer awaits it inside a bare onMounted(async () => …) whose promise Vue never inspects — so it surfaces as an unhandled rejection, pandocProbe stays null, and the pane shows no status line at all with nothing in the log tying it to pandoc.
|
|
||
| <!-- The note lives on the group, not on the switch: a disabled `bool` dims its whole | ||
| section, which would fade the very line that explains why it is disabled. --> | ||
| <compound :notes="pandocStatus"> |
There was a problem hiding this comment.
A Windows install path is one unbreakable token, and the note has no wrapping rule.
.notes (pages/preference.vue:98) is display: block with no overflow-wrap/word-break. The paths pandocLocations() returns — %LOCALAPPDATA%\Microsoft\WinGet\Links\pandoc.exe, %USERPROFILE%\scoop\shims\pandoc.exe — usually contain no space and no /, and CSS offers no break opportunity at \, so Found: {path} becomes a single word. Inside .pref-content (max-width: calc(100vw - var(--prefSideBarWidth))) it overflows and gives the settings window a horizontal scrollbar — on the platform this feature targets. overflow-wrap: anywhere on the note fixes it.
| const bool = block.slice(block.indexOf('<bool')) | ||
| const switchTag = bool.slice(0, bool.indexOf('/>') + 2) | ||
|
|
||
| expect(switchTag).toContain(':disable="pandocDisabled"') |
There was a problem hiding this comment.
These three assertions test the SFC's source text, not its behavior.
pandocBlock() reads index.vue as a string and slices between <compound and </compound>, then matches the literals :disable="pandocDisabled" and :notes=. Renaming the computed, writing <Bool, reordering attributes, or Prettier moving /> onto its own line turns the suite red with no behavior change; conversely it stays green for any wiring that merely spells those attributes correctly. The pure-function cases above them are the part carrying real value — I'd drop these three rather than keep assertions that can only be wrong in both directions.
| } | ||
| ] | ||
| }, | ||
| { |
There was a problem hiding this comment.
This move left a comment pointing at a submenu that no longer holds these targets.
utils/pandoc.ts:10 still reads Targets offered by "Export → Convert with Pandoc", while the entry is now a sibling of Export ▸. It is the only in-repo description of where the nine targets surface, so the next reader goes looking under Export and doesn't find them.
|
|
||
| const pandocDisabled = computed<boolean>(() => pandocSwitch.value.disabled) | ||
|
|
||
| onMounted(async () => { |
There was a problem hiding this comment.
Worth adding: a manual re-check button beside the status.
The probe runs once, on mount. A user who reads "Not found", installs pandoc, and returns to the still-open settings window is told the same thing, and this pane is the only place the app reports detection. An explicit button is clearer here than re-probing on focus or on a timer.
There is a precedent in prefComponents/image/components/uploader/index.vue — the PicGo detection block with its re-test button. Worth taking the button and nothing else: that block also polls every 30s with exponential backoff to five minutes (scheduleNextDetection, :404), keeps three timers, inserts two artificial 500ms delays in manualDetection (:585), and hardcodes toLocaleString('zh-CN', …) in its timestamps. For pandoc it is a button plus an isProbing ref around the invoke that already exists.
One layout note: the status rides on compound's notes prop deliberately, so a disabled bool does not dim the line explaining why it is disabled. A button needs its own markup — rendering the status and the button in the #children slot as a sibling of <bool> keeps that property without touching the shared compound.
| recentlyUsedFiles: string[] | ||
| ): MenuItemConstructorOptions { | ||
| const { autoSave } = userPreference.getAll() as { autoSave?: boolean } | ||
| const { autoSave, showPandocConvert } = userPreference.getAll() as { |
There was a problem hiding this comment.
Nit: userPreference.getAll() returns IUserPreferences, which this PR just taught about showPandocConvert (shared/types/preferences.ts:75), so the widened assertion is no longer buying anything and turns a checked read into an unchecked one. Dropping it type-checks on its own.
Answers the review on marktext#5513. The pane now reports the file an export would spawn rather than the way it was looked up, and the settings read as labels instead of sentences. - `findOnPath()` resolves `PATH` in the main process. `command-exists` answers yes or no and discards the path it found, so `Found on PATH` was all macOS and Linux ever saw (marktext#2751) — `/opt/homebrew/bin/pandoc` is a better answer. - `PandocCommandInfo` moves to `shared/types/pandoc.ts`, declared once for both processes. Its `onPath` field and the twelve `foundOnPath` messages have no reader once the note names a file, so they go with it. - A `recheck` button sits beside the note, for the reader who installs pandoc while the pane is open. - `.notes` wraps: a Windows path offers no break opportunity at a backslash, and the long ones widened the settings window. - The IPC handler catches what the lookup can throw, so the pane keeps drawing a status line instead of staying blank. - `preference-file-sync.spec.ts` compares each schema default with the file, which surfaced four pairs that disagreed. The schema follows the file, since that is what a first start writes — 4482641 settled this for two of them.
Name the pandoc an export would spawn on every platform: `findOnPath()` walks
`PATH` (`PATHEXT` on win32, `X_OK` on POSIX) instead of falling back to a
yes/no, so `Found on PATH` becomes `Found: {path}` everywhere. A batch shim
`spawn` refuses to run is reported as found rather than missing.
Addresses the review on marktext#5513:
- the switch description is a short label in all twelve locales, with the
requirement sentence moved to `detailedDescription` and the search bucket
left as prose
- `PandocCommandInfo` is declared once in `shared/types/`
- the IPC handler logs and answers "no pandoc" instead of rejecting
- the note wraps, so a Windows install path cannot overflow the pane
- the re-check button sits in `#children` beside the `<bool>`, so `compound`
is untouched
- `preference-file-sync.spec.ts` compares default values and checks the
reverse direction, which found `treePathExcludePatterns` undeclared
- the SFC source-text assertions are dropped, and the widened assertion at
`file.ts:15` with them
df54f20 to
cb927f4
Compare
|
Thanks for the pass — every point is addressed, and the branch is now a single commit rebased on the current In short: the descriptions read as labels, the pane reports the binary rather than a name, |
|
CI looks like failed? |
|
Not this change. The same commit passed 4 of 5 runs on a fork, and the one failure was a different spec — |
I rerun the failed CI, and please solve the conflicts. |
… preference The Pandoc submenu sat under Export among the formats MarkText renders itself, which hid that it needs an external binary. It moves to the File menu, appears only while `showPandocConvert` is on, and the preference pane reports which binary an export would actually spawn rather than a name that may not exist (marktext#2751). The switch greys out when no pandoc is found, unless it is already on, since the switch itself is the only way back out.
Upstream marktext#5513 moved Pandoc conversion out of File > Export into its own File submenu, shown only with showPandocConvert on. The installed editorial pass turns it on and uses that path. The artifact launch spec closes its debugging connection before quitting the app, which does not quit while one is attached, and stops the app's process group outright if it lingers.

Follow-up to the four points from #5379 (comment), after #5379 (234a9b3) merged. Points 1–3 are implemented here; point 4 stays as issues.
Summary
1.
Convert with Pandoc ▸moves out ofExport ▸. It is a converter with its own target list, not a third export format, so it now sits directly belowExport ▸in the File menu at the same level. The separator that used to split HTML/PDF from the converter went with it, leavingExport ▸with its two real formats and dropping one menu level.2. A preference gates the menu entry, off by default. New
showPandocConvert(General), defaultfalse. TheConvert with Pandoc ▸entry is only added when it is on, so users without pandoc no longer see nine entries that cannot do anything. The switch goes throughMenu.updateAppMenu()via the existingbroadcast-preferences-changeddispatch, as you noted — the entry is global rather than per-window, so nothing needs adding to theupdateMenuItemlist.3. The switch reports which pandoc was found.
resolvePandocCommand()answers both "is it there" and "which binary", and the settings pane showsFound: <path>/Found on PATH/Not foundnext to the switch. It is deliberately not the existingmt::cmd::existscheck: that one only looks atPATH, while the Windows installers putpandoc.exein aPandocfolder underProgramFiles/LOCALAPPDATAthatPATHnever covers (#2751, #3917). When pandoc is missing the switch is greyed out, and the note keeps explaining why.Also fixed: the switch did not survive a restart.
static/preference.jsonis the known-settings listPreference.init()prunes against on every launch, so a key added only toschema.jsonwas deleted and then re-filled with the schema default.preference-file-sync.spec.tsnow asserts the two files agree, so the next preference key cannot repeat this.Closes #3917.
Type of change
Test plan
Two specs,
pandoc-switch-availability.spec.tsandpreference-file-sync.spec.ts(114 lines). The availability rule lives inprefComponents/general/pandoc.tsas a pure function, so the spec exercises it directly rather than compiling the SFC — the desktop unit environment has no@vue/test-utils. Each new case was checked by mutating the source and watching the intended assertion turn red.Full suite with the two known-timing-flaky files excluded (
move-image-to-folder,pdf): 866 passed / 1 skipped.vue-tsc --noEmitclean,eslint0 errors. Manually confirmed on a Windows build: menu entry appears/vanishes with the switch, the switch survives a restart, and a machine without pandoc shows a greyed switch with a readable note.Notes for reviewers [optional]
static/preference.jsonandschema.jsonmust both carry a new preference key — the first is the prune list, the second only carries the default. This is what caused the restart bug; the new spec guards it.ru, which landed ondevelopafter feat(export): convert Markdown to arbitrary formats via Pandoc #5379).locale-validation.spec.tsrequires every locale to matchen.jsonkey for key, so the six new strings are translated in all of them.By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.