fix: make CJK font conditional and locale-aware
Address code review feedback from cubic-dev-ai: P1: Unconditionally passing CJKmainfont broke non-CJK conversions in environments without CJK fonts. Now the source file is scanned for CJK characters and the font argument is only added when CJK content is detected. P2: Hardcoded Noto Sans CJK SC only covers Simplified Chinese. Now detects the specific CJK language (Japanese Kana, Korean Hangul, or Chinese Hanzi) and selects the appropriate font variant (JP/KR/SC). Changes: - Add detectCJKFont() that reads the source file and returns the correct Noto Sans CJK variant, or null for non-CJK content - CJK font argument is only added when detectCJKFont returns non-null - Detection order: Japanese (Kana) > Korean (Hangul) > Chinese (Hanzi) since Japanese also uses Kanji shared with Chinese - Graceful fallback: if file can't be read, no CJK font is added - Update tests to use temp files with real CJK content (11 tests total)
This commit is contained in:
parent
143ade3d36
commit
0abd934046
2 changed files with 157 additions and 20 deletions
|
|
@ -1,4 +1,5 @@
|
||||||
import { execFile as execFileOriginal } from "node:child_process";
|
import { execFile as execFileOriginal } from "node:child_process";
|
||||||
|
import { readFileSync } from "node:fs";
|
||||||
import { ExecFileFn } from "./types";
|
import { ExecFileFn } from "./types";
|
||||||
|
|
||||||
export const properties = {
|
export const properties = {
|
||||||
|
|
@ -120,6 +121,45 @@ export const properties = {
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Detects CJK characters in a file and returns the appropriate Noto Sans CJK
|
||||||
|
* font variant for the detected language. Returns null for non-CJK content
|
||||||
|
* so that the CJK font argument is omitted entirely, keeping non-CJK
|
||||||
|
* conversions working in environments without CJK fonts installed.
|
||||||
|
*
|
||||||
|
* Detection order matters: Japanese Kana is checked first (most specific),
|
||||||
|
* then Korean Hangul, then CJK Unified Ideographs (Chinese). This is because
|
||||||
|
* Japanese text also uses Kanji (shared with Chinese), but the presence of
|
||||||
|
* Kana uniquely identifies Japanese content.
|
||||||
|
*/
|
||||||
|
function detectCJKFont(filePath: string): string | null {
|
||||||
|
let content: string;
|
||||||
|
try {
|
||||||
|
content = readFileSync(filePath, "utf-8");
|
||||||
|
} catch {
|
||||||
|
// If the file can't be read, don't add a CJK font to avoid breaking
|
||||||
|
// conversions. Pandoc will still process the file independently.
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Japanese: Hiragana (U+3040-309F) or Katakana (U+30A0-30FF)
|
||||||
|
if (/[\u3040-\u309f\u30a0-\u30ff]/.test(content)) {
|
||||||
|
return "Noto Sans CJK JP";
|
||||||
|
}
|
||||||
|
|
||||||
|
// Korean: Hangul Syllables (U+AC00-D7AF)
|
||||||
|
if (/[\uac00-\ud7af]/.test(content)) {
|
||||||
|
return "Noto Sans CJK KR";
|
||||||
|
}
|
||||||
|
|
||||||
|
// Chinese: CJK Unified Ideographs (U+4E00-9FFF)
|
||||||
|
if (/[\u4e00-\u9fff]/.test(content)) {
|
||||||
|
return "Noto Sans CJK SC";
|
||||||
|
}
|
||||||
|
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
export function convert(
|
export function convert(
|
||||||
filePath: string,
|
filePath: string,
|
||||||
fileType: string,
|
fileType: string,
|
||||||
|
|
@ -136,11 +176,14 @@ export function convert(
|
||||||
|
|
||||||
if (xelatex.includes(convertTo)) {
|
if (xelatex.includes(convertTo)) {
|
||||||
args.push("--pdf-engine=xelatex");
|
args.push("--pdf-engine=xelatex");
|
||||||
// Specify a CJK-capable main font so that documents containing Chinese,
|
// Detect CJK characters in the source file and set an appropriate
|
||||||
// Japanese or Korean characters render correctly instead of producing
|
// CJK font only when needed. This avoids breaking non-CJK conversions
|
||||||
// blank/missing glyphs. "Noto Sans CJK SC" is provided by the
|
// in environments without CJK fonts, and selects the correct locale-
|
||||||
// fonts-noto-cjk package installed in the Dockerfile.
|
// specific font variant (JP/KR/SC) for proper glyph rendering.
|
||||||
args.push("-V", "CJKmainfont=Noto Sans CJK SC");
|
const cjkFont = detectCJKFont(filePath);
|
||||||
|
if (cjkFont) {
|
||||||
|
args.push("-V", `CJKmainfont=${cjkFont}`);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
args.push(filePath);
|
args.push(filePath);
|
||||||
|
|
|
||||||
|
|
@ -1,9 +1,34 @@
|
||||||
import { beforeEach, expect, test, describe } from "bun:test";
|
import { beforeEach, expect, test, describe, afterEach } from "bun:test";
|
||||||
import { convert } from "../../src/converters/pandoc";
|
import { convert } from "../../src/converters/pandoc";
|
||||||
import type { ExecFileFn } from "../../src/converters/types";
|
import type { ExecFileFn } from "../../src/converters/types";
|
||||||
|
import { writeFileSync, unlinkSync } from "node:fs";
|
||||||
|
import { join } from "node:path";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
|
||||||
describe("convert", () => {
|
describe("convert", () => {
|
||||||
let mockExecFile: ExecFileFn;
|
let mockExecFile: ExecFileFn;
|
||||||
|
let tempFiles: string[] = [];
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
for (const f of tempFiles) {
|
||||||
|
try {
|
||||||
|
unlinkSync(f);
|
||||||
|
} catch {
|
||||||
|
/* ignore */
|
||||||
|
}
|
||||||
|
}
|
||||||
|
tempFiles = [];
|
||||||
|
});
|
||||||
|
|
||||||
|
function makeTempFile(content: string, ext = "md"): string {
|
||||||
|
const path = join(
|
||||||
|
tmpdir(),
|
||||||
|
`pandoc-test-${Date.now()}-${Math.random().toString(36).slice(2)}.${ext}`,
|
||||||
|
);
|
||||||
|
writeFileSync(path, content, "utf-8");
|
||||||
|
tempFiles.push(path);
|
||||||
|
return path;
|
||||||
|
}
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
mockExecFile = (cmd, args, callback) => callback(null, "output-data", "");
|
mockExecFile = (cmd, args, callback) => callback(null, "output-data", "");
|
||||||
|
|
@ -45,10 +70,11 @@ describe("convert", () => {
|
||||||
callback(null, "output-data", "");
|
callback(null, "output-data", "");
|
||||||
};
|
};
|
||||||
|
|
||||||
await convert("input.md", "markdown", "pdf", "output.pdf", undefined, mockExecFile);
|
const filePath = makeTempFile("# Hello World\n");
|
||||||
|
await convert(filePath, "markdown", "pdf", "output.pdf", undefined, mockExecFile);
|
||||||
|
|
||||||
expect(calledArgs[1][0]).toBe("--pdf-engine=xelatex");
|
expect(calledArgs[1][0]).toBe("--pdf-engine=xelatex");
|
||||||
expect(calledArgs[1]).toContain("input.md");
|
expect(calledArgs[1]).toContain(filePath);
|
||||||
expect(calledArgs[1]).toContain("-f");
|
expect(calledArgs[1]).toContain("-f");
|
||||||
expect(calledArgs[1]).toContain("markdown");
|
expect(calledArgs[1]).toContain("markdown");
|
||||||
expect(calledArgs[1]).toContain("-t");
|
expect(calledArgs[1]).toContain("-t");
|
||||||
|
|
@ -57,33 +83,85 @@ describe("convert", () => {
|
||||||
expect(calledArgs[1]).toContain("output.pdf");
|
expect(calledArgs[1]).toContain("output.pdf");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should add CJK mainfont argument for pdf output", async () => {
|
test("should not add CJK mainfont for non-CJK content", async () => {
|
||||||
let calledArgs: Parameters<ExecFileFn> = ["", [], () => {}];
|
let calledArgs: Parameters<ExecFileFn> = ["", [], () => {}];
|
||||||
mockExecFile = (cmd, args, callback) => {
|
mockExecFile = (cmd, args, callback) => {
|
||||||
calledArgs = [cmd, args, callback];
|
calledArgs = [cmd, args, callback];
|
||||||
callback(null, "output-data", "");
|
callback(null, "output-data", "");
|
||||||
};
|
};
|
||||||
|
|
||||||
await convert("input.md", "markdown", "pdf", "output.pdf", undefined, mockExecFile);
|
const filePath = makeTempFile("# Hello World\nThis is English text.\n");
|
||||||
|
await convert(filePath, "markdown", "pdf", "output.pdf", undefined, mockExecFile);
|
||||||
|
|
||||||
const cjkFontIndex = calledArgs[1].indexOf("-V");
|
expect(calledArgs[1].some((arg) => arg.startsWith("CJKmainfont"))).toBe(false);
|
||||||
expect(cjkFontIndex).toBeGreaterThan(-1);
|
|
||||||
expect(calledArgs[1][cjkFontIndex + 1]).toBe("CJKmainfont=Noto Sans CJK SC");
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should add CJK mainfont argument for latex output", async () => {
|
test("should add CJKmainfont=SC for Chinese content", async () => {
|
||||||
let calledArgs: Parameters<ExecFileFn> = ["", [], () => {}];
|
let calledArgs: Parameters<ExecFileFn> = ["", [], () => {}];
|
||||||
mockExecFile = (cmd, args, callback) => {
|
mockExecFile = (cmd, args, callback) => {
|
||||||
calledArgs = [cmd, args, callback];
|
calledArgs = [cmd, args, callback];
|
||||||
callback(null, "output-data", "");
|
callback(null, "output-data", "");
|
||||||
};
|
};
|
||||||
|
|
||||||
await convert("input.md", "markdown", "latex", "output.tex", undefined, mockExecFile);
|
const filePath = makeTempFile("# 中文标题\n这是一段中文内容。\n");
|
||||||
|
await convert(filePath, "markdown", "pdf", "output.pdf", undefined, mockExecFile);
|
||||||
|
|
||||||
|
expect(calledArgs[1].some((arg) => arg === "CJKmainfont=Noto Sans CJK SC")).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("should add CJKmainfont=JP for Japanese content", async () => {
|
||||||
|
let calledArgs: Parameters<ExecFileFn> = ["", [], () => {}];
|
||||||
|
mockExecFile = (cmd, args, callback) => {
|
||||||
|
calledArgs = [cmd, args, callback];
|
||||||
|
callback(null, "output-data", "");
|
||||||
|
};
|
||||||
|
|
||||||
|
const filePath = makeTempFile("# 日本語タイトル\nこれは日本語の内容です。\n");
|
||||||
|
await convert(filePath, "markdown", "pdf", "output.pdf", undefined, mockExecFile);
|
||||||
|
|
||||||
|
expect(calledArgs[1].some((arg) => arg === "CJKmainfont=Noto Sans CJK JP")).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("should add CJKmainfont=KR for Korean content", async () => {
|
||||||
|
let calledArgs: Parameters<ExecFileFn> = ["", [], () => {}];
|
||||||
|
mockExecFile = (cmd, args, callback) => {
|
||||||
|
calledArgs = [cmd, args, callback];
|
||||||
|
callback(null, "output-data", "");
|
||||||
|
};
|
||||||
|
|
||||||
|
const filePath = makeTempFile("# 한국어 제목\n이것은 한국어 내용입니다.\n");
|
||||||
|
await convert(filePath, "markdown", "pdf", "output.pdf", undefined, mockExecFile);
|
||||||
|
|
||||||
|
expect(calledArgs[1].some((arg) => arg === "CJKmainfont=Noto Sans CJK KR")).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("should prefer Japanese font when both Kanji and Kana present", async () => {
|
||||||
|
let calledArgs: Parameters<ExecFileFn> = ["", [], () => {}];
|
||||||
|
mockExecFile = (cmd, args, callback) => {
|
||||||
|
calledArgs = [cmd, args, callback];
|
||||||
|
callback(null, "output-data", "");
|
||||||
|
};
|
||||||
|
|
||||||
|
// Japanese text with both Kanji and Hiragana
|
||||||
|
const filePath = makeTempFile("# 日本語のテスト\nこれは漢字とひらがなの文章です。\n");
|
||||||
|
await convert(filePath, "markdown", "pdf", "output.pdf", undefined, mockExecFile);
|
||||||
|
|
||||||
|
expect(calledArgs[1].some((arg) => arg === "CJKmainfont=Noto Sans CJK JP")).toBe(true);
|
||||||
|
expect(calledArgs[1].some((arg) => arg === "CJKmainfont=Noto Sans CJK SC")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("should not add CJK mainfont for latex when content is non-CJK", async () => {
|
||||||
|
let calledArgs: Parameters<ExecFileFn> = ["", [], () => {}];
|
||||||
|
mockExecFile = (cmd, args, callback) => {
|
||||||
|
calledArgs = [cmd, args, callback];
|
||||||
|
callback(null, "output-data", "");
|
||||||
|
};
|
||||||
|
|
||||||
|
const filePath = makeTempFile("# English Title\nSome text.\n");
|
||||||
|
await convert(filePath, "markdown", "latex", "output.tex", undefined, mockExecFile);
|
||||||
|
|
||||||
expect(calledArgs[1][0]).toBe("--pdf-engine=xelatex");
|
expect(calledArgs[1][0]).toBe("--pdf-engine=xelatex");
|
||||||
const cjkFontIndex = calledArgs[1].indexOf("-V");
|
expect(calledArgs[1].some((arg) => arg.startsWith("CJKmainfont"))).toBe(false);
|
||||||
expect(cjkFontIndex).toBeGreaterThan(-1);
|
|
||||||
expect(calledArgs[1][cjkFontIndex + 1]).toBe("CJKmainfont=Noto Sans CJK SC");
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test("should not add CJK mainfont for non-pdf/latex output", async () => {
|
test("should not add CJK mainfont for non-pdf/latex output", async () => {
|
||||||
|
|
@ -93,15 +171,31 @@ describe("convert", () => {
|
||||||
callback(null, "output-data", "");
|
callback(null, "output-data", "");
|
||||||
};
|
};
|
||||||
|
|
||||||
await convert("input.md", "markdown", "html", "output.html", undefined, mockExecFile);
|
const filePath = makeTempFile("# 中文标题\n");
|
||||||
|
await convert(filePath, "markdown", "html", "output.html", undefined, mockExecFile);
|
||||||
|
|
||||||
expect(calledArgs[1].some((arg) => arg.startsWith("CJKmainfont"))).toBe(false);
|
expect(calledArgs[1].some((arg) => arg.startsWith("CJKmainfont"))).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("should handle unreadable file gracefully without CJK font", async () => {
|
||||||
|
let calledArgs: Parameters<ExecFileFn> = ["", [], () => {}];
|
||||||
|
mockExecFile = (cmd, args, callback) => {
|
||||||
|
calledArgs = [cmd, args, callback];
|
||||||
|
callback(null, "output-data", "");
|
||||||
|
};
|
||||||
|
|
||||||
|
// Use a non-existent file path
|
||||||
|
await convert("/nonexistent/file.md", "markdown", "pdf", "output.pdf", undefined, mockExecFile);
|
||||||
|
|
||||||
|
expect(calledArgs[1][0]).toBe("--pdf-engine=xelatex");
|
||||||
|
expect(calledArgs[1].some((arg) => arg.startsWith("CJKmainfont"))).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
test("should reject if execFile returns an error", async () => {
|
test("should reject if execFile returns an error", async () => {
|
||||||
|
const filePath = makeTempFile("# Test\n");
|
||||||
mockExecFile = (cmd, args, callback) => callback(new Error("fail"), "", "");
|
mockExecFile = (cmd, args, callback) => callback(new Error("fail"), "", "");
|
||||||
await expect(
|
await expect(
|
||||||
convert("input.md", "markdown", "html", "output.html", undefined, mockExecFile),
|
convert(filePath, "markdown", "html", "output.html", undefined, mockExecFile),
|
||||||
).rejects.toMatch(/error: Error: fail/);
|
).rejects.toMatch(/error: Error: fail/);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue