-
-
Notifications
You must be signed in to change notification settings - Fork 592
Add Chinese Font Support for Pandoc #471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 issues found across 5 files
Prompt for AI agents (all 4 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="docker-compose.yml">
<violation number="1" location="docker-compose.yml:9">
P1: Hardcoded JWT_SECRET with a real-looking value is a security risk. Users may copy this directly into production. Use a clearly-marked placeholder like the existing `compose.yaml` uses (`aLongAndSecretStringUsedToSignTheJSONWebToken1234`) or reference an external secret.</violation>
</file>
<file name="compose.yaml">
<violation number="1" location="compose.yaml:21">
P3: The comment is misleading - this environment variable specifies the font path inside the container, not on the host. The volume mount (line above) handles mapping host fonts to the container path. Consider changing the comment to clarify this is the container path.</violation>
</file>
<file name="Dockerfile">
<violation number="1" location="Dockerfile:77">
P2: Trailing whitespace after backslash line continuation characters. While this may work in many shells, it's considered bad practice and can cause issues. Remove trailing spaces after the `\` characters.</violation>
<violation number="2" location="Dockerfile:87">
P1: Incorrect font path: `fonts-noto-cjk` installs to `/usr/share/fonts/opentype/noto/`, not `/usr/share/fonts/truetype/noto/`. The symlink will point to a non-existent directory.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
docker-compose.yml
Outdated
| ports: | ||
| - "3001:3000" | ||
| environment: | ||
| - JWT_SECRET=ivanzhangwb # will use randomUUID() if unset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Hardcoded JWT_SECRET with a real-looking value is a security risk. Users may copy this directly into production. Use a clearly-marked placeholder like the existing compose.yaml uses (aLongAndSecretStringUsedToSignTheJSONWebToken1234) or reference an external secret.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docker-compose.yml, line 9:
<comment>Hardcoded JWT_SECRET with a real-looking value is a security risk. Users may copy this directly into production. Use a clearly-marked placeholder like the existing `compose.yaml` uses (`aLongAndSecretStringUsedToSignTheJSONWebToken1234`) or reference an external secret.</comment>
<file context>
@@ -0,0 +1,16 @@
+ ports:
+ - "3001:3000"
+ environment:
+ - JWT_SECRET=ivanzhangwb # will use randomUUID() if unset
+ - HTTP_ALLOWED=true # uncomment this if accessing it over a non-https connection
+ - PANDOC_ENABLE_CHINESE_FONT=true
</file context>
✅ Addressed in 695a17a
compose.yaml
Outdated
| - TZ=Europe/Stockholm # set your timezone, defaults to UTC | ||
| # - UNAUTHENTICATED_USER_SHARING=true # for use with ALLOW_UNAUTHENTICATED=true to share history with all unauthenticated users / devices | ||
| # - PANDOC_ENABLE_CHINESE_FONT=true # enable Chinese font support for Pandoc PDF/LaTeX conversion | ||
| # - PANDOC_CHINESE_FONT_PATH=/usr/share/fonts/truetype/chinese # path to Chinese font directory on host |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: The comment is misleading - this environment variable specifies the font path inside the container, not on the host. The volume mount (line above) handles mapping host fonts to the container path. Consider changing the comment to clarify this is the container path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At compose.yaml, line 21:
<comment>The comment is misleading - this environment variable specifies the font path inside the container, not on the host. The volume mount (line above) handles mapping host fonts to the container path. Consider changing the comment to clarify this is the container path.</comment>
<file context>
@@ -16,5 +17,8 @@ services:
- TZ=Europe/Stockholm # set your timezone, defaults to UTC
# - UNAUTHENTICATED_USER_SHARING=true # for use with ALLOW_UNAUTHENTICATED=true to share history with all unauthenticated users / devices
+ # - PANDOC_ENABLE_CHINESE_FONT=true # enable Chinese font support for Pandoc PDF/LaTeX conversion
+ # - PANDOC_CHINESE_FONT_PATH=/usr/share/fonts/truetype/chinese # path to Chinese font directory on host
+ # - PANDOC_CHINESE_FONT_FAMILY=NotoSansCJK # Chinese font family name
ports:
</file context>
| # - PANDOC_CHINESE_FONT_PATH=/usr/share/fonts/truetype/chinese # path to Chinese font directory on host | |
| # - PANDOC_CHINESE_FONT_PATH=/usr/share/fonts/truetype/chinese # path to Chinese font directory in container (mount fonts via volumes above) |
✅ Addressed in 695a17a
Dockerfile
Outdated
| texlive-latex-extra \ | ||
| texlive-latex-recommended \ | ||
| texlive-xetex \ | ||
| texlive-lang-chinese \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Trailing whitespace after backslash line continuation characters. While this may work in many shells, it's considered bad practice and can cause issues. Remove trailing spaces after the \ characters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Dockerfile, line 77:
<comment>Trailing whitespace after backslash line continuation characters. While this may work in many shells, it's considered bad practice and can cause issues. Remove trailing spaces after the `\` characters.</comment>
<file context>
@@ -74,9 +74,19 @@ RUN apt-get update && apt-get install -y \
texlive-latex-extra \
texlive-latex-recommended \
texlive-xetex \
+ texlive-lang-chinese \
+ fonts-noto-cjk \
+ fonts-wqy-zenhei \
</file context>
| texlive-lang-chinese \ | |
| texlive-lang-chinese \ |
✅ Addressed in 695a17a
Dockerfile
Outdated
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| RUN mkdir -p /usr/share/fonts/truetype/chinese && \ | ||
| ln -sf /usr/share/fonts/truetype/noto /usr/share/fonts/truetype/chinese/noto && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Incorrect font path: fonts-noto-cjk installs to /usr/share/fonts/opentype/noto/, not /usr/share/fonts/truetype/noto/. The symlink will point to a non-existent directory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Dockerfile, line 87:
<comment>Incorrect font path: `fonts-noto-cjk` installs to `/usr/share/fonts/opentype/noto/`, not `/usr/share/fonts/truetype/noto/`. The symlink will point to a non-existent directory.</comment>
<file context>
@@ -74,9 +74,19 @@ RUN apt-get update && apt-get install -y \
&& rm -rf /var/lib/apt/lists/*
+RUN mkdir -p /usr/share/fonts/truetype/chinese && \
+ ln -sf /usr/share/fonts/truetype/noto /usr/share/fonts/truetype/chinese/noto && \
+ fc-cache -f -v
+
</file context>
| ln -sf /usr/share/fonts/truetype/noto /usr/share/fonts/truetype/chinese/noto && \ | |
| ln -sf /usr/share/fonts/opentype/noto /usr/share/fonts/truetype/chinese/noto && \ |
✅ Addressed in 695a17a
|
Nice! Is it possible to configure it in any way so that it automatically uses a CJK font when needed, so not everyone have to configure it? I think that would be extremely nice, but no worries if not :) |
Thanks for the feedback! Auto-switching for CJK fonts would be very convenient. Currently, it still needs to be manually enabled, as I haven't yet implemented reliable detection for CJK content in source files. 😅 . I'll keep this in mind for future improvements. |
Summary
Added optional Chinese font support for the Pandoc converter, enabling proper rendering of Chinese characters in
Markdown to PDF conversions.
Changes
Environment Variables (src/helpers/env.ts)
Pandoc Converter Enhancement (src/converters/pandoc.ts)
Docker Configuration Example (compose.yaml)
Usage
Enable Chinese font support:
1 environment:
2 - PANDOC_ENABLE_CHINESE_FONT=true
3 - PANDOC_CHINESE_FONT_FAMILY=NotoSansCJK
Mount host font files:
1 volumes:
2 - /usr/share/fonts/truetype/chinese:/usr/share/fonts/truetype/chinese:ro
Testing Recommendations
Test the following scenarios:
This improvement enhances ConvertX's friendliness for Chinese users while maintaining code simplicity and
maintainability.
Summary by cubic
Added optional Chinese font support for Pandoc PDF/LaTeX conversions so Chinese text renders correctly. Controlled via environment variables and disabled by default.
New Features
Migration
Written for commit 498a35b. Summary will update automatically on new commits.