Skip to content

Feature/templates#78

Open
m-messer wants to merge 5 commits into
mainfrom
feature/templates
Open

Feature/templates#78
m-messer wants to merge 5 commits into
mainfrom
feature/templates

Conversation

@m-messer

Copy link
Copy Markdown
Member

Problem/Motivation

A user asked whether Lambda Feedback could be used for teaching Korean, which requires generated PDFs to correctly render Hangul — something the base template can't do without CJK font/package support.

The prior unicode-support change (#76) added that CJK support, but made it unconditional: every generated PDF now always loads xeCJK and defaults to the Noto Sans font, replacing the previous lmodern/sans-serif default for all documents, not just the ones that need Korean/CJK typesetting. This
changed the visual output for every existing caller and paid the CJK-rendering cost even when it wasn't needed.

This PR splits CJK support out into its own, opt-in template so the original lmodern-based rendering remains the default for existing users, while Korean/CJK typesetting is available on request for cases like this one.

Changes

  • Split src/template.latex into two standalone templates:
  • Added an optional template field ("default" | "cjk", defaults to "default") to the request schema in index.ts, mapped through an internal path lookup (not string-concatenated) to select the template passed to Pandoc.
  • Updated Dockerfile to copy both templates into the image; CJK fonts/packages remain installed since the same image serves both templates at runtime.
  • Extended tests:
    • index.test.ts — schema validation for the new template values, and handler assertions that the correct --template= path is passed for "default" (or omitted) vs "cjk".
    • src/compile.test.ts — existing end-to-end tests now run against the default.latex; added a new end-to-end test compiling Korean markdown through cjk.latex and verifying it renders via pdftotext.
  • Updated README.md with a request-fields table and a "Templates" section documenting the template and its values.

Tested by calling the PDF Generator Lambda function directly. Further development for supporting parameters should be added to the platform.

cjk_template_test.pdf
default_template_test.pdf

@peterbjohnson peterbjohnson 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.

This is a good way to add the second template. The cjk template is arbitrary, and there can conceivably be a huge number of templates. The dockerfile imports them all, which is good. The remainder of the code is quite brittle in relying on the hard coded pair of templates currently available.

Questions:

How should we organise templates? Is 'cjk' an appropriate name for a template? Will it be followed by cjk_2?! Or cjk_plus_maths_for_andy?

This is a microservice and we need to consider how it interfaces with clients that call it e.g.

  • Behaviour when template parameter is neither default not cjk? (what does a client want? Is defaulting to 'default' acceptable or not?
  • How will a client discover which templates are available in this microservice? E.g. will the app give a free text box to write a template name (hence see above point)? Or will the service publish an enum that can populate a multiple-choice drop-down in the app? And what if things change - how do we synch?

I appreciate that the proposed change seems innocuous as it doesn't appear to reduce any functionality and adds useful functionality; but it also opens the potential for reliability and maintainability issues in future once the newly introduced features are used.

@m-messer

Copy link
Copy Markdown
Member Author

"The remainder of the code is quite brittle in relying on the hard coded pair of templates currently available."

  • Do we want to support any template that is adding to that directory? We could load from the directory instead of specifying the templates as a constant but it depends on what you want the feature to be.

_Questions:

How should we organise templates? Is 'cjk' an appropriate name for a template? Will it be followed by cjk_2?! Or cjk_plus_maths_for_andy?_

Again, this depends on how you envision the template feature. CJK is the common term for Chinese, Japanese and Korean language support, but in the future it might be different versions. If users can upload arbitary templates, then I imagine the templates will not be stored in the image

_This is a microservice and we need to consider how it interfaces with clients that call it e.g.

Behaviour when template parameter is neither default not cjk? (what does a client want? Is defaulting to 'default' acceptable or not?
How will a client discover which templates are available in this microservice? E.g. will the app give a free text box to write a template name (hence see above point)? Or will the service publish an enum that can populate a multiple-choice drop-down in the app? And what if things change - how do we synch?_

Maybe this could be the first definition for muEd content generation? But without knowing the full requirements, it's difficult to say what the default should be and how the templates will be made available. We could add a page in the admin settings for templates, and save the templates to S3 and the database with the paths that they microservice could pick up. But this is out of scope of my work as it requires work on the app.

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.

2 participants