Follow-up to #5379: move Pandoc conversion out of Export, gate it behind a preference, and report which binary was found by dimbc · Pull Request #5513 · marktext/marktext · GitHub
Skip to content

Follow-up to #5379: move Pandoc conversion out of Export, gate it behind a preference, and report which binary was found - #5513

Merged
Jocs merged 1 commit into
marktext:developfrom
dimbc:pandoc-followup
Oct 8, 2026
Merged

Jocs merged 1 commit into
marktext:developfrom
dimbc:pandoc-followup

Conversation

@dimbc

@dimbc dimbc commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

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 of Export ▸. It is a converter with its own target list, not a third export format, so it now sits directly below Export ▸ in the File menu at the same level. The separator that used to split HTML/PDF from the converter went with it, leaving Export ▸ with its two real formats and dropping one menu level.

2. A preference gates the menu entry, off by default. New showPandocConvert (General), default false. The Convert 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 through Menu.updateAppMenu() via the existing broadcast-preferences-changed dispatch, as you noted — the entry is global rather than per-window, so nothing needs adding to the updateMenuItem list.

3. The switch reports which pandoc was found. resolvePandocCommand() answers both "is it there" and "which binary", and the settings pane shows Found: <path> / Found on PATH / Not found next to the switch. It is deliberately not the existing mt::cmd::exists check: that one only looks at PATH, while the Windows installers put pandoc.exe in a Pandoc folder under ProgramFiles/LOCALAPPDATA that PATH never 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.json is the known-settings list Preference.init() prunes against on every launch, so a key added only to schema.json was deleted and then re-filled with the schema default. preference-file-sync.spec.ts now asserts the two files agree, so the next preference key cannot repeat this.

Closes #3917.

Type of change

  • Bug fix (non-breaking, fixes an issue)
  • New feature (non-breaking, adds functionality)
  • Breaking change (causes existing functionality to change)
  • Documentation update

Test plan

  • New tests added (or explain why not needed)
  • Manually tested on: Windows

Two specs, pandoc-switch-availability.spec.ts and preference-file-sync.spec.ts (114 lines). The availability rule lives in prefComponents/general/pandoc.ts as 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 --noEmit clean, eslint 0 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.json and schema.json must 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.
  • No migration is written on purpose: startup already back-fills a missing key from the schema.
  • 12 locale files changed (11 existing + ru, which landed on develop after feat(export): convert Markdown to arbitrary formats via Pandoc #5379). locale-validation.spec.ts requires every locale to match en.json key for key, so the six new strings are translated in all of them.
  • Points from Emoji at the begining of header doesn't work #4 (reference templates, conversion options, per-format settings) are intentionally not here — happy to open one issue per idea if you want to gauge interest first.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@dimbc

dimbc commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

@Jocs Jocs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/desktop/static/locales/en.json Outdated
},
"pandoc": {
"title": "Pandoc Conversion",
"description": "Show the Pandoc conversion menu. Requires Pandoc to be installed.",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
"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 }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Jocs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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([])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/desktop/src/main/ipc/cmd.ts Outdated
})

// Report the binary an export would spawn: `mt::cmd::exists` only asks `PATH` (#2751).
ipcMain.handle('mt::pandoc::command', () => resolvePandocCommand())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
]
},
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

dimbc added a commit to dimbc/marktext that referenced this pull request Oct 2, 2026
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.
dimbc added a commit to dimbc/marktext that referenced this pull request Oct 7, 2026
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
@dimbc
dimbc force-pushed the pandoc-followup branch 2 times, most recently from df54f20 to cb927f4 Compare October 7, 2026 16:01
@dimbc

dimbc commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the pass — every point is addressed, and the branch is now a single commit rebased on the current develop.

In short: the descriptions read as labels, the pane reports the binary rather than a name, PandocCommandInfo is declared once, the handler has an error path, the note wraps, PREFERENCES.md gained its row, and there is a re-check button beside the status. The two specs that asserted against the SFC's source text now test behaviour instead.

@Jocs

Jocs commented Oct 7, 2026

Copy link
Copy Markdown
Member

CI looks like failed?

@dimbc

dimbc commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Not this change. The same commit passed 4 of 5 runs on a fork, and the one failure was a different spec — all-blocks-roundtrip.spec.ts, a 5s expect.poll timing out — same shape as the runs on feat/diagram-viewer-4363. A re-run should clear it.

@Jocs

Jocs commented Oct 8, 2026

Copy link
Copy Markdown
Member

Not this change. The same commit passed 4 of 5 runs on a fork, and the one failure was a different spec — all-blocks-roundtrip.spec.ts, a 5s expect.poll timing out — same shape as the runs on feat/diagram-viewer-4363. A re-run should clear it.

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.
@Jocs
Jocs merged commit 3d5beb1 into marktext:develop Oct 8, 2026
12 checks passed
@Jocs

Jocs commented Oct 8, 2026

Copy link
Copy Markdown
Member

csells added a commit to csells/MarkText that referenced this pull request Oct 13, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Under File Menu, add convert menu that calls Pandoc to convert Markdown to other formats

2 participants